Added TestData attribute with named data sets and variadic parameter support

[davidbyoung]

This is meant to simplify how inline test data is passed into tests. Rather than requiring everything to be wrapped in an array like

#[TestWith(['foo', 'bar'])]
public function testSomething(string $a, string $b): void
{
    // ...
}

You can now just pass in the parameters directly using the new #[TestData] attribute:

#[TestData('foo', 'bar')]
public function testSomething(string $a, string $b): void
{
    // ...
}

which looks more like an invocation of the test method. Additionally, this PR introduces support for naming data sets using a name parameter, eg

#[TestData('foo', 'bar', name: 'Tests something with "foo" and "bar"')]
public function testSomething(string $a, string $b): void
{
    // ...
}

I have purposely not created an annotation version of this because PHPDoc is not a good place to define PHP values to pass into test methods.

[davidbyoung]

I have a few open questions:

• Should TestWith and TestWithJson be deprecated in favor of the new TestData attribute to unify how to pass inline data to a test?
• I built my implementation sort of off the TestWith functionality, eg marking the TestData metadata's isTestWith() as true. Should I completely separate this implementation from TestWith?
• This solution uses an optional named argument to name the data set. Typically, PHPUnit has avoided supporting named arguments. Is my approach acceptable, or should I consider an alternative?

[sebastianbergmann]

have purposely not created an annotation version

Good, as no new annotations will be added to PHPUnit.

[sebastianbergmann]

This solution uses an optional named argument to name the data set. Typically, PHPUnit has avoided supporting named arguments. Is my approach acceptable, or should I consider an alternative?

Please consider an alternative.

[sebastianbergmann]

I built my implementation sort of off the TestWith functionality, eg marking the TestData metadata's isTestWith() as true. Should I completely separate this implementation from TestWith?

I think we do not need a new Metadata object, only a new attribute. The attribute parser should create the same Metadata object from the two attributes.

[sebastianbergmann]

Should TestWith and TestWithJson be deprecated in favor of the new TestData attribute to unify how to pass inline data to a test?

I do not think so, at least not for now.

[codecov]

Codecov Report

Merging #5167 (eb4cc7f) into main (9c1d355) will decrease coverage by 0.06%.
The diff coverage is 40.47%.

@@             Coverage Diff              @@
##               main    #5167      +/-   ##
============================================
- Coverage     81.24%   81.18%   -0.06%     
- Complexity     5827     5844      +17     
============================================
  Files           629      631       +2     
  Lines         18459    18499      +40     
============================================
+ Hits          14997    15019      +22     
- Misses         3462     3480      +18     

Impacted Files	Coverage Δ
src/Framework/Attributes/TestData.php	0.00% <0.00%> (ø)
src/Metadata/Metadata.php	98.09% <0.00%> (-1.91%)	⬇️
src/Metadata/TestData.php	0.00% <0.00%> (ø)
src/Metadata/Parser/AttributeParser.php	99.61% <80.00%> (-0.39%)	⬇️
src/Metadata/Api/DataProvider.php	54.24% <92.85%> (+8.14%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[davidbyoung]

Thanks for the feedback! Looking to xUnit and NUnit for inspiration, it looks like xUnit does not support named data sets in their [InlineData] attribute, but NUnit does via named parameters, similar to my original approach. As far as I can tell, we have the following options:

• Use named parameters, although this would be a break with how PHPUnit has typically worked.
• Have a separate attribute #[NamedTestData('This is the name', 'foo', 'bar')] whose first parameter is the name and subsequent parameters are passed to the test method. We could still introduce the #[TestData('foo', 'bar')] attribute to prevent us from having to pass in the parameters in an array, just without supporting named data sets.
• Require data sets to always be named as the first parameter in #[TestData('This is the name', 'foo', 'bar')].
• Continue to wrap parameters in an array, similar to this approach but with the negative of having to wrap all parameters in an array, which, at least to me, feels unintuitive.
• Scrap the concept of named data sets altogether.

I can implement a PR for any of these approaches, but wanted to get your input first.

[davidbyoung]

@sebastianbergmann Did you have any preference on my list of proposed ways forward above?

[davidbyoung]

Hi @sebastianbergmann, have you had an opportunity to review the different options I enumerated above? Do any sound preferable to you? Or should I abandon this feature altogether? Thanks!

[sebastianbergmann]

The more I think about this, the more I think that things should stay as they are. Sorry.