Probably no one cares, but current state is still like the OP reports and that's rather annoying since it prevents fluent interface.
Thank you. My final idea here would be to add a separate class holding all the configuration of the test runner - mimicking somehow what PHPUnit or Paratest are doing. That would include processing of the CLI arguments but also of the ENV varaibles that right now is spread in many places. But that would be even bigger change... so here it's a transitional state.
Anyway... sorry to bump you across the issue hierarchy, but 📌 Reduce run-tests.sh complexity in spawning subprocesses Active would need to be done prior than this as that one will remove some of the CLI arguments and make this one simpler.
I’ll add tests, no worries. But I’d rather like the referenced issue be done first, so if you can please review it and its blocker.
There are no tests for run-tests.sh… that’s why people including me have been trying to move away its logic in separate testable classes.
Thanks, I've added a new commit on the existing MR then. The idea is to not add the logic to run-tests.sh but to the discovery class. In general, I'd rather take away as much as possible from run-tests.sh into task specific classes. There are a few issues open already for that.
Funnily, it looks like PHPUnit\TextUI\Configuration\TestSuiteBuilder is not using the groups passed in configuration to pre-filter the test - all tests are discovered regardless of their group based on the testsuite, and then only those not part of the specified groups skipped while executing them (all PHPUnit internals here, I mean). So... forget about my proposal.
Thank you, can you please target branch 5.0.x - we will likely not have a 4.1.0 release for the same reason of 💬 Stable release imagemagick 4.1.0 Active .
We can backport to 4.0.x once committed to the next major.
Changes are good, but maybe the update test does not need to ignore deprecations in the first place. Worth trying to remove the attribute and see if the test still passes?
It looks like there will never be a 4.1.0, and we will need to jump to 5.0.0 straight, given this is removing support for Drupal 10 - see 🌱 Guidelines for semantic versioning and Drupal core support Active . But 5.0.0 will be after the policy is finalized, and Drupal 11.3 and PHP 8.5 will be GA.
There was a lot of pushback in other modules I maintain when I released a minor dropping support for Drupal 10, connected to what Drupal's update manager reports back: 🐛 file_mdm 3.2.0 update notification issues Active , 🐛 Module reported not compatible with Drupal 10.5.1 Active .
I also found the problem described in #4. Doing ✨ [PP-4] Change run-tests.sh to use Symfony Console classes for command line parsing Postponed would help here, Symfony’s argv parsing logic is very robust.
Thanks - is it correct there are no calls to expectDeprecation() despite the test being flagged to ignore them?
We should not use #[Group('legacy')], but the dedicated #[IgnoreDperecations] attribute.
From testing subsystem maintainership I think this is OK, the documentation still says backports are very conditional on the complexity of the backport. I think it's more a call for release manager, though.
More in general, I think the release cycle of PHPUnit (one major per year) rather collides with Drupal's one - and major disruption to the testing framework is occurring right now and we should somehow acknowledge it.
Example: Drupal 10 is using PHPUnit 9 for testing, where metadata attributes were not introduced yet. We are currently replacing annotations with attributes in Drupal 11 (that is using PHPUnit 11) because PHPUnit 12 no longer supporting them. So current Drupal 11 tests, even now, cannot be backported to Drupal 10 without quite some rework.
But that's a separate discussion I guess?
#8 that's a side effect of cleaning up the same annotation from the abstract base class views/tests/src/Kernel/Plugin/CastedIntFieldJoinTestBase: since class-level annotations are noop on abstract classes, I removed that from there, but then added it back to the concrete classes.
@quietone there will be more coming with 📌 Complete test annotations to attributes conversion for all remaining tests Postponed . With that done, we can proceed with 📌 Enforce removal of PHPUnit annotations from test classes' class metadata Postponed to finally prevent regressions.
The MR is built on top of 🐛 [CI] Components tests coverage metrics differ by PHP version Active , but it's reviewable per se.
It splits the pipeline in three stages
- Lint
- Unit tests
- Tests (i.e. Build+Kernel+Functional that all require a db)
'Lint' and 'Unit tests' starts independently from each other, while 'Tests' starts when both the former ones have completed.
'Unit tests' use a parallel matrix to run multiple PHP versions, instead of individual jobs.
Thanks @smustgrave in fact there are still few outliers that we can tackle once 📌 Complete test annotations to attributes conversion for all remaining tests Postponed is done - and in fact we can adjust the PHPstan rule to prevent more to sneak in. But a followup’s issue.
This issue has completed its purpose as all bulk tests conversions were done.
I'd close this 'Closed (works as designed)' but I thought to set to RTBC first to see if any comments.
@nixvan as far as I remember, the db driver related implementation was just the minimum necessary to fill the needs of listing them as extensions in the install process. So yes, all what was not necessary was intentionally left out, if that's what you mean.
What I also meant in #4 (it's not clear) is that if we want to allow PHPUnit 12 in Drupal 11, this issue is a must AND we need to throw an exception (not a deprecation) if the attribute is not added - that is why I suggest to add the deprecation here for D11.3, and change it to an exception in D11.4 (i.e. in a minor, not a major).
#64 you're right. It's not in 11.2.x, it was only committed to 11.x (yes, at the time when 11.2.0 was still in RC, but not to its branch). So it will be GA only when 11.3.0 will be released.
Changed again the CR.
Thanks!
mondrake → changed the visibility of the branch 3547849-add-runtestsinseparateprocesses-attribute to active.
mondrake → changed the visibility of the branch 3547849-add-runtestsinseparateprocesses-attribute to hidden.
I think this one could be one problem:
PHPStan cache URL - https://git.drupalcode.org/api/v4/projects/59858/jobs/artifacts/3547567-testing-drupal-lint/raw/core/phpstan-tmp/resultCache.php?job=Lint%20cache%20warming
the commit pipeline saves the artifacts in
PHPStan cache URL - xxxxx://git.drupalcode.org/api/v4/projects/59858/jobs/artifacts/11.x/raw/core/phpstan-tmp/resultCache.php?job=Lint%20cache%20warming
instead
Will be fixed by 📌 Complete test annotations to attributes conversion for all remaining tests Postponed .