I just had a random test fail in ManageGitIgnoreTest, during standard testing with PHP 7.1 & MySQL 5.7. When I hit retest, the tests all passed.
Here's the failing job:
https://www.drupal.org/pift-ci-job/1360793
In case it goes away, here are the results:
|2 | Scaffold.Drupal\Tests\Component\Scaffold\Functional\ManageGitIgnoreTest|
|--- | ---|
| | ✗ | Drupal\Tests\Component\Scaffold\Functional\ManageGitIgnoreTestexception: [Other] Line 0 of sites/default/files/simpletest/phpunit-558.xml: PHPunit Test failed to complete; Error: PHPUnit 6.5.14 by Sebastian Bergmann and contributors. Testing Drupal\Tests\Component\Scaffold\Functional\ManageGitIgnoreTest E.. 3 / 3 (100%) Time: 6.67 seconds, Memory: 289.00MB There was 1 error: 1) Drupal\Tests\Component\Scaffold\Functional\ManageGitIgnoreTest::testManageGitIgnore Exception: Fixtures::runComposer failed to set up fixtures. Command: 'install --no-ansi --no-scripts' Exit code: 1 Output: In Filesystem.php line 217: Could not delete /var/www/.composer/cache/files/masterminds/html5/2832edfbf 5a86e6fe33d640c386772b36f9b00bd.zip: /var/www/html/core/tests/Drupal/Tests/Component/Scaffold/Fixtures.php:383 /var/www/html/core/tests/Drupal/Tests/Component/Scaffold/Functional/ManageGitIgnoreTest.php:95 /var/www/html/core/tests/Drupal/Tests/Component/Scaffold/Functional/ManageGitIgnoreTest.php:105 ERRORS! Tests: 3, Assertions: 12, Errors: 1. THE ERROR HANDLER HAS CHANGED!|
| | ✗ | Unknownfail: [run-tests.sh check] Line 0 of : FATAL Drupal\Tests\Component\Scaffold\Functional\ManageGitIgnoreTest: test runner returned a non-zero error code (2).|
It seems that every test process is using /var/www/.composer/cache/ as the Composer cache. If two tests try to clear the Composer cache at the same time, one might decide to delete a file right the instant before another test deletes the same file. Then you get the file-not-found error.
We'll just need to modify the functional tests to relocate the Composer cache for the tests into a unique temporary directory.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3070853-3.patch | 8.88 KB | greg.1.anderson |
Comments
Comment #2
greg.1.anderson commentedAs discussed in slack:
greg.1.anderson
This one is a race condition.
It seems that every test process is using /var/www/.composer/cache/ as the Composer cache.
If two tests try to clear the Composer cache at the same time, one might decide to delete a file right the instant before another test deletes the same file.
Then you get the file-not-found error.
We'll just need to modify the functional tests to relocate the Composer cache for the tests into a unique temporary directory.
Should be easier to fix than last time.
andypost
I bet composer cache should be shared, just cache clear should not be called from ci, imo it is a server's territory to clear caches
greg.1.anderson
Composer is deciding on its own to do some cache management when 'composer install' is executed. The functional tests in the scaffold component are carefully designed to not need to download anything from packagist. It should be fine if they have their own isolated cache.
I'm not sure why called to composer install never break in the ordinary tests.
Comment #3
greg.1.anderson commentedHere's a patch to isolate the Composer cache directory. I also disable the packagist repository in all of the tests to demonstrate that the tests do not have much need for sharing the general cache.
Comment #4
feyp commentedThanks for looking into this so quickly. Updated IS and issue title based on #2.
Comment #5
greg.1.anderson commentedThe tests pass here, but I'm not sure how to prove anything about these tests. The failure we're trying to fix here was not evidenced the last time when we increased the number of iterations running simultaneously.
Comment #6
mile23Why not just
exec()composer in the environment, rather than making a newApplicationobject? Use--working-dirto specify the fixture directory.Comment #7
greg.1.anderson commentedMaking a new Application object is better because it runs the test code in the same execution context. This is helpful for code coverage calculations (
execed code is not counted as covered) and source-level debugging (although I never had cause for the later yet).Some of the tests
execComposer because making a new Application object does not always work. I don't remember details about why I needed to exec sometimes; I favored using a direct call through the Application object wherever possible.Comment #8
MixologicComment #9
jhodgdonI just got this fail today on https://www.drupal.org/pift-ci-job/1375636 -- looks like the same messages that are in the issue summary here.
Comment #10
MixologicAs we've seen before, we can run these tests several times in parallel to hope to get a race conflict, but that doesnt guarantee that we hit it.
This definitely removes one of the ways that these tests can collide with each other, so Im moving this to RTBC.
Comment #11
larowlanCommitted b1cf107 and pushed to 8.8.x. Thanks!