Problem/Motivation
See #3285193: Temporarily skip random test failures that hide real test failures, part 4.
We currently have tests that fail randomly. Currently we mark the test skipped at fragile point in the test. This means that we lose all coverage of that point and all test coverage coming after this 100% of the time.
Steps to reproduce
See current solution in #3285193: Temporarily skip random test failures that hide real test failures, part 4
Proposed resolution
It would be good if these tests are only marked skipped if the test actually fails in the part of the test that was identified as being fragile. That way we only lose coverage when a random fail occurs
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3285270-2.patch | 2.31 KB | lendude |
Comments
Comment #2
lendudeSomething like this
Comment #3
mondrakeNice, but this would conflict with, for instance, #3271214: Change BrowserTestBase to flag PHPUnit failures instead of throwing Mink exceptions in the usage of
onNotSuccessfulTest. Sounds like we would need an overarching trait for Drupal base test classes to use here, so that any Drupal's test failure post-processing could be orchestrated.Comment #4
mondrakeComment #5
lendudeYeah the fact that everything needs
onNotSuccessfulTestis kind of a pain. Incorporating this in one trait with #3271214: Change BrowserTestBase to flag PHPUnit failures instead of throwing Mink exceptions when that lands sounds like a sensible thing to do.Comment #6
smustgrave commentedYou mean no more random ckeditor5 failures?? Yes please
Triggering for 10
Comment #7
smustgrave commentedNo issues on D10
Comment #8
quietone commentedComment #9
mondrakeFWIW, beware of https://github.com/sebastianbergmann/phpunit/issues/5202 ; in PHPUnit 10 currently TestCase::onNotSuccessfulTest cannot mark tests as skipped or failed anymore.
Comment #10
longwaveIs there anywhere we could start using this right now? We seem to have fixed a lot of the random fails recently, so I am wondering if we should hold off on actually adding this if we currently don't need it - and if we do need it, let's start using it somewhere.
Comment #11
longwaveDiscussed with @Lendude in Slack. Given that PHPUnit 10 won't allow us to do this, there seems little point in adding this now only to have to remove it again later.