- Issue created by @jonathan1055
- ๐ช๐ธSpain fjgarlin
It might be a PHPUnit 10 vs 11+ thing.
I see this commit has a lot of logic for it https://git.drupalcode.org/project/drupal/-/commit/64212ca3018ce85b76e54... and that the error is coming from the file that was created in that commit
core/lib/Drupal/Core/Test/PhpUnitTestDiscovery.php
- ๐ฌ๐งUnited Kingdom jonathan1055
It might be a PHPUnit 10 vs 11+ thing.
Both the failed and woking jobs are running PHPUnit 10.5.46
Yes is does seem like ๐ Deprecate TestDiscovery test file scanning, use PHPUnit API instead Active is linked with this, as that issue was the reason @catch asked me to test in #28 on #3469828-28: Lower default concurrency โ (not to test this specifically, but to see if affected run times)
That commit also has lots of changes to run-tests.sh too.
This is really good that we are now testing 11.2.0-dev so that we find these things early. My tests are using ref 'main' and that is how this suddently showed up now.
- ๐ฌ๐งUnited Kingdom jonathan1055
A similar problem is happening in Project Browser, where no tests are specfied when running against 11.2.0-dev. The executed run-tests.sh command has the final arguments of
--types PHPUnit-Functional --module project_browser
https://git.drupalcode.org/project/project_browser/-/jobs/5393446
Background on this slack thread
- ๐ฌ๐งUnited Kingdom jonathan1055
This core Change Record โ explains what was done and how to fix tests. We now need to work out the solution for Gitlab Templates. We might need to add some version-checking logic in the phpunit job, so that we have a seemless transition from 11.1 to 11.2, instead of make it dependent on changes in the 'next minor' variant.
- ๐ฎ๐นItaly mondrake ๐ฎ๐น
Generally speaking, I would suggest to bump PHPUnit to 11 for testing Drupal 11.2 onwards. Core is doing it for PHP 8.4 and above, but PHPUnit 11 also supports PHP 8.3. That should fix test discoverability hopefully .
Missing group metadata sounds to me itโs missing as an annotation either, but a @group annotation is requested by run-tests.sh (and by code standards IIRC). Should not have a problem if running via PHPUnit CLI directly.
- ๐ฎ๐นItaly mondrake ๐ฎ๐น
Sorry I see the job failed, but cannot understand what is the MR that originated it? Can you share? Mind if I give a couple tries?
- ๐ฎ๐นItaly mondrake ๐ฎ๐น
IMHO the first thing that should happen here is to bump PHPUnit to 11 anytime Drupal SUT is 11.2 or above, but composer isnโt friendly here, it seems drupal/core-recommended locks packages down.
See attempts at #3527063-5: Update for changes required by D11.2 โ .
Then yes, there maybe bugs to fix in latest core.
However, Image Effects seems to run tests just fine when specifying
--directory
instead of--module
:https://git.drupalcode.org/project/image_effects/-/pipelines/508827 - ๐ฌ๐งUnited Kingdom catch
The core-recommended requirement is because core requires sebastian/diff - but any version 4-7, but then phpunit 10 requires 5.1.1, so co-recommended is getting the version from there. This feels like a fundamental issue in core-recommended - it's checking composer.lock, but composer.lock is based on the full set of dependencies including dev.
Only thing I can think of is gitlab templates switching away from core-recommended entirely, that should allow everything to resolve then.
Changing this to 'critical bug' because it's preventing contrib testing on 11.2.x
- ๐ฌ๐งUnited Kingdom jonathan1055
Thanks for all your input above. It's a lot to grok all at once. Thanks for marking it a bug, but are you sure it's a bug in Gitlab Templates, as we cannot control the changes to run-tests.sh that caused this shift between 11.1 and 11.2. Shouldn't this be resolved in Core? I need to do more testing and investigation, to see how we can progress from here, for the existing contrib projects using @group to divide up their test jobs.
- ๐ฌ๐งUnited Kingdom catch
@jonathan1055 no I'm not sure it's a bug in gitlab templates at all. It might be we have to revert ๐ Deprecate TestDiscovery test file scanning, use PHPUnit API instead Active or commit a follow-up to 11.2 to make things work here. However contrib not being able to test against 11.2 is definitely a critical bug in general.
I do think it's worth a separate issue to stop testing against core-recommended so that things like upgrading phpunit conditionally are possible.
- ๐ช๐ธSpain fjgarlin
Yeah, I would have thought that
drupal/core-dev
would have forced the right version ofphpunit/phpunit
based on the core version.---
Extract from a Project Browser thread talking about this: https://drupal.slack.com/archives/C01UHB4QG12/p1748443110991539
@mondrake so given the cmdline dump above, I think the problem is that you are not passing a directory of tests but a combination types+module in this case PHPUnit will look at the directories to scan in the corresponding testsuite in phpunit.xml, and in PHPUnit 10 they must be explicitly defined for the module directory two options: 1) bump PHPUnit to 11: that will allow using the globstar patterns to find test files also in all the modules/ subdirectories, and files should be found without further ado 2) keep PHPUnit 10 but add a module phpunit.xml or edit the existing one adding explicitly the module directories to scan for tests
From that:
adding explicitly the module directories to scan for tests
Maybe we could use
prepare-phpunit-xml.php
to do something.We also have
expand_composer_json.php
where we could add some logic to force a version of PHPUnit.---
We will continue investigating here, but yeah, I would agree that the real issue seems to be in
drupal/core-dev
and not here since we are calling the script with the documented options. - ๐ฎ๐นItaly mondrake ๐ฎ๐น
One thought: would bumping core lock to PHPUnit 11 with option to downgrade (opposite than HEAD now that locks PHPUnit 10 with option to upgrade) be an option here?
- ๐ช๐ธSpain fjgarlin
I commented on the core issue to see if there is the possibility of reverting, at least to a point where the script works with the documented usage.
- ๐ช๐ธSpain fjgarlin
Core reverted the changes, so we should get tests running again as before: https://www.drupal.org/project/drupal/issues/3497431#comment-16127407 ๐ Deprecate TestDiscovery test file scanning, use PHPUnit API instead Active
We need to re-evaluate if this is now fixed (for now). No code required, just checking that pipelines running 11.2.x+ work again as before.