Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Our automated tests have implicit dependencies (i.e., not documented or asserted, if even well-known) on apparently arbitrary environment configuration details, leading to tests failing in unpredictable ways depending on where they're run. The result is developers waiting for long test runs that are doomed to fail from the start and then getting cryptic and misleading error messages that don't help them identify or solve the actual problem.
Steps to reproduce
Proposed resolution
We should identify ambient or environment-level preconditions for tests to run and assert their correctness at the beginning of test test runs and exit early with helpful, explicit error messages. (Eventually, we should eliminate as many such dependencies as possible, but let's start here.)
Remaining tasks
- ⛔ Identify as many environment-level test dependencies
- ⛔ Assert their correctness at the beginning of test runs and, if unmet, fail early with clear error messages explaining how to fix the problem
- ⛔ Create a follow-up issue to eliminate as much of the environment-sensitivity as possible
- ⛔ Create a follow-up issue to ensure that there's parity between production requirements and test preconditions. (We don't want tests to require different conditions than production code!)
User interface changes
API changes
Data model changes
Original report
In #3319497-6: Build test failure on local in 8.x-2.x due to drupal core packages being added to vendor.json and #3319497-7: Build test failure on local in 8.x-2.x due to drupal core packages being added to vendor.json, we discovered that \Drupal\Tests\package_manager\Build\TemplateProjectTestBase::createVendorRepository() behaves in an environment-dependent way.
Once AU moves into Drupal core, it's unacceptable for its tests to not work on environments with certain additional packages installed in a certain way.
That'd violate the testing gate.
The patch in #3319497-6: Build test failure on local in 8.x-2.x due to drupal core packages being added to vendor.json provides momentary relief to unblock the people working on AU, but does not provide a systemic solution yet. This issue exists to do precisely that: solve it systemically. Or, at the very least, fail explicitly.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3319679-16-experiment.patch | 3.84 KB | wim leers |
| #6 | 3319679-6-PoC.patch | 1.17 KB | wim leers |
Comments
Comment #2
wim leers#3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly is related, and probably the solution we come up with here (or alternatively the detection logic to fail explicitly) should point to the docs that introduces. Marking this as postponed on that, because it needs to exist first for this to be a comprehensive solution. It's possible we work on both in tandem, but this should not land without #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly being done too.
Comment #3
tedbowunblocked
Comment #4
tedbowComment #5
wim leers@tedbow: so you disagree with what I wrote in #2 — that #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly should happen first?
Comment #6
wim leersHere's a start for how we could harden against this.
But I still haven't figured out what it is about #3319497: Build test failure on local in 8.x-2.x due to drupal core packages being added to vendor.json that it fixed this. So that's why I started adding debug output. Hopefully @tedbow or @Travis.Carden can continue this 🤞
Comment #7
traviscarden commentedUpdating issue title and summary.
Comment #8
wim leers🤩👍
Comment #9
wim leersObservations, not conclusions:
9.4.5(I changed to a different branch which had an additional contrib module installed through composer) and gotas output.
9.5.x(plain 9.5, with just the AU dependencies added), and got:and
composer install! Let's do that:now I get:
Conclusion: it's possible that much of Drupal still works if
vendor/*does not match whatcomposer installwould do. Perhaps we need to do a dry run to verify nothing would happen to verify the environment is in a correct state?Comment #10
wim leersWe use
COMPOSER_MIRROR_PATH_REPOS=1 composer installin our tests.But
setup_local_dev.shuses that too. This seems wrong. That's A) not realistic for real-world scenarios, B) unnecessary avoiding of symlinking.Related: our symlink validation seems unable to distinguish between intra-package symlinks vs inter-package symlinks. The latter are dangerous, the former are not.
Comment #11
tedbow@Wim Leers
re
This is under "Original report" so I am not sure if this still applies
But are you referring to ability to have other composer packages whether they be drupal project or otherwise installed the drupal site you are using for development? If so this will not work and this is similar existing core tests
For instance if you have clone of 9.5.x and you run
This should pass locally
but then if you
composer require drupal/webformRun the same test it will fail. this is because the composer command will change files under
composer/Metapackagebecause in core's composer.json we haveThis because of
For
\Drupal\BuildTests\Composer\Template\ComposerProjectTemplatesTestthe files are undercomposer/Metapackageare basically test fixtures because it needs use them to create a new composer project.Similarly our build tests need to run
composer create-projectfrom the core templates so they are also essentially test fixtures for us. Once we are core they will have same relationship to our tests as existing core test that requirecomposer/MetapackageThis why above you had the similar problem above
as I demonstrated in the core build test with drupal/webform.
In both our build test and the existing composer build test if you
git restore composer/MetapackageThe test will pass again
Comment #12
wim leers#11
The test example you're providing is the one test that specifically tests
composer.json(it's called\Drupal\BuildTests\Composer\Template\ComposerProjectTemplatesTest), so yeah, of course that will fail when additional modules are installed.I was not referring to
composer/Metapackagechanges. In #10, I was referring to the fact that usingCOMPOSER_MIRROR_PATH_REPOS=1(see https://getcomposer.org/doc/03-cli.md#composer-mirror-path-repos) will result in path repos being mirrorred (cp -RI presume?) instead ofsymlinked (the defaultcomposerbehavior). This causesdrupal/core-project-messageanddrupal/core-vendor-hardeningto be mirrored rather than symlinked, which will not be the case on most real-world projects.Crucially, both
setup_local_dev.shand\Drupal\Tests\package_manager\Build\TemplateProjectTestBase::createTestProject()do this. The consequences can be seen easily by doing this:8.x-2.5scripts/setup_local_dev.sh⚠️ not symlinks⚠️
git clone https://git.drupalcode.org/project/drupal.git && cd drupal && composer install⚠️ symlinks⚠️
Now, the only change that #3319497: Build test failure on local in 8.x-2.x due to drupal core packages being added to vendor.json made was adding a special provision for those two packages that in here are seen to be symlinked once but not the other time, the only difference being …
COMPOSER_MIRROR_PATH_REPOS.Conclusions:
COMPOSER_MIRROR_PATH_REPOS=0aka symlinking (which is the default) orCOMPOSER_MIRROR_PATH_REPOS=1aka mirroring (which is whatsetup_local_dev.shimposes)Comment #13
wim leersComment #14
wim leersYay, @TravisCarden's MR at #3321994: Fail early in all Automatic Updates tests if there's a failure marker is introducing foundations for this to build further on — and it's now ready for review from @tedbow! 👍
Comment #15
traviscarden commentedUnassigning myself. I don't really have anything else to add.
Comment #16
wim leersLet's get this moving forward again. Let's start with the
COMPOSER_MIRROR_PATH_REPOS=1stuff mentioned in #12, because that affects #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly and friends too…I am very curious to see what the impact is of removing
COMPOSER_MIRROR_PATH_REPOS=1from our tests altogether, because virtually nobody in the real world uses that. IOW: our test coverage is doing things not representative of the real world. Then again, in the real world the actual composer would be used to install packages, so perhaps it is the best way to simulate actual packages being installed. Still, it'd be valuable to see what would fail 😇Comment #18
wim leersPer #3320792-14: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers), this is blocked on that issue. This will be the finishing touch, but #3320792 is an obvious thing that should happen first.
Also, #16 shows that at the very least we need to document and understand the current reliance on
COMPOSER_MIRROR_PATH_REPOS=1better.Comment #19
wim leers#16 failed. This is due to
SymlinkValidator. The test coverage improvements between #3331168-13: Limit trusted Composer plugins to a known list, allow user to add more and #3331168-15: Limit trusted Composer plugins to a known list, allow user to add more make the failures 💯 times clearer:Comment #20
wim leersActually, given that #3331168: Limit trusted Composer plugins to a known list, allow user to add more is itself blocked on 2 issues, the dependency chain is quite long!
Comment #21
wim leers#16 can only possibly be fixed if and only if #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly gets fixed — that's literally what the test failure output in #19 is showing:
Only once #3319507 is solved can we make properly informed decisions, because the current
SymlinkValidatorforces the use ofCOMPOSER_MIRROR_PATH_REPOS=1, which is not the defaultcomposerbehavior and hence makes 100% of our build tests not representative of the real world.But it's possible that we may need
COMPOSER_MIRROR_PATH_REPOS=1after all, but it should be necessary for reasons realism, not for the current reason, which is solely to work aroundSymlinkValidator, which is whyTemplateProjectBase(used by all of AU/PM'sBuildtests) has this:Bumping to because of that, and postponing on #3319507.
Comment #22
wim leersBecause #3328234: Improve test DX *and* confidence: stop using VFS landed, #3331168: Limit trusted Composer plugins to a known list, allow user to add more is now down from blocked on 2 to only 1 issue. Updating this accordingly.
Comment #23
wim leers#3331168: Limit trusted Composer plugins to a known list, allow user to add more is RTBC, not postponed: decreasing postponedness.
#3320792: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) landed: decreasing postponedness.
The only blockers left are #3331168: Limit trusted Composer plugins to a known list, allow user to add more and #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly.
Comment #24
wim leers#3331168: Limit trusted Composer plugins to a known list, allow user to add more is in — that leaves only #3319507: Add symlink support to Composer Stager 2.0, require that version, and simplify UX & tests accordingly.
Comment #25
tedbowComment #26
wim leersI think thanks to the significant changes that have happened since November:
ComposerUtilityis almost gone, in favor of relying on onlycomposercommands (including inFixtureManipulator)… all that combined means that #3337054: Assert status checks pass as early as possible in kernel and functional tests now effectively will achieve the same as this I think!
But let's wait for that to land and then re-assess this.
Comment #27
wim leersIf @tedbow agrees with #3337054-19: Assert status checks pass as early as possible in kernel and functional tests, I think we can close this too.
Comment #28
tedbow#27 yep we are in much better place as far as testing now. child issues here fixed a lot of the problems
the only remaining issue here I see is the "Needs documentation updates" tag for
COMPOSER_MIRROR_PATH_REPOSuse.Assigning to @phenaproxima to create the follow-up or anybody else wants to they could
Comment #29
wim leersI'd be fine with just repurposing this issue to avoid overhead.
Comment #30
xjmComment #31
tedbowCreated follow-up #3387379: Document why use COMPOSER_MIRROR_PATH_REPOS in build tests, So closing. Thanks for all the work on this issue, and the child issues