Add per-test fixture definition

[martin-rueegg]

Currently, test data is configured in the test config yaml files and can be tweaked in the HumHubDbTestCase::$fixtureConfig, a feature that is hardly documented.

Loading and unloading of data takes time and makes tests slow. So it would be preferred to do it as little as possible.

To test a class that has both methods that need DB access and other that don't, you either need to create two test classes, one with and one without fixtures to be loaded. Alternatively you can tweak HumHubDbTestCase::$fixtureConfig during setUp based on the Unit::getName(). However, while the former solution might still load unnecessary data, the latter solution disconnects the specification of the data-set from the test.

Suggested Feature (update 2023-11-30)

This PR doe two things:

• deprecate the current HumHubDbTestCase::$fixtureConfig in favor of a new attribute-based solution.
• introduce a new attribute type (FixtureConfig) that is supported on class and method level and allows to customize the fixture configuration on the respective level.

The following functionality is supported:

• test-level configuration overwrites unit-level configuration overwrites configuration-level setting
• use the attribute #[FixtureEmpty] to disable fixtures for the current unit/test
• the base attribute FixtureConfig allows to
  • define a default set of fixtures, that are available in the attribute configuration
  • use, reduce, or extend the default set through the attribute configuration:
    • $useConfig a flag to fall back to the configuration-level setting
    • $useDefault a flag to fall back to the default configuration of the attribute that extends from FixtureConfig and defines such a default set (see FixtureDefault)
    • $fixtures an array to cusomize the configuration
      • can be a pure fixture configuration, i.e. ['user' => ['class' => UserFixture::class], ...]
      • can include the keyword default (as array value) which will mix in the default set
      • can include any "alias" that corresponds to an array key in the default set (e.g. user)

Examples

Disable fixtures on class-level, but use fixtures from configuration for one test

#[FixtureEmpty]
class SomeTest extends Unit
{
	// #[FixtureEmpty] is synonym with #[FixtureConfig([], useConfig: false, useDefault: false)]

	public function testFoo(): void
	{
	  // this test uses no fixtures (inherited from the class)
	}

	#[FixtureConfig(useConfig: true)]
	public function testBar(): void
	{
	  // this test uses the fixtures from the configuration
	}
}

Define fixtures on class-level, but disable them on one test

#[FixtureConfig([
  'user' => ['class' => UserFixture::class],
  'group' => ['class' => GroupFixture::class],
])]
class SomeTest extends Unit
{
	public function testFoo(): void
	{
	  // this test uses the "user" and "group" fixtures (inherited from the class)
	}

	#[FixtureEmpty]
	public function testBar1(): void
	{
	  // this test uses no fixtures
	}

	#[FixtureConfig(['user'])]
	public function testBar2(): void
	{
	  // this test uses only the "user" fixture from the class
	}
}

Define a default set of fixtures and use that in the test

#[Attribute]
class FixtureDefault extends FixtureConfig
{
    public function getDefaultFixtures(): array
    {
       return [
            'user' => ['class' => UserFixture::class],
            'group' => ['class' => GroupFixture::class],
        ];
    }
}


#[Attribute]
class FixtureSpecialSet extends FixtureDefault
{
    public function getDefaultFixtures(): array
    {
       return parent::getDefaultFixtures() + [
            'articles' => ['class' => ArticleFixture::class],
        ];
    }
}



#[FixtureDefault]
class SomeTest extends Unit
{
	public function testFoo(): void
	{
	  // this test uses the "user" and "group" fixtures (inherited from the class, defined in FixtureDefault)
	}

	#[FixtureEmpty]
	public function testBar1(): void
	{
	  // this test uses no fixtures
	}

	#[FixtureDefault(['user'])]
	public function testBar2(): void
	{
	  // this test uses only the "users" fixture from the class
	}

	#[FixtureDefault(['user', ['articles' => ['class' => ArticleFixture::class]])]
	public function testBar3(): void
	{
	  // this test uses only the "user" fixture from FixtureDefault, and adds an additional fixture "articles"
	}

	#[FixtureDefault(['default', ['articles' => ['class' => ArticleFixture::class]])]
	public function testBar4(): void
	{
	  // this test uses all fixtures from FixtureDefault, and adds an additional fixture "articles"
	}

	#[FixtureSpecialSet]
	public function testBar5(): void
	{
	  // this test uses all fixtures from FixtureSpecialSet (which includes those from FixtureDefault)
	}
}

The advantage of the attribute-only solution is that

• there are not too many places to specify fixtures, which all need to be resolved
• There are:
  • configuration
  • class attribute
  • method attribute
• default sets can be made available to be re-used in class or method attributes alike
• the specified fixture set is easy to find as IDEs support navigation on the attribute class, where the fixtures are defined.

Suggested Feature (outdated)

This PR doe two things:

1. It allows new settings in $fixtureConfig:
   • 'default' rather than just ['default']
   • 'alias' or ['alias']: the alias denotes the fixture definition key as specified in the current class' getDefaultFixtures(). It will be replaced with the respective fixture definition.
   • false: fixtures are disabled
   • true: If configured, the fixtures from the configuration will be used, or the default set otherwise. This
     differs from null insofar as it will fall back to default if there is no fixture specified in the
     configuration.
2. Allows to specify the fixture set in the test method's $fixtures parameter, e.g. like so:

   public function testFunctionWithDefaultFixtureSet($fixtures = 'default') {
      // useful when `$fixtureConfig = null`
   };
   
   public function testFunctionWithNoFixtures($fixtures = false) {
      // useful when `$fixtureConfig = 'default'`
   };
   
   public function testFunctionWithDefaultFixtureSet($fixtures = 'file') {
      // useful when `$fixtureConfig = null` and only the file table is required
   };
   
   public function testFunctionWithDefaultFixtureSet($fixtures = null) {
      // useful when `$fixtureConfig = file` but you actually need the data set from the configuration
   };

PR Admin

What kind of change does this PR introduce?

• Feature

Does this PR introduce a breaking change?

• No

The PR fulfills these requirements

• It's submitted to the develop branch, not the master branch if no hotfix
• When resolving a specific issue, it's referenced in the PR's description (e.g. Fix #xxx[,#xxx], where "xxx" is the Github issue number)
• All tests are passing
• New/updated tests are included
• Changelog was modified

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

[martin-rueegg]

Ping @luke-

[luke-]

@martin-rueegg Thanks for the PR.

• I'm not a big fan when a variable like $fixtureConfig now can have different types as value. With your changes now string, array, boolean are supported. Do we really need this flexibility?

• I understand the problem with the different fixtures per test method. But currently the test methods in PHPUnit & Co are always without additional parameters. https://codeception.com/docs/UnitTests#Unit-Testing Not sure it's a good idea to add some specials here. Maybe this could also led to problems in future, when PHPUnit introduces any changes here. What do you think of the following idea to solve the problem:
  Introduce a new $fixtureMethodConfig = [['method1', 'method2'] => ['fixture1', 'fixture4'], ['method3'] => []]; attribute which allows a per method mapping of fixture. If a method is not specified here, the default fixture is used.

[martin-rueegg]

• I'm not a big fan when a variable like $fixtureConfig now can have different types as value. With your changes now string, array, boolean are supported. Do we really need this flexibility?

I too favor typed variables. In this case I see it as a convenience, as IDEs do easier support different types at the top level of an variable as they do within an array. But yes, writing 'default' rather than ['default'] is not a big gain in convenience, it just makes it more intuitive, unless the field is actually strongly typed to e.g. ?array.

So, would the following better suit you?

protected ?array $fixtureConfig = ['default'];

• ['default']: use getDefaultFixtures()
• ['alias1', 'alias2', ...]: use only the fixture(s) denoted by the corresponding key(s) as specified in the current class' getDefaultFixtures().
• null: fixtures are disabled
• [] (empty array): If configured, the fixtures from the configuration will be used, or the default set otherwise. This
  differs from null insofar as it will fall back to default if there is no fixture specified in the
  configuration.

• I understand the problem with the different fixtures per test method. But currently the test methods in PHPUnit & Co are always without additional parameters. https://codeception.com/docs/UnitTests#Unit-Testing Not sure it's a good idea to add some specials here. Maybe this could also led to problems in future, when PHPUnit introduces any changes here.

I see.

What do you think of the following idea to solve the problem:
Introduce a new $fixtureMethodConfig = [['method1', 'method2'] => ['fixture1', 'fixture4'], ['method3'] => []]; attribute which allows a per method mapping of fixture. If a method is not specified here, the default fixture is used.

In comparison to my solution, this places the definition for a given method to a different place within the code. I liked the fact that the definition of the fixture is visible and obvious straight away.

But I'm sure we could still do something along the lines and the author of the test can still add a @see static::$fixtureMethodConfig to make the customized fixture noticeable.

The suggested format ([['method1', 'method2'] => ['fixture1', 'fixture4'], ['method3'] => []] would not work though, as ['method1', 'method2'] is not a valid array key. But we could maybe do:

$fixtureMethodConfig = [
   // option 1: multiple methods with non-associative index:
   [
       // methods in key 0
       ['method1', 'method2'],
       // fixtures in key 1 (following the rules of $fixtureConfig)
       ['fixture1', 'fixture4'],
    ]

    // option 2: single-method configuration with associative index:
    'method3' => [
       // fixtures in key 0 (following the rules of $fixtureConfig)
    ]
];

Or we always put the fixture configuration in key 0, and the method name(s) in either the parent key or as an array in key 1 (simply changing the order of the arrays in option 1 above):

$fixtureMethodConfig = [
   // option 1: multiple methods with non-associative index:
   [
       // fixtures in key 0 (following the rules of $fixtureConfig)
       ['fixture1', 'fixture4'],
       // methods in key 1
       ['method1', 'method2'],
    ]

    // option 2: single-method configuration with associative index:
    'method3' => [
       // fixtures in key 0 (following the rules of $fixtureConfig)
    ]
];

[luke-]

• I'm not a big fan when a variable like $fixtureConfig now can have different types as value. With your changes now string, array, boolean are supported. Do we really need this flexibility?

I too favor typed variables. In this case I see it as a convenience, as IDEs do easier support different types at the top level of an variable as they do within an array. But yes, writing 'default' rather than ['default'] is not a big gain in convenience, it just makes it more intuitive, unless the field is actually strongly typed to e.g. ?array.

So, would the following better suit you?

protected ?array $fixtureConfig = ['default'];

• ['default']: use getDefaultFixtures()
• ['alias1', 'alias2', ...]: use only the fixture(s) denoted by the corresponding key(s) as specified in the current class' getDefaultFixtures().
• null: fixtures are disabled
• [] (empty array): If configured, the fixtures from the configuration will be used, or the default set otherwise. This
  differs from null insofar as it will fall back to default if there is no fixture specified in the
  configuration.

Looks good for me. Thank you!

What do you think of the following idea to solve the problem:
Introduce a new $fixtureMethodConfig = [['method1', 'method2'] => ['fixture1', 'fixture4'], ['method3'] => []]; attribute which allows a per method mapping of fixture. If a method is not specified here, the default fixture is used.

In comparison to my solution, this places the definition for a given method to a different place within the code. I liked the fact that the definition of the fixture is visible and obvious straight away.

Maybe annotations could be a cool solution?

    /**
     * @fixture key1
     * @fixture key2
     */
    public function testSomething(){}

Unfortunately, I have not found anything for Codeception and PHPUnit. Perhaps worth to contribute there?
https://docs.phpunit.de/en/10.3/annotations.html

But I'm sure we could still do something along the lines and the author of the test can still add a @see static::$fixtureMethodConfig to make the customized fixture noticeable.

The suggested format ([['method1', 'method2'] => ['fixture1', 'fixture4'], ['method3'] => []] would not work though, as ['method1', 'method2'] is not a valid array key. But we could maybe do:

$fixtureMethodConfig = [
   // option 1: multiple methods with non-associative index:
   [
       // methods in key 0
       ['method1', 'method2'],
       // fixtures in key 1 (following the rules of $fixtureConfig)
       ['fixture1', 'fixture4'],
    ]

    // option 2: single-method configuration with associative index:
    'method3' => [
       // fixtures in key 0 (following the rules of $fixtureConfig)
    ]
];

Or we always put the fixture configuration in key 0, and the method name(s) in either the parent key or as an array in key 1 (simply changing the order of the arrays in option 1 above):

$fixtureMethodConfig = [
   // option 1: multiple methods with non-associative index:
   [
       // fixtures in key 0 (following the rules of $fixtureConfig)
       ['fixture1', 'fixture4'],
       // methods in key 1
       ['method1', 'method2'],
    ]

    // option 2: single-method configuration with associative index:
    'method3' => [
       // fixtures in key 0 (following the rules of $fixtureConfig)
    ]
];

I'm open two both variants. It is somewhat similar to the Yii2 Model Rules.

[martin-rueegg]

Maybe annotations could be a cool solution?

    /**
     * @fixture key1
     * @fixture key2
     */
    public function testSomething(){}

Attributes would even be cooler, I think. As they support PHP values and can be accessed using reflection. However, they're only available as of PHP v8. When do you plan to drop v7.4 support?

Otherwise, yes, annotations might be something, as there are tools to read them. But it would require more engineering.

I'm open two both variants. It is somewhat similar to the Yii2 Model Rules.

Yes, exactly. My suggestion 1 is maybe a bit more intuitive to use. Suggestion 2 would be slightly easier to evaluate. Maybe we should go for suggestion 1 then. Unless we wait for PHP 8 and use Attributes.

[luke-]

Maybe annotations could be a cool solution?

    /**
     * @fixture key1
     * @fixture key2
     */
    public function testSomething(){}

Attributes would even be cooler, I think. As they support PHP values and can be accessed using reflection. However, they're only available as of PHP v8. When do you plan to drop v7.4 support?

Otherwise, yes, annotations might be something, as there are tools to read them. But it would require more engineering.

I'm open two both variants. It is somewhat similar to the Yii2 Model Rules.

Yes, exactly. My suggestion 1 is maybe a bit more intuitive to use. Suggestion 2 would be slightly easier to evaluate. Maybe we should go for suggestion 1 then. Unless we wait for PHP 8 and use Attributes.

Actually the plan is to "soft" drop PHP 7.4 support with v1.16. So for me we can use Attributes

[martin-rueegg]

I see. So the v7.4 tests will be dropped, too? Because those would fail. Or would it mean to somehow exclude such tests from 7.4 test suite? E.g. by extending the @skip annotation to use with @skip 7.4 or something along those lines.

[luke-]

Hmm, I'm not sure how we're going to handle it yet.
It's a good idea to skip them for now with <8.0. Maybe something like here? https://www.amitmerchant.com/skip-a-phpunit-test-conditionally-in-php/

[luke-]

@martin-rueegg Should we really mark "fixtureConfig" as deprecated. Shouldn't we just allow the attribute as an additional overwrite for methods?

[martin-rueegg]

@martin-rueegg Should we really mark "fixtureConfig" as deprecated. Shouldn't we just allow the attribute as an additional overwrite for methods?

Usually it is you in favor of less options ... :-)

I personally think whether adding a line

#[FixtureDefault]
// which is an "alias" for
#[FixtureConfig(['default'])]

or a line

protected ?array $fixtureConfig = ['default'];

does not really make a difference in terms of effort/convenience.

The advantage of the attribute-only solution is that

• there are not too many places to specify fixtures, which all need to be resolved
• There are:
  • configuration
  • class attribute
  • method attribute
• default sets can be made available to be re-used in class or method attributes alike
• the configuration is more type-restricted. There is no special treatment of [] or [false] which need more documentation than just using #[FixtureConfig(useDefault: true)] or #[FixtureEmpty] (though the later could maybe be named FixtureNone for more clarity)

I guess the attribute-base mechanism could be proposed to codeception, as you suggested?

[luke-]

@martin-rueegg Should we really mark "fixtureConfig" as deprecated. Shouldn't we just allow the attribute as an additional overwrite for methods?

Usually it is you in favor of less options ... :-)

Hehe, that's right. Besides Keep it Simple, I want to change or refactor as few things as possible :-)

I personally think whether adding a line

#[FixtureDefault]
// which is an "alias" for
#[FixtureConfig(['default'])]

or a line

protected ?array $fixtureConfig = ['default'];

does not really make a difference in terms of effort/convenience.

Ok, I just saw that '$fixtureConfig` is not used in our modules and only a little in the core. This means we could also remove it directly. Without marking it as deprecated first.

The advantage of the attribute-only solution is that

• there are not too many places to specify fixtures, which all need to be resolved

• There are:

  • configuration
  • class attribute
  • method attribute

• default sets can be made available to be re-used in class or method attributes alike

• the configuration is more type-restricted. There is no special treatment of [] or [false] which need more documentation than just using #[FixtureConfig(useDefault: true)] or #[FixtureEmpty] (though the later could maybe be named FixtureNone for more clarity)

I guess the attribute-base mechanism could be proposed to codeception, as you suggested?

I'm not that familiar with the project, but it would of course be great to implement it there as a feature and to have the documentation for it there too.

Would you like to try that?

[martin-rueegg]

Ok, I just saw that '$fixtureConfig` is not used in our modules and only a little in the core. This means we could also remove it directly. Without marking it as deprecated first.

Awesome.

I guess the attribute-base mechanism could be proposed to codeception, as you suggested?

I'm not that familiar with the project, but it would of course be great to implement it there as a feature and to have the documentation for it there too.

Would you like to try that?

It is being discussed in

• [Feature] Per-unit and per-class fixture configuration - PR welcome? Codeception/module-yii2#92