Problem/Motivation
See https://git.drupalcode.org/project/drupal/-/jobs/94869
PHPUnit Test failed to complete; Error: PHPUnit 9.6.8 by Sebastian Bergmann
and contributors.
Testing Drupal\Tests\system\Functional\Theme\ToolbarClaroOverridesTest
F 1 / 1
(100%)
Time: 00:08.330, Memory: 4.00 MB
There was 1 failure:
1)
Drupal\Tests\system\Functional\Theme\ToolbarClaroOverridesTest::testClaroAssets
Failed asserting that false is true.
/builds/project/drupal/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/builds/project/drupal/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/project/drupal/core/modules/system/tests/src/Functional/Theme/ToolbarClaroOverridesTest.php:141
/builds/project/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 1, Assertions: 36, Failures: 1.
Steps to reproduce
Repeat test 1000 times
Run 1 - 4/1000 fails
Run 2 - 18/1000 fails
Proposed resolution
As per comment in #4 it seems we need to wait for config save and cache writes to complete before issuing the second drupalGet. Using WaitTerminateTestTrait seems to fix this when repeating 1000 times.
Run 1 - 0/1000 fails
Run 2 - 0/1000 fails
Note that using the trait needed a return type added, which in turn removed a bunch of things from the baseline. I think that's better than adding a new baseline entry.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3387939
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3387939-repeat-fix
changes, plain diff MR !11610
- 3387939-repeat
changes, plain diff MR !11607
- 3387939-fix
changes, plain diff MR !11612
Comments
Comment #2
catchComment #3
quietone commentedComment #4
mstrelan commentedRan in to this again at https://git.drupalcode.org/issue/drupal-3316317/-/jobs/4767068, The line number is now 143, not 141, but otherwise the same.
This one is unusual because it's not a javascript test, so it's not a matter of waiting until the class is applied, it should have been there from the start. I've attached the browser output from a fail and a pass, and sure enough the
claro-classes don't appear in the output of the failed test. The only other differences are the assets query string and the user permissions hash.I looked at where the
claro-toolbarandclaro-toolbar-menuclasses come from and they are added directly in twig files in the claro theme. These files are overrides of twig files in the toolbar module, with no other changes, so this points to an issue with theme suggestions not being correctly applied. The test is loading the page with initial config, then setting config via API, then loading the same page again. I'm thinking we need to manually clear a cache, or perhaps wait for a cache to finish writing before loading the page.Comment #10
mstrelan commentedUsing
WaitTerminateTestTraitfixes this. Updated the IS with links to test runs.Comment #11
acbramley commentedRan into this one on #3347015: Consider using the administration theme when editing or creating content by default which was really confusing as it seemed like it was caused by the changes, spent quite a while trying to figure that out so this would be great to fix permanently.
The explanation in #4 and IS lines up with the solution and the repeated branches demonstrate this fixes the random fails.
Comment #14
larowlanCommitted to 11.x - thanks!