Repository navigation
test(e2e): lock the behavior of the foreground up #14300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+180
−0
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
88a83c7
test(e2e): lock the behavior of the foreground up
ndeloof 0e34c01
test(e2e): stop asserting pre_start output the foreground up never st…
ndeloof 4e8a4d0
test(e2e): harden the foreground up scenarios against review findings
ndeloof 7061d16
test(e2e): lock pre_start's streamed output and tighten the foregroun…
ndeloof File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
14 changes: 14 additions & 0 deletions
14
pkg/e2e/testdata/TestUpInteractiveDependencyTimeout/compose.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| services: | ||
| db: | ||
| image: alpine | ||
| command: sleep infinity | ||
| healthcheck: | ||
| test: ["CMD", "false"] | ||
| interval: 1s | ||
| retries: 100 | ||
| web: | ||
| image: alpine | ||
| command: echo web-started | ||
| depends_on: | ||
| db: | ||
| condition: service_healthy |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| services: | ||
| app: | ||
| image: alpine |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| services: | ||
| app: | ||
| image: alpine | ||
| # the sleep keeps the container running while post_start execs into it and | ||
| # the session streams its output | ||
| command: sh -c 'echo "app-read:$(cat /shared/init.txt)"; sleep 5' | ||
| volumes: | ||
| - data:/shared | ||
| pre_start: | ||
| - image: alpine | ||
| # prints a marker the service never logs, so its presence in the | ||
| # session can only come from the hook's own output stream | ||
| command: sh -c 'echo pre-start-hook-stdout; echo "pre-start-file-content" > /shared/init.txt' | ||
| post_start: | ||
| - command: echo post-start-hook-ran | ||
| volumes: | ||
| data: |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| services: | ||
| quick: | ||
| image: alpine | ||
| command: echo hi | ||
| long: | ||
| image: alpine | ||
| command: sleep infinity | ||
| init: true |
7 changes: 7 additions & 0 deletions
7
pkg/e2e/testdata/TestUpInteractiveRecreateStreamsNewContainer/compose.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| services: | ||
| app: | ||
| image: alpine | ||
| # the sleep keeps the container running while the session attaches to it | ||
| command: sh -c 'echo "value=$VALUE"; sleep 2' | ||
| environment: | ||
| VALUE: ${VALUE:-one} |
4 changes: 4 additions & 0 deletions
4
pkg/e2e/testdata/TestUpInteractiveRestartsExitedContainer/compose.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| services: | ||
| app: | ||
| image: alpine | ||
| command: sh -c 'if [ -f /tmp/ran ]; then echo second-run; else touch /tmp/ran; fi' |
22 changes: 22 additions & 0 deletions
22
pkg/e2e/testdata/TestUpInteractiveWaitsForHealthy/compose.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| services: | ||
| db: | ||
| image: alpine | ||
| command: sh -c 'sleep 3; touch /shared/ready; sleep 60' | ||
| init: true | ||
| volumes: | ||
| - shared:/shared | ||
| healthcheck: | ||
| test: ["CMD", "test", "-f", "/shared/ready"] | ||
| interval: 1s | ||
| timeout: 1s | ||
| retries: 30 | ||
| web: | ||
| image: alpine | ||
| command: sh -c 'if [ -f /shared/ready ]; then echo web-saw-ready; else echo web-too-early; fi' | ||
| volumes: | ||
| - shared:/shared | ||
| depends_on: | ||
| db: | ||
| condition: service_healthy | ||
| volumes: | ||
| shared: |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| //go:build e2e | ||
|
|
||
| /* | ||
| Copyright 2026 Docker Compose CLI authors | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package e2e | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| // Scenarios locking the observable behavior of the foreground (non-detached) | ||
| // `compose up`. Most e2e coverage of the start phase runs detached. | ||
|
|
||
| func TestUpInteractiveHookLogs(t *testing.T) { | ||
| NewScenario(t, "a foreground up must run the hooks around the service and stream their output into its session"). | ||
| Step("up shows both hooks' output, and the service reads what pre_start wrote before it started", | ||
| ComposeCmd("up", "--menu=false").Within(120*time.Second), | ||
| StdoutContains("pre-start-hook-stdout"), | ||
| StdoutContains("app-read:pre-start-file-content"), | ||
| StdoutContains("post-start-hook-ran"), | ||
| ServiceState("app", "exited")) | ||
| } | ||
|
|
||
| func TestUpInteractiveImmediateExit(t *testing.T) { | ||
| NewScenario(t, "a foreground up --abort-on-container-exit must notice a container that exits as soon as it starts"). | ||
| Step("up aborts on the instantly exiting container instead of waiting for the long-running one", | ||
| ComposeCmd("up", "--menu=false", "--abort-on-container-exit").Within(60*time.Second), | ||
| OutputContains("quick-1 exited with code 0"), | ||
| ServiceState("quick", "exited"), | ||
| ServiceState("long", "exited")) | ||
| } | ||
|
|
||
| func TestUpInteractiveRestartsExitedContainer(t *testing.T) { | ||
| NewScenario(t, "a foreground up must start a container that exited earlier, reusing it"). | ||
| Step("up -d leaves the service exited after its first run", | ||
| ComposeCmd("up", "-d"), | ||
| Eventually(ServiceState("app", "exited"), 15*time.Second)). | ||
| Step("a foreground up runs the same container a second time", | ||
| ComposeCmd("up", "--menu=false").Within(60*time.Second), | ||
| NotRecreated("app"), | ||
| StdoutContains("second-run"), | ||
| ServiceState("app", "exited")) | ||
| } | ||
|
|
||
| func TestUpInteractiveWaitsForHealthy(t *testing.T) { | ||
| NewScenario(t, "a foreground up must not start a service before its service_healthy dependency is healthy"). | ||
| Step("up starts web only once db reports healthy", | ||
| ComposeCmd("up", "--menu=false", "--abort-on-container-exit").Within(120*time.Second), | ||
| StdoutContains("web-saw-ready"), | ||
| OutputNotContains("web-too-early"), | ||
| ServiceState("web", "exited")) | ||
| } | ||
|
|
||
| func TestUpInteractiveRecreateStreamsNewContainer(t *testing.T) { | ||
| NewScenario(t, "a foreground up after a config change must recreate the service and stream the new container's output"). | ||
| Step("up -d creates the service with its first configuration", | ||
| ComposeCmd("up", "-d"), | ||
| Eventually(ServiceState("app", "exited"), 15*time.Second)). | ||
| Step("a foreground up with another value recreates it and shows the new output", | ||
| ComposeCmd("up", "--menu=false").WithEnv("VALUE=two").Within(60*time.Second), | ||
| Recreated("app"), | ||
| ContainerEnv("app", "VALUE", "two"), | ||
| StdoutContains("value=two"), | ||
| OutputNotContains("value=one")) | ||
| } | ||
|
|
||
| func TestUpInteractiveDependencyTimeout(t *testing.T) { | ||
| NewScenario(t, "a foreground up with --wait-timeout must fail once a dependency stays unhealthy, without starting the dependent"). | ||
| Step("up fails on the dependency timeout; web exists but was never started", | ||
| ComposeCmd("up", "--menu=false", "--wait-timeout", "3").MayFail().Within(60*time.Second), | ||
| ExitCode(1), | ||
| OutputContains("timeout waiting for dependencies"), | ||
| OutputNotContains("deadline exceeded"), | ||
| OutputNotContains("web-started"), | ||
| ServiceState("db", "running"), | ||
| // web's container is created in the create phase, before the start | ||
| // phase blocks on db's health: "created" (not "no container") is | ||
| // what says it was prepared but never started. | ||
| ServiceState("web", "created")) | ||
|
ndeloof marked this conversation as resolved.
|
||
| } | ||
|
|
||
| func TestUpInteractiveDryRun(t *testing.T) { | ||
| // the progress lines name the container after the project | ||
| s := NewScenario(t, "a foreground up --dry-run must report the container creation, never start it and leave nothing behind") | ||
| s.Step("up --dry-run reports the creation, no start, and leaves no container behind", | ||
| ComposeCmd("up", "--menu=false", "--dry-run").Within(60*time.Second), | ||
| OutputContains("Container "+s.Project()+"-app-1 Created"), | ||
| OutputNotContains(s.Project()+"-app-1 Start"), | ||
| ServiceNotCreated("app")) | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[medium]
ServiceState("long", "exited")is racy withoutEventuallyafter--abort-on-container-exitThis diff removes the
Eventually(ServiceState("long", "exited"), 10*time.Second)guard that was deliberately added in the previous review cycle to absorb a documented race condition.The race: when
--abort-on-container-exitfires, compose sends SIGTERM to the remaining containers and then returns. Thelongcontainer (runningsleep infinity) may still be transitioning fromrunning→exitedat the exact moment the scenario framework checks its state. The bareServiceState("long", "exited")snapshots state synchronously with no retry, so it will fail spuriously whenever the daemon hasn't recorded the final state yet.The prior review thread (resolved by ndeloof in
76664d550) noted compose appeared to wait for the stop to complete before returning across 15 local runs — but theEventuallywas kept precisely as a defensive guard against scheduler-dependent timing on CI runners. Removing it reintroduces the intermittent failure.Suggested fix: restore the
Eventuallywrapper:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I'm keeping the bare
ServiceStatehere on purpose, and I'm leaving this thread open for you to confirm rather than resolving it.The premise doesn't hold on this path:
--abort-on-container-exitrunsstopApplication, which schedulesu.stop(...)on the session's errgroup (pkg/compose/up.go,stopApplication), andContainerStoponly returns once the container has exited.upreturns afteru.eg.Wait(), so by the time the scenario takes its snapshotlongis alreadyexited: there is no window for a daemon-side transition to still be pending. AnEventuallywould suggest an asynchrony that doesn't exist and could hide a real regression (a start phase that returns before the application is stopped).This is backed by runs rather than only by reading: the six e2e jobs are green on this head, and the scenarios passed on repeated local runs. If this ever flakes in CI I'll restore the retry together with the evidence of what is asynchronous.
On the low-confidence note about dropping
!windowsfrom the build tag:scenario.goandchecks.gocarry no build constraint, 49 e2e files use the plaine2etag (including the watch, wait, volumes and secrets ones, which use the samesh -candsleepfixtures), and the e2e CI matrix has no Windows runner, so the tag change doesn't enable anything new there.