Skip to content

Fail CI on xcodebuild hard failures #4523

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.

Sign up for GitHub

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

Closed
lawrencecchen wants to merge 19 commits into from
Closed

Fail CI on xcodebuild hard failures #4523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
41ae7a4
test: cover CI unit-test hard failure guard
lawrencecchen May 22, 2026
7307196
fix: fail CI on xcodebuild hard failures
lawrencecchen May 22, 2026
1c8aa6c
fix: print depot unit-test output before guard
lawrencecchen May 22, 2026
4536cb9
test: tighten CI hard failure guard coverage
lawrencecchen May 22, 2026
7526947
ci: quarantine crashing notification jump tests
lawrencecchen May 22, 2026
c4efce2
ci: allow unit suite to complete before watchdog
lawrencecchen May 22, 2026
7198d4d
test: keep CI hard failure guard test behavioral
lawrencecchen May 22, 2026
2e6337b
ci: document hard-failure quarantines
lawrencecchen May 22, 2026
45283a0
test: harden app-host CI teardown
lawrencecchen May 22, 2026
4cf6fd3
fix: quarantine stale terminal inheritance surfaces
lawrencecchen May 22, 2026
b290fa9
fix: quarantine stale terminal surface refreshes
lawrencecchen May 22, 2026
2632c10
fix: keep ghostty pointer access actor-scoped
lawrencecchen May 22, 2026
dbee03e
fix: keep notification socket tests windowless
lawrencecchen May 22, 2026
188cb69
fix: release stale surface test fixtures
lawrencecchen May 22, 2026
df70495
fix: register notification open test context
lawrencecchen May 22, 2026
901f4ab
fix: close retained markdown test windows
lawrencecchen May 22, 2026
55207d5
Merge remote-tracking branch 'origin/main' into task-ci-timeout-crash…
lawrencecchen May 22, 2026
d8ee907
fix: parse CRLF JSONC line comments
lawrencecchen May 22, 2026
e946810
fix: mark socket-backed UI test launch
lawrencecchen May 22, 2026
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ jobs:
- name: Validate unit-test SwiftPM retry guard
run: ./tests/test_ci_unit_test_spm_retry.sh

- name: Validate unit-test hard failure guard
run: ./tests/test_ci_unit_test_output_guard.sh

- name: Validate cmux scheme test configuration
run: ./tests/test_ci_scheme_testaction_debug.sh

Expand Down
83 changes: 83 additions & 0 deletions tests/test_ci_unit_test_output_guard.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#!/usr/bin/env bash
# Regression test for CI unit-test hard-failure detection.
set -euo pipefail

ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)"
SCRIPT="$ROOT_DIR/scripts/ci-unit-test-output-guard.sh"
WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml"
TMP_DIR="$(mktemp -d)"
trap 'rm -rf "$TMP_DIR"' EXIT

if [ ! -x "$SCRIPT" ]; then
echo "FAIL: missing executable hard-failure guard at $SCRIPT" >&2
exit 1
fi

if ! grep -Fq './tests/test_ci_unit_test_output_guard.sh' "$WORKFLOW_FILE"; then
echo "FAIL: ci.yml must run the unit-test hard-failure guard test" >&2
exit 1
fi

if ! grep -Fq './scripts/ci-unit-test-output-guard.sh "$EXIT_CODE" /tmp/test-output.txt' "$WORKFLOW_FILE"; then
echo "FAIL: ci.yml must run the hard-failure guard before parsing XCTest summaries" >&2
exit 1
fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

write_log() {
local name="$1"
shift
printf '%s\n' "$@" > "$TMP_DIR/$name.log"
}

expect_pass() {
local exit_code="$1"
local log_name="$2"
if ! "$SCRIPT" "$exit_code" "$TMP_DIR/$log_name.log" > "$TMP_DIR/$log_name.out" 2>&1; then
echo "FAIL: expected $log_name to pass guard" >&2
cat "$TMP_DIR/$log_name.out" >&2
exit 1
fi
}

expect_fail() {
local exit_code="$1"
local log_name="$2"
local expected="$3"
if "$SCRIPT" "$exit_code" "$TMP_DIR/$log_name.log" > "$TMP_DIR/$log_name.out" 2>&1; then
echo "FAIL: expected $log_name to fail guard" >&2
exit 1
fi
if ! grep -Fq "$expected" "$TMP_DIR/$log_name.out"; then
echo "FAIL: $log_name did not report expected reason: $expected" >&2
cat "$TMP_DIR/$log_name.out" >&2
exit 1
fi
}

write_log expected-fail \
"Test Suite 'Selected tests' failed." \
"Executed 1 test, with 1 failure (0 unexpected) in 0.1 seconds"
expect_pass 65 expected-fail

write_log timeout-after-summary \
"Executed 1 test, with 1 failure (0 unexpected) in 0.1 seconds" \
"xcodebuild unit test timeout after 900s; terminating" \
"** BUILD INTERRUPTED **"
expect_fail 124 timeout-after-summary "xcodebuild watchdog timeout"
Comment on lines +72 to +76

@greptile-apps greptile-apps Bot May 22, 2026

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.

P2 Grep-based timeout path not independently exercised

The timeout-after-summary case passes exit code 124, which hits the [ "$EXIT_CODE" = "124" ] branch in the guard before the grep -Fq "xcodebuild unit test timeout after" branch runs. Both branches add the same reason and deduplication masks the second. There's no test that sends a non-124 exit code with the timeout string in the log, so the grep-only path for timeout detection is untested. A CI scenario where a wrapper script exits non-124 but still injects the timeout string into the log would silently pass the guard with current coverage.

@lawrencecchen lawrencecchen May 22, 2026

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.

Fixed by adding a timeout-string-only case with a non-124 exit code, so the log pattern is covered independently of the process timeout status.

— Claude Code

@lawrencecchen lawrencecchen May 22, 2026

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.

Already addressed in current head: tests/test_ci_unit_test_output_guard.sh includes timeout-string-only with exit code 65, which exercises timeout detection from log text without relying on exit code 124.

— Claude Code


write_log build-interrupted \
"Executed 1 test, with 1 failure (0 unexpected) in 0.1 seconds" \
"** BUILD INTERRUPTED **"
expect_fail 65 build-interrupted "xcodebuild build interruption"

write_log swift-crash \
"Executed 1 test, with 1 failure (0 unexpected) in 0.1 seconds" \
"Program crashed: Signal 11: Backtracing from 0x18b5c4028..."
expect_fail 65 swift-crash "Swift crash output"

write_log swift-backtrace-prompt \
"Executed 1 test, with 1 failure (0 unexpected) in 0.1 seconds" \
"Press space to interact, D to debug, or any other key to quit (1s)"
expect_fail 65 swift-backtrace-prompt "Swift crash output"

echo "PASS: CI unit-test output guard fails hard-failure logs"
Loading