Repository navigation
test: read the embed provider list even when parallel workers inherit it - #272
Closed
GTS-MEGHNI wants to merge 1 commit into
Closed
GTS-MEGHNI wants to merge 1 commit into
GTS-MEGHNI wants to merge 1 commit into
Conversation
CI copies .env.example, which sets EMBEDS_PROVIDERS to an empty string, and runs the tests in parallel. The workers inherit that value from the parent process, and Laravel's env repository never overwrites a variable that is already set, so the test's own value was ignored and the provider list came back empty. The test now sets the variable through a repository that can overwrite it, and puts the original value back afterwards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Appreciate it! We were late merging though and it has already been fixed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
productionCI fails on one test, so every open PR shows a redtestscheck:The test passes when you run it alone, which is why it's easy to miss.
Cause
It only fails when both of these are true, which is exactly what CI does:
.envdefinesEMBEDS_PROVIDERS=. CI runscp .env.example .env, and.env.examplehas that line.pest --parallel). The main process loads.envinto the real environment, and every worker inheritsEMBEDS_PROVIDERS="".Laravel's env repository never overwrites a variable that is already set. So
Env::getRepository()->set('EMBEDS_PROVIDERS', ' youtube, ,spotify ')did nothing in a worker, andconfig/embeds.phpread the empty value.Fix
Only the test changes. It now uses a repository that is allowed to overwrite the variable (
RepositoryBuilder::createWithDefaultAdapters()), and puts the original value back in afinallyblock, so other tests in the same worker see what they saw before.config/embeds.phpand the app don't change.Testing
Ran
tests/Unit/Services/EmbedsTest.phplocally in three setups:.env.env.example(like CI)--parallel.env.example(like CI)EMBEDS_PROVIDERSline--parallelPint, Rector and type coverage (100%) pass.
Note
Several open PRs carry their own copy of a fix for this test inside unrelated changes (for example #125, #126, #141, #155, #172, #198, #236, #260, #265). Merging this one on its own unblocks all of them, and they can drop their copies.
🤖 Generated with Claude Code