Skip to content

Commit 8dc5de3

Browse files
authored
chore: fix make test-json hang under Swift 6.3.1 + restore CI test execution (#172)
* ci: switch test target filter to underscore form for Swift 6.3.1 Verified empirically: the prior dash form `Container-Compose-StaticTests` matches **zero tests** under Swift 6.3.1 (which normalizes module names dashes -> underscores in the test filter grammar). The 5 most recent CI runs on main / phase-4c branches all silently reported: warning: No matching test cases were run Executed 0 tests, with 0 failures Test run with 0 tests in 0 suites passed after 0.001 seconds conclusion: success Both matrix targets (Static + Dynamic) were affected. Switching to the underscore form `Container_Compose_StaticTests` / `Container_Compose_DynamicTests` makes `swift test --filter` actually match the intended targets. PR 170 + CHAOS-1507 follow-up surfaced this. * chore(makefile): switch test-json to working --filter pattern (drop --experimental-event-stream-output) Under Swift 6.3.1 the prior `make test-json` hangs indefinitely: * `--experimental-event-stream-output .build/test-events.jsonl` opens a socket-backed event stream that errors out without an active reader, deadlocking the whole `swift test` invocation. Tail of `.build/test-events.jsonl` shows the socket errors when this hits. * `--skip Container-Compose-DynamicTests` is non-functional in 6.3.1 — the dash form matches no targets (same root cause as the CI workflow fix in the previous commit). New target uses positive `--filter Container_Compose_StaticTests --no-parallel`: * Underscore form actually selects the static target (verified locally — dash form returns 'No matching test cases were run'). * `--no-parallel` is explicit (not just defaulted) because multiple suites `dup2` global STDOUT to a Pipe to capture warnings (ResourceArgsTests, LifecycleArgsTests, GpusBlkioTests, SecurityArgsTests, NetworkArgsTests, ComposePort runtime-argv, ComposeDown orphan-volume recovery, ComposeUp block-image migration guard, ShutdownWatchdog) and race on the shared FD under `--parallel`. * JSONL + `swift run test-report` pipeline is removed — plain swift-testing stdout is the canonical agent-readable surface again. * docs(agents): update test invocation guidance for Swift 6.3.1 Reflect the fixes from the two preceding commits: * `make test-json` now uses `swift test --filter Container_Compose_StaticTests --no-parallel` (underscore form because Swift 6.3.1 normalizes module-name dashes; `--no-parallel` because multiple static suites `dup2` STDOUT under parallel and race on the shared FD). * JSONL + `swift run test-report` pipeline removed — plain swift-testing stdout is the canonical agent-readable surface again. Exit code is the plain `swift test` exit code. * Expanded list of dup2-using suites (ResourceArgsTests, LifecycleArgsTests, GpusBlkioTests, SecurityArgsTests, NetworkArgsTests, ComposePort runtime-argv, ComposeDown orphan-volume recovery, ComposeUp block-image migration guard, ShutdownWatchdog) is documented so contributors know not to re-introduce `--parallel` to the local test target. Cross-references PR #170 + CHAOS-1507 follow-up where this was empirically verified. * fix(tests): reset warn-once dedup in SecurityFeatureIntegrationTests `SecurityArgsTests` runs alphabetically before `SecurityFeatureIntegrationTests` and exercises `service.privileged`, `service.security_opt`, and `service.group_add` via `SecurityArgs.build()`, consuming the warnings in the process-global `warnUnsupportedRuntimeFieldOnce` dedup set. When the static suite runs serially via `make test-json` / `swift test --no-parallel`, `SecurityFeatureIntegrationTests` then asserts that `captureStdout(...).contains("Note: ...")` succeeds — but the dedup gate suppresses the second emission, so captured stdout is empty and three tests fail. Mirror the pattern from `LifecycleArgsTests.init`: reset the warn-once set in `init()`. The suite is already `.serialized` so the reset runs deterministically before each test. Surfaced by `--filter Container_Compose_StaticTests` (the underscore form Swift 6.3.1 actually matches). The legacy dashed filter on main matched zero tests and silently passed. * fix(runtime): honor absolute paths + guard empty argv in ProductionRunner.resolveBinary `resolveBinary(forArgv:)` previously routed everything-not-`"container-compose"` to the `container` binary unconditionally. The `LineBufferTests` PLAN.md §4 regression guard (`endToEndChunkedStdoutProducesOneLineAtATime`) drives `ProductionRunner` against `/bin/sh -c "..."` and was silently spawning `container -c "..."` instead, which exits 64 with `Error: unknown option '-c'` on any host where apple/container is installed. Two changes, both no-ops for production callsites that always pass an unqualified program name: 1. argv[0] starting with `/` now short-circuits to `URL(fileURLWithPath: first)` and bypasses the cached resolver. Production never fires this branch; it exists so test code can drive a real `/bin/sh` subprocess through `ProductionRunner` without being silently re-routed. 2. Empty argv now throws `RuntimeError.backendFailure` via `guard let` instead of silently falling through to `containerBin` (which would launch the apple/container CLI with no arguments and print its help banner — a confusing diagnostic for what is really a caller bug). Surfaced together with the SecurityFeatureIntegrationTests failures once `--filter Container_Compose_StaticTests` actually executed the suite under Swift 6.3.1. * chore(ci): fail loudly when test filter matches < 1500 tests CI's `swift test --filter Container-Compose-StaticTests` was a silent no-op: Swift 6.3.1 normalizes the dashed target name to underscores in the filter grammar, so the dashed form matched zero tests, swift test exited 0, and the job reported green. Earlier commits on this branch correct the filter to `Container_Compose_StaticTests`. Add a min-tests assertion as defense-in-depth so a future filter typo, target rename, or Swift Testing grammar shift cannot silently regress the suite to a zero-match again: - `.github/workflows/tests.yml`: matrix entries gain a `min_tests` field (1500 for Static, 0 for Dynamic — CI runners do not ship apple/container so Dynamic legitimately reports `0 tests`). The run step pipes through `tee`, parses the Swift Testing summary line for the executed count, and fails with `::error::` when count is below the floor. - `Makefile`: `test-json` gets the same guard inline. Honors the upstream swift-test exit code; only overrides on the filter-matched-too-few condition. - `AGENTS.md`: documents the guard alongside the existing `--parallel` caveat and the underscore-filter rationale.
1 parent 3efd36f commit 8dc5de3

5 files changed

Lines changed: 121 additions & 30 deletions

File tree

.github/workflows/tests.yml

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,23 @@ jobs:
5353
matrix:
5454
include:
5555
- suite: Static
56-
target: Container-Compose-StaticTests
56+
target: Container_Compose_StaticTests
5757
extra_args: "--parallel"
58+
# Silent-skip guard (CHAOS-1507 follow-up). Swift 6.3.1 normalizes
59+
# target-name dashes to underscores in the filter grammar; a typo or
60+
# regression to the dashed form `Container-Compose-StaticTests`
61+
# silently matches zero tests and exits 0. Fail loudly if fewer
62+
# than `min_tests` ran. Floor chosen well below the current 1806
63+
# so additions/removals don't trigger noise.
64+
min_tests: 1500
5865
- suite: Dynamic
59-
target: Container-Compose-DynamicTests
66+
target: Container_Compose_DynamicTests
6067
extra_args: ""
68+
# CI runners do not ship apple/container, so every dynamic test
69+
# self-skips via `.enabled(if: RuntimeAvailability.isAvailable())`.
70+
# Swift Testing reports the run as `0 tests`. No floor is
71+
# enforceable here; we still build the target to catch compile drift.
72+
min_tests: 0
6173

6274
steps:
6375
- name: Checkout code
@@ -88,7 +100,22 @@ jobs:
88100
run: swift build --build-tests
89101

90102
- name: Run ${{ matrix.suite }} tests
91-
run: swift test --filter "${{ matrix.target }}" ${{ matrix.extra_args }}
103+
shell: bash
104+
run: |
105+
set -o pipefail
106+
log_file="test-output-${{ matrix.suite }}.log"
107+
swift test --filter "${{ matrix.target }}" ${{ matrix.extra_args }} 2>&1 | tee "$log_file"
108+
test_exit=${PIPESTATUS[0]}
109+
# Swift Testing prints e.g. `Test run with 1806 tests in 151 suites passed`.
110+
# XCTest prints e.g. `Executed N tests`. Capture either form.
111+
count=$(grep -oE 'Test run with [0-9]+ tests|Executed [0-9]+ tests' "$log_file" | grep -oE '[0-9]+' | tail -1 || true)
112+
count=${count:-0}
113+
echo "::notice::Suite ${{ matrix.suite }} executed $count tests (min ${{ matrix.min_tests }})"
114+
if [ "$count" -lt "${{ matrix.min_tests }}" ]; then
115+
echo "::error::Filter '${{ matrix.target }}' executed $count tests (expected >= ${{ matrix.min_tests }}). Did the Swift Testing target-name normalization change, or did the filter typo into a zero-match form?"
116+
exit 2
117+
fi
118+
exit $test_exit
92119
93120
- name: Upload test artifacts on failure
94121
if: failure()

AGENTS.md

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -393,16 +393,28 @@ schema changes that ripple into `up`/`down`/`build`.
393393
Prefer `make test-json` over raw `swift test` when an agent (or any
394394
automation) is parsing results. From the [Makefile](./Makefile):
395395

396-
- Routes the static suite through swift-testing's event stream and emits
397-
structured JSON via `swift run test-report` — agent-parseable, no
398-
`tail -N` truncation games.
399-
- Skips `Container-Compose-DynamicTests` automatically (equivalent to
400-
`--filter Container-Compose-StaticTests` but with the right tooling).
401-
- Forbids `--parallel`. Several static suites (`LifecycleArgsTests`,
402-
`ResourceArgsTests`, `GpusBlkioTests`) `dup2` global `STDOUT_FILENO`
403-
to a `Pipe` to capture printed warnings; under `--parallel` they race
404-
on the shared FD and `readDataToEndOfFile()` hangs indefinitely.
405-
- Exit codes: `0` = all passed, `1` = any failed, `2` = no events emitted.
396+
- Runs the static suite via `swift test --filter Container_Compose_StaticTests`
397+
(underscore form — Swift 6.3.1 normalizes the dashed target name and the
398+
legacy `Container-Compose-StaticTests` filter matches **zero tests**).
399+
See PR #170 + the CHAOS-1507 follow-up.
400+
- The previous JSONL + `swift run test-report` pipeline was removed because
401+
`--experimental-event-stream-output` now hangs indefinitely under Swift
402+
6.3.1 when no socket reader is attached. Plain swift-testing stdout is the
403+
canonical agent-readable surface again.
404+
- Forbids `--parallel`. Multiple suites `dup2` global `STDOUT_FILENO` to a
405+
`Pipe` to capture printed warnings: `LifecycleArgsTests`, `ResourceArgsTests`,
406+
`GpusBlkioTests`, `SecurityArgsTests`, `NetworkArgsTests`, the ComposePort
407+
runtime-argv tests, ComposeDown orphan-volume recovery, the ComposeUp
408+
block-image migration guard, and `ShutdownWatchdog`. Under `--parallel` they
409+
race on the shared FD and `readDataToEndOfFile()` hangs indefinitely — do
410+
not re-introduce `--parallel` to this target.
411+
- Exit code is the plain `swift test` exit code (0 = pass, non-zero = fail).
412+
- **Silent-skip guard.** Both `make test-json` and the CI workflow assert that
413+
the static suite executes **>= 1500 tests** (current count: ~1806). If the
414+
filter typo'd back to a zero-match form (e.g. the dashed `Container-Compose-StaticTests`)
415+
the guard prints an `ERROR: filter executed only N tests` message and exits
416+
2 — defending against the same footgun that caused CI to be a silent no-op
417+
between the Swift 6.3.1 upgrade and the CHAOS-1507 follow-up.
406418

407419
Previously, `VolumeMountIntegrationTests` could leak real Docker Hub pulls when its environment wraps missed `RunnerEnvironment.$current`. Fixed by wrapping `RecordingRunner()` in every test. All test `projectName` values are now prefixed `cc-test-` (e.g. `cc-test-vol-up-<uuid>`) to make test-originated container names unambiguous. When sweeping this convention to other static-suite test files, use the same `cc-test-` prefix. Leaving this note as a defensive reminder.
408420

@@ -442,8 +454,9 @@ the broad `--no-parallel` workaround from CHAOS-1314.
442454
- **Don't break the topo sort.** `Service.topoSortConfiguredServices` is on
443455
the hot path of `up`. Cycle detection there must keep throwing.
444456
- **Commits.** Small, focused commits. Rebuild with `make build` and run
445-
`make test-json` (or full `swift test` if your change touches dynamics)
446-
before pushing.
457+
`make test-json` (which is `swift test --filter Container_Compose_StaticTests`
458+
under Swift 6.3.1; see PR #170 / CHAOS-1507) or full `swift test` if your
459+
change touches dynamics before pushing.
447460
- **Background agents for long-running tasks.** Always dispatch test runs
448461
(prefer `make test-json` for structured, agent-parseable output; raw
449462
`swift test` only when you need the full run including
@@ -503,9 +516,11 @@ make build # release config, copies binary to .build/release/
503516
make install # symlinks into /usr/local/bin (sudo may be required)
504517

505518
# Run tests
506-
make test-json # static suite, structured JSON (preferred for agents)
519+
make test-json # static suite only (Swift 6.3.1 underscore filter; preferred for agents)
507520
swift test # full run including Container-Compose-DynamicTests
508521
swift test --filter LifecycleArgsTests # single suite (replace with target)
522+
# Equivalent to `make test-json` if you need to invoke directly:
523+
swift test --filter Container_Compose_StaticTests # underscore form — dashes match 0 tests under Swift 6.3.1 (see PR #170)
509524

510525
# Use locally
511526
.build/release/container-compose up -f path/to/docker-compose.yml

Makefile

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,38 @@ build-tests:
7272
test:
7373
swift test
7474

75-
# Run the static suite under swift-testing's event stream and emit a structured
76-
# report. Useful for agent-driven test reading or CI parsing. Defaults to
77-
# fast-feedback path: skipping the dynamic Apple-container target (use
78-
# `make test` for the full run including dynamics).
75+
# Run the static test suite only — fast feedback path for agents and CI-equivalent
76+
# local checks. Dynamic tests are gated by RuntimeAvailability so this filter is
77+
# the canonical way to skip them on hosts without apple/container.
7978
#
80-
# NOTE: --parallel intentionally NOT used. The static suite contains tests
81-
# (ResourceArgsTests, LifecycleArgsTests, GpusBlkioTests) that `dup2` global
82-
# STDOUT_FILENO to a Pipe to capture printed warnings; under --parallel they
83-
# race on the shared FD and `readDataToEndOfFile()` hangs indefinitely.
79+
# CHAOS-1507 follow-up ("make test-json hang"): under Swift 6.3.1 the prior
80+
# implementation used `--experimental-event-stream-output` to feed a JSONL file
81+
# into `swift run test-report`. That socket-backed event stream now hangs
82+
# indefinitely when no reader attaches in time, and `--skip Container-Compose-DynamicTests`
83+
# silently matches nothing because Swift 6.3.1 normalizes target dashes to
84+
# underscores in the test filter grammar. Both have been removed; we use the
85+
# underscore form `--filter Container_Compose_StaticTests` instead.
8486
#
85-
# Exit code mirrors the report: 0 = all passed, 1 = any failed, 2 = no events.
87+
# NOTE: --parallel intentionally NOT used. Multiple suites `dup2` global
88+
# STDOUT_FILENO to a Pipe to capture printed warnings (ResourceArgsTests,
89+
# LifecycleArgsTests, GpusBlkioTests, SecurityArgsTests, NetworkArgsTests,
90+
# ComposePort runtime-argv tests, ComposeDown orphan-volume recovery,
91+
# ComposeUp block-image migration guard, ShutdownWatchdog). Under --parallel
92+
# they race on the shared FD and `readDataToEndOfFile()` hangs indefinitely.
8693
test-json:
87-
rm -f .build/test-events.jsonl
88-
-swift test --skip Container-Compose-DynamicTests --experimental-event-stream-output .build/test-events.jsonl
89-
swift run test-report .build/test-events.jsonl --format json
94+
@set -o pipefail; \
95+
mkdir -p .build; \
96+
log_file=.build/test-json.log; \
97+
swift test --filter Container_Compose_StaticTests --no-parallel 2>&1 | tee "$$log_file"; \
98+
test_exit=$$?; \
99+
count=$$(grep -oE 'Test run with [0-9]+ tests|Executed [0-9]+ tests' "$$log_file" | grep -oE '[0-9]+' | tail -1 || true); \
100+
count=$${count:-0}; \
101+
echo "test-json: executed $$count tests"; \
102+
if [ "$$count" -lt 1500 ]; then \
103+
echo "ERROR: filter executed only $$count tests (expected >= 1500). Did the Swift Testing target-name normalization change?"; \
104+
exit 2; \
105+
fi; \
106+
exit $$test_exit
90107

91108
# Regenerate coverage.json from the inline JSON in coverage.html.
92109
coverage:
@@ -108,7 +125,7 @@ help:
108125
@echo " debug Debug build of $(binary_name)"
109126
@echo " build-tests Compile tests without running (CI-equivalent)"
110127
@echo " test Run all tests (dynamic ones self-skip without Apple container)"
111-
@echo " test-json Run static tests + emit structured JSON report (skips dynamic; for agents/CI)"
128+
@echo " test-json Run static suite only (Swift 6.3.1 underscore filter; CI-equivalent agent-friendly target)"
112129
@echo " coverage Regenerate coverage.json from coverage.html"
113130
@echo " clean Remove .build/"
114131
@echo " install Build + install to \$$(bindir) (default $(bindir))"

Sources/Container-Compose/Runtime/RunCommandRunner.swift

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,29 @@ public struct ProductionRunner: RunCommandRunner {
203203

204204
/// Pick the right cached binary for an argv whose first element is the
205205
/// program name (`"container"` or `"container-compose"`).
206+
///
207+
/// Absolute paths (e.g. `"/bin/sh"`) are honored as-is and bypass the cache.
208+
/// Production callsites always pass an unqualified program name, so this
209+
/// branch is a no-op in production; it exists so test code can drive
210+
/// `ProductionRunner` against a real `/bin/sh` subprocess without being
211+
/// silently re-routed to the apple/container CLI.
212+
///
213+
/// Empty argv is a programmer error: every `RunRequest` must carry at
214+
/// least one element. Fail loudly via `RuntimeError.backendFailure`
215+
/// rather than silently routing an empty-args invocation to the
216+
/// `container` binary (which would launch the CLI with no arguments and
217+
/// print its help banner — a confusing diagnostic for what is really a
218+
/// caller bug).
206219
fileprivate static func resolveBinary(forArgv argv: [String]) throws -> URL {
207-
let cached: Result<URL, RuntimeError> = (argv.first == "container-compose") ? selfBin : containerBin
220+
guard let first = argv.first else {
221+
throw RuntimeError.backendFailure(
222+
message: "ProductionRunner.resolveBinary: argv is empty"
223+
)
224+
}
225+
if first.hasPrefix("/") {
226+
return URL(fileURLWithPath: first)
227+
}
228+
let cached: Result<URL, RuntimeError> = (first == "container-compose") ? selfBin : containerBin
208229
return try cached.get()
209230
}
210231

Tests/Container-Compose-StaticTests/SecurityFeatureIntegrationTests.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,17 @@ import Yams
6060
@Suite("Security feature integration tests — YAML → SecurityArgs.build() → argv", .serialized)
6161
struct SecurityFeatureIntegrationTests {
6262

63+
/// Reset the process-wide warn-once dedup set before each test so the
64+
/// `captureStdout(...).contains("Note: ...")` assertions don't flake based
65+
/// on which sibling suite ran first. Peer suite `SecurityArgsTests` also
66+
/// exercises `service.privileged`, `service.security_opt`, and
67+
/// `service.group_add` via `SecurityArgs.build()`, which would otherwise
68+
/// consume the one-shot warning under serial execution. Mirrors the
69+
/// pattern established in `LifecycleArgsTests.init`.
70+
init() {
71+
resetUnsupportedRuntimeFieldWarningsForTesting()
72+
}
73+
6374
// MARK: - Helpers
6475

6576
/// Decode a `DockerCompose` from YAML and return the named service.

0 commit comments

Comments
 (0)