Closed (fixed)
Project:
Drupal core
Version:
9.5.x-dev
Component:
phpunit
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
3 May 2021 at 16:07 UTC
Updated:
17 Oct 2022 at 14:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
catchComment #3
alexpottThis happens when a unit test that touches the database runs at the same time as tests in\Drupal\Tests\Scripts\TestSiteApplicationTest::testInstallWithNonExistingFile
You can cause the error by running
./vendor/bin/phpunit -v core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php --filter testInstallScriptin one terminal and then in another terminal you can do./vendor/bin/phpunit -v core/tests/Drupal/Tests/Scripts/TestSiteApplicationTest.php --filter testInstallWithNonExistingFilerepeatedly till it fails for this reason.This used to happen on tests for security issues because they ran all tests in one go whereas core tests use core/drupalci.yml which splits them up and makes this not happen because this test's methods are run sequentially and there are no other unit tests that create database tables.
Comment #4
alexpottAfter discussing this with mixologic it's because of #3038596: Mechanism that adds custom drupalci.yml configurations does not detect configs in new branches. and I've fixed this before - see #3122328: TestSiteApplicationTest::testInstallWithFileWithNoClass() fails on 9.1.x
Let's leave this one open so we can find it and also fix it once and for all. The correct fix is to do something in CI about #3038596: Mechanism that adds custom drupalci.yml configurations does not detect configs in new branches.
@mixologic has inserted a row into the CI db to fix the problem for 9.3.x
Comment #5
catchTrying a re-title.
Comment #6
ressaThanks for fixing this, it looks like the failing test passes now:
19:08:01 Drupal\Tests\Scripts\TestSiteApplicationTest 7 passesFrom https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/38131/...
Comment #8
quietone commentedAccording to #4 this will be fixed by #3038596: Mechanism that adds custom drupalci.yml configurations does not detect configs in new branches., which was committed 5 days ago.
What, if anything is left to do here?
Comment #9
quietone commentedThis is one of the fortnightly triage issues for bug smash.
Comment #10
alexpottI think given that #3038596: Mechanism that adds custom drupalci.yml configurations does not detect configs in new branches. this is no longer critical. However I think we could still decide to remove the table counting from \Drupal\Tests\Scripts\TestSiteApplicationTest::testInstallWithNonExistingFile() as it is counting tables on the main connection of the runner so that's always going to be fragile. Maybe the correct thing to do here is to test something else from the install process other than table creation. Looking at \Drupal\TestSite\Commands\TestSiteInstallCommand::execute() and when \Drupal\TestSite\Commands\TestSiteInstallCommand::getSetupClass() perhaps the best we can do it test for the exception text (as we are already doing) and be done.
Comment #11
alexpottHere's the suggested fix
Comment #12
quietone commentedJust IS updates
Comment #14
borisson_That seems like a reasonable thing to do, the removal of this part reduces fragility which is a great thing to do.
Comment #16
catchCommitted/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!