Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 3 additions & 0 deletions pkg/e2e/testdata/TestUpInteractiveDryRun/compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
services:
app:
image: alpine
17 changes: 17 additions & 0 deletions pkg/e2e/testdata/TestUpInteractiveHookLogs/compose.yaml
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:
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
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}
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 pkg/e2e/testdata/TestUpInteractiveWaitsForHealthy/compose.yaml
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:
105 changes: 105 additions & 0 deletions pkg/e2e/up_interactive_test.go
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"))

Copy link
Copy Markdown
Contributor

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 without Eventually after --abort-on-container-exit

This 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-exit fires, compose sends SIGTERM to the remaining containers and then returns. The long container (running sleep infinity) may still be transitioning from running → exited at the exact moment the scenario framework checks its state. The bare ServiceState("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 the Eventually was kept precisely as a defensive guard against scheduler-dependent timing on CI runners. Removing it reintroduces the intermittent failure.

Suggested fix: restore the Eventually wrapper:

Suggested change
ServiceState("long", "exited"))
Eventually(ServiceState("long", "exited"), 10*time.Second))
Confidence Score
🟡 moderate 57/100

Copy link
Copy Markdown
Contributor Author

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 ServiceState here 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-exit runs stopApplication, which schedules u.stop(...) on the session's errgroup (pkg/compose/up.go, stopApplication), and ContainerStop only returns once the container has exited. up returns after u.eg.Wait(), so by the time the scenario takes its snapshot long is already exited: there is no window for a daemon-side transition to still be pending. An Eventually would 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 !windows from the build tag: scenario.go and checks.go carry no build constraint, 49 e2e files use the plain e2e tag (including the watch, wait, volumes and secrets ones, which use the same sh -c and sleep fixtures), and the e2e CI matrix has no Windows runner, so the tag change doesn't enable anything new there.

}

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"))
Comment thread
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"))
}
Loading