Remove the requirement for @group annotation in all tests

Example: https://git.drupalcode.org/project/examples/-/jobs/2454428

Drupal\Core\Test\Exception\MissingGroupException: Missing @group annotation in Drupal\Tests\examples\Unit\TraitsTest in /builds/project/examples/web/core/lib/Drupal/Core/Test/TestDiscovery.php:343

Stack trace:
#0 /builds/project/examples/web/core/lib/Drupal/Core/Test/TestDiscovery.php(170): Drupal\Core\Test\TestDiscovery::getTestInfo('Drupal\\Tests\\ex...', '/**\n * Tests fo...')
#1 /builds/project/examples/web/core/scripts/run-tests.sh(912): Drupal\Core\Test\TestDiscovery->getTestClasses(NULL, Array, 'modules/custom/...')
#2 /builds/project/examples/web/core/scripts/run-tests.sh(168): simpletest_script_get_test_list()
#3 {main}

The test class was missing @group.

Why not just assigning a default group? I'd be easy to change from this:

    if (empty($annotations['group'])) {
      // Concrete tests must have a group.
      throw new MissingGroupException(sprintf('Missing @group annotation in %s', $classname));
    }

To this:

    if (empty($annotations['group'])) {
      // Concrete tests must have a group. Use "default" if none is set.
      $annotations['group'] = "default";
    }

I'll create an MR in case this wants to be considered.

The above suggestion is ready for review in the MR.

The unit test failures seem legitimate as the exception is no longer thrown; there are dailing tests that do not report details and need investigation.

Yeah, I check the errors and there are some tests testing for this missing group, so rather than going after fixing tests, etc. I guess we should ask whether this issue is desired or not.

Do we want to force all classes to have a @group (whether it's via dockblock or annotation)? It might be a reason for this requirement, so it'd be great to know before continuing. If there is a legit reason, then we can close because it works as designed.

To me this is a legitimate issue. @group is only needed in TestDiscovery which is only needed by run-tests.sh which is a Drupal-specific tool. The fix IMHO is good because it only affects TestDiscovery, and allows developers to prevent using a Drupal-specific feature if they’re OK to run tests in the PHPUnit CLI only.

In that case, I made a few more changes and it is ready for review again.

> Why not just assigning a default group? I'd be easy to change from this:

What's the point?

Can we just fix the thing that complains when there's no group?

The part complaining about it is now fixed with the changes in the MR @joachim.

There was some logic in case it was a trait, or abstract class... that's why there is the "default" group still added for actual tests, and no "group" at all for other cases.

With the changes in the MR, you should be able to have an actual test without the "group" annotation and it should still run ok.

I don't understand the need to add a 'default' group. What happens if we don't add it?

From what I see in the code, the whole list of tests is a multiarray sorted by group, so not adding any group would break the array structure.

The current coding standards state that @group is required → .

If this is being changed, then shouldn't that be agreed with the Coding Standards team first? as per the process on https://www.drupal.org/project/coding_standards →

I found two Coding Standards issue relating to this:
#2297541: [policy, no patch] Secondary/additional test @group names →
#2325985: No documentation about @group @coversDefaultClass @covers →

Sooner or later we'll be removing test annotation on behalf of attributes - see 📌 TestDiscovery expects @group annotations, fails with #[Group()] attributes Active which BTW could use some review.

I would suggest to postpone this on
a) that issue
b) conversion of core tests to use of attributes
c) then we could add a PHPStan rule for core to ensure that core tests have the #[Group] attribute to comply with the coding standards, and avoid the current hard failure in run-tests.sh

> From what I see in the code, the whole list of tests is a multiarray sorted by group

Where is that in the code?

> The current coding standards state that @group is required.

I expect they state that because of the requirement in the code which this issue is dealing with. We need to remove the hard requirement before we can remove the standard.

> then we could add a PHPStan rule for core to ensure that core tests have the #[Group] attribute to comply with the coding standards

No, because the whole goal of this issue is to AVOID having to have a pointless group for a test!

Re first part of #30

In core/lib/Drupal/Core/Test/TestDiscovery.php:

    // Sort the groups and tests within the groups by name.
    uksort($list, 'strnatcasecmp');
    foreach ($list as &$tests) {
      uksort($tests, 'strnatcasecmp');
    }

Then in core/scripts/run-tests.sh, all the calls are like $groups = $test_discovery->getTestClasses($args['module']); and the result is accessed like foreach ($groups as $group => $tests) {...

If you want to run all the tests for one module, you can say --group that_module when you run PHPUnit. It's quite useful.

+1 on #29.

Postponing on what we learn in 📌 TestDiscovery expects @group annotations, fails with #[Group()] attributes Active

> If you want to run all the tests for one module, you can say --group that_module when you run PHPUnit. It's quite useful.

I think it's simpler to use the path to the module. You get path autocomplete on the command line that way.