A check to prevent the use of annotation is already done by PHPUnit itself when it is configured to fail on PHPUnit deprecations. See 📌 Complete test annotations to attributes conversion for Drupal/Test/Component Active for an example.
IMHO we should not replicate such a check here.
Well, the new rule introduces new errors, so either we fix them or we baseline them :)
Because fixing the errors that this rule surfaces is not trivial. So better leave that to follow ups IMHO, while the rule prevents adding more leaks.
@ceonizm confirmed that the MR here fixes 🐛 Error at the end of any PHPunit test Active
This is implemented in PhpUnitTestDiscovery.
Can you check if 📌 [PP-1] Convert all transactions to explicit COMMIT Postponed solves the problem?
#43 you mean manually? An idea could be to create via createTable
a PostgreSQL table with a very long name. Then inspecting the database you would find a table with an abbreviated name 63 chars long. Any Database API select/insert/update/query etc with the long name in Drupal would return data from the real table with abbreviated name.
mondrake → created an issue.
#7 that’s 📌 [CI] Stop post editing regenerate PHPStan baseline Active . It’s blocked on a PHPStan issue upstream.
📌 Allow indicating alternative phpunit.xml than core's when testing via run-tests.sh Active is committed to core.
Underlying code in HEAD changed significantly in the last year. Let's close this now.
Blocked on upstream expectUserDeprecationMessage*() fails when test is run in separate process
mondrake → created an issue.
mondrake → created an issue.
📌 Allow indicating alternative phpunit.xml than core's when testing via run-tests.sh Active has taken over here. We need an issue to remove PHPUnit 10 support, but let's create a new one instead of reverting this one to its early state.
This is now getting done in 📌 Allow indicating alternative phpunit.xml than core's when testing via run-tests.sh Active
mondrake → created an issue.
No longer necessary.
Since run-tests.sh is no longer spawning subprocesses of run-tests.sh itself, we no longer need to persist the test result SQLite database on disk to allow parent-child processing. So, we can try using a :memory:
SQLite database within the main run-tests.sh process.
mondrake → created an issue.
rerolled
mondrake → created an issue.
Discussed this issue status in Slack, https://drupal.slack.com/archives/C079NQPQUEN/p1752837466231169
Return this to Fixed as it was before reopening. PHPStan is already running on PHP 8.4 as part of separate issue, so I will file a separate issue for the follow-up currently open.
Added draft CR
To be precise, PHP 8.4 is not a strict requirement. PHPUnit 11 works with PHP 8.3. as well (core tests are by default executed on 8.3). This issue 📌 Switch the default test environment to PHP 8.4 Active is going to change the default to PHP 8.4.
@fjgarlin I commented inline - can you please elaborate the suggestion?
Think this can be closed now.
Replied inline. Yes, it's odd. Hopefully one day we will get rid of the test results database and of the --sqlite
argument in the run-tests.sh script.
Changes were made through other issues; let's close and open a different one based on current state if needed.
mondrake → created an issue. See original summary → .
#17
1. No. We need a meta and reparenting all these issues to that. I will open one.
2. Not that I am aware of.
3. Not that I am aware of.
4. No plan. I can only comment that PHPUnit 10+ does have a #[CoversMethod]
, but that only can be set at the class level. So theoretically tests classes could be split per each method that wants to be covered in detail, but that is a lot of work, and manual one. Maybe a policy issue for discusssion?
Changed mind and added a PHPStan rule checking for either annotations or attributes on abstract test base classes. We have 17. errors to be solved.
mondrake → created an issue.
#14 thanks, I think we need to accept that commits still add annotations for now, it's too big to stop. We'll enforce later by enabling PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION: true
like we're doing in
📌
Complete test annotations to attributes conversion for Drupal/Test/Component
Active
for component tests. But too early now.
Regenerated patch and tagged 'Avoid commit conflicts' tag per https://drupal.slack.com/archives/C079NQPQUEN/p1752247969204049
PHPStan fails in latest pipeline because, since this issue was RTBCed, issue
🐛
The migrate Row class API is incomplete
Active
has introduced a test method with 2 @covers
annotations. Given this MR removed the @coversDefaultClass
annotation from the class, the annotations added do not have a reference class anymore and therefore PHPStan cannot identify to what methods they refer to.
This MR touches 342 files, can therefore become easily stale, and it's requiring non-trivial work to reroll: it's not a rebase, but a full regeneration of the automated patch, reset of the branch, application of the patch, and force push.
I'll ask in Slack if it's worth tagging the issue 'avoid commit conflicts'.
mondrake → created an issue.
This touches 1,304 files, so it could become stale easily. I would encourage at reviewing as is also if not mergeable (at the end of the day this is again and again the same conversion pattern repeating), and then re-run the patch generation last minute before commit, if necessary. Otherwise we spend time running the same steps again with no value.
I suggest to wait for conversions of annotations to attributes to be completed, then check on attributes instead. Probably a PHPStan rule should be written here.
We need to convert all tests from annotation to attributes, first.
I'm afraid we have to reopen this is light of PHPUnit 12 - that is making final
the constructor of TestCase
. We can no longer override it to set programmatically at the base class level.
The issue was reported upstream but no alternatives were suggested: https://github.com/issues/created?issue=sebastianbergmann%7Cphpunit%7C5838
So I think we need to
- add the #[RunTestsInSeparateProcesses] attribute to all existing kernel and functional tests
- add a PHPStan rule that will prevent new kernel and functional tests to miss the attribute
Despite this reducing the number of CPUs, after 📌 Consider moving quickstart tests to build tests Active that moved the two tests to the Build suite, now 📌 Autodetect available CPUs in run-tests.sh for concurrency Active has a record execution time of 14 secs for core PHPUnit tests under PHP 8.5:
https://git.drupalcode.org/issue/drupal-3526459/-/jobs/5815987/viewer
8.3 and 8.4 are slightly below 30 secs, but I think it's the actual runner making the real difference here.
Sorry I can't understand #40 - tests are green and the comment seems not related to this issue. Can you elaborate please?
(not so) random recurring test failures
mondrake → created an issue.
Adjusted the docblocks.
good catch. on that
mondrake → created an issue.
I did not mean to open a fork… must have tapped something inadvertently.
mondrake → made their first commit to this issue’s fork.
mondrake → created an issue.