Deprecate Drupal\Tests\TestRequirementsTrait::checkRequirements and ::checkModuleRequirements

One proposal could be to replace the annotation @requires module xxxx with an attribute, say #[RequiresDrupalModules(['xxxx']), and usual deprecation dance.

However, there only are 3 tests using this feature:

• KernelTestBaseTest
• BrowserTestBaseTest
• FileUploadTest

Two are to test the feature itself, and for the third (FileUploadTest) the check is actually not even triggered, not sure why - but in any case it can be replace with a simple markTestSkipped.

So not sure - is it something we need to keep supporting, or can we get rid of it altogether?

This has been broken for some time, see #3261817: TestRequirementsTrait::checkRequirements() does not work as expected →

Well two years not working means no one needs it. I propose to deprecate for removal.

Well two years not working means no one needs it. I propose to deprecate for removal.

Valid point IMHO, deprecation dance++

Deprecation seems good and has test coverage.

More accurate title.

Updated CR

After a night sleep (I know, I shouldn't do that...), I'm a bit worried to drop this functionality.

Core doesn't use it, that's clear by now, and it's broken, but contrib uses it (a lot): http://codcontrib.hank.vps-private.net/search?text=%40requires%20module&...

Not sure if since it's not working, fixing it/replacing it with an annotation could/would mean contrib tests will break, or is this long superseded by adding the modules-on-which-to-depend in the require-dev section of the modules composer.json and is this also dead code in contrib?

This has probably been broken for 4 years now, since when #2950132: Support PHPUnit 7 optionally in Drupal 8, while keeping support for ^6.5 → was committed.

Personally in the contrib modules I maintain I add the test required modules in the require-dev of the composer.json file. In fact it’s a must have if you want the CI tools to install them.

Given there was little activity over in #3261817: TestRequirementsTrait::checkRequirements() does not work as expected → I also suspect this has been broken since then but nobody in contrib land has noticed, so to me that means it's not worth maintaining (or in this case, refactoring for PHPUnit 10), and so I agree this should be deprecated for removal.

Personally in the contrib modules I maintain I add the test required modules in the require-dev of the composer.json file.

(Emphasis mine)

Thanks, there we have it, kill it with fire!

We should have a followup though to clean up FileUploadTest, as those are six test methods that we should skip entirely if we can.

I think better to that here, it’s an unusual deprecation and leving things around won’t be good. Thre’s a comment somewhere, too.

Done.

Clean up makes sense, nice catch in core/tests/Drupal/KernelTests/KernelTestBase.php

The wiki has a reference to using @requires module that should be removed.

https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/phpunit-... →

Thanks @quietone, removed section from the wiki page.

Committed 614ff9d and pushed to 11.x. Thanks!

Also published the change record.

mondrake → closed merge request !6412

Automatically closed - issue fixed for 2 weeks with no activity.