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

CommentFileSizeAuthor
#2 3285270-2.patch2.31 KBlendude

Comments

Lendude created an issue. See original summary.

lendude’s picture

Status: Active » Needs review
StatusFileSize
new2.31 KB

Something like this

mondrake’s picture

Nice, 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.

mondrake’s picture

lendude’s picture

Yeah the fact that everything needs onNotSuccessfulTest is 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.

smustgrave’s picture

You mean no more random ckeditor5 failures?? Yes please

Triggering for 10

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

No issues on D10

quietone’s picture

Version: 9.3.x-dev » 10.1.x-dev
mondrake’s picture

FWIW, beware of https://github.com/sebastianbergmann/phpunit/issues/5202 ; in PHPUnit 10 currently TestCase::onNotSuccessfulTest cannot mark tests as skipped or failed anymore.

longwave’s picture

Is 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.

longwave’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

Discussed 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.