Define a standard for naming data providers in PHPUnit-based tests

Björn Brala (bbrala) commented in the coding standards meeting, #3456119: Coding Standards Meeting Tuesday 2024-06-18 2100 UTC → , that there is a rector for removing test from dataproviders, https://github.com/rectorphp/rector-phpunit/blob/main/docs/rector_rules_...

The agreement and documenting of this standard has become more urgent in the last few weeks, because the PHPStan in contrib pipelines running 'next minor' (which is currently 11.2-dev) now report errors such as

----- --------------------------------------------------------------- 
  Line   tests/src/Functional/SchedulerTokenReplaceTest.php                  
 ------ --------------------------------------------------------------- 
  17     @dataProvider dataSchedulerTokenReplacement() related method not    
         found.                                                              
         🪪  phpunit.dataProviderMethod  

I don't know what method name it is expecting, but whatever it is I guess this should be our new standard, to be documented in the Coding ld be. Probably it would be a new paragraph in php/php-coding-standards#naming →

The link in #2 is broken. Any ideas where it was meant to point?

I found the core issue that fixed the names - 📌 Fix PHPStan L1 errors "@dataProvider Foo related method not found." Fixed

But actually, PHPStan at level 1 is not actually checking any expected name of the data provider function. The warning above is when checking that the test function contains the expected tag @dataProvider whateverYouCalledIt but without the () after the name.

This could also be added into the new section of the standards page.

Strong -1 from me
We don't enforce method names anywhere else, so I don't think enforcing one for data providers is appropriate

Yes, the urgency which I stated in #3 was in fact not the case. The PHPStan warning is only complaining about the matching @dataProvider tag in the test function's doc block.

We do have a standard for method/function names and class names which core violates. I am not convinced that Drupal should not enforce it's own standard.

As for core, they are not enforced because a core committer has pushed back on implementing the naming standard for methods 📌 Use 'Drupal.NamingConventions.ValidFunctionName' in core/tests RTBC . Instead of discussing that I chose to work on implementing other standards.

For completeness that standards for naming a function are in item 2, 3, and 4 of Classes, Methods and Properties → . It states the following

Methods and class properties should use lowerCamel naming. In Drupal 8, properties of configuration entities are exempt of these conventions. Those properties are allowed to use underscores.
If an acronym is used in a class or method name, make it CamelCase too (SampleXmlClass, not SampleXMLClass). [Note: this standard was adopted in March 2013, reversing the previous standard.]

It is about style not the actual name.

I'm +1 on this.

But:

> *single-use* providers

What happens when a single-use provider is converted to a multi-use provider because we add another test which uses it too?

The link in #2 no longer works.
I also tend to agree with @joachim in #11, single use providers can easily go to provide for multiple test methods.

I also just commented this in the slack discussion:

I don't think the names have to match the test names. I have gotten into the habit of doing something like provideMenuLinks - a name that is based on what is being returned. If there are multiple needs it could be provideInvalidMenuLinks/provideValidMenuLinks or even provideMenuLinksAllUpperCase.
I agree with the having provide or provider in the method names, and I personally prefer the earlier, because that way we kind of force people to come up with a name that means something?

#6:

We don't enforce method names anywhere else, so I don't think enforcing one for data providers is appropriate

I mean you're right about this, it's not in the coding standards, but all test methods start with test (public function testSomething). It's also possible to use another name and annotate with @test, but I don't think we do that anywhere? At least I couldn't find something.

On the other hand, if dataproviders are required to be documented then the name doesn't really matter.