Skip to content

Fix two persistent CI-blocking test-harness bugs (macOS SIGSEGV + Linux death-test timeout) - #185

Closed
beaucollins wants to merge 1 commit into
mainfrom
bcollins/fix-flaky-death-test-and-macos-gui-guard
Closed

Fix two persistent CI-blocking test-harness bugs (macOS SIGSEGV + Linux death-test timeout)#185
beaucollins wants to merge 1 commit into
mainfrom
bcollins/fix-flaky-death-test-and-macos-gui-guard

Conversation

@beaucollins

Copy link
Copy Markdown
Collaborator

Problem

Two jobs have been persistently red on mainmacOS: C++ & Platform Tests and Linux: C++ Tests (same shape across the last several nightly runs). Neither is a product regression; both are test-harness bugs that only manifest on CI runners.

Root causes & fixes

Linux — //valdi:test TIMEOUT

RuntimeTests/RuntimeFixture.AsyncStrictModeSyncCallAssertsOnMainThread is the only EXPECT_DEATH in the suite. It uses the default "fast" death-test style, which fork()s in place. The fixture owns a live runtime with several worker/JS threads, so the forked child inherits them frozen mid-flight (gtest even warns: "Death tests use fork() … detected 12 threads"). On loaded CI runners the child then stalls ~94 s on an orphaned lock before the assert's abort() completes; three parameterized variants blow the //valdi:test timeout.

Fix: switch this test to "threadsafe" death-test style, which re-execs a fresh process for the death check so no locked mutexes are inherited.

macOS — SIGSEGV in valdi_macos_objc_test

testPhysicalTextEditingAppliesWillChangeAndSynchronizesNativeOverrides intends to skip on headless CI, but gated the skip on CGSessionCopyCurrentDictionary() != NULL. GitHub's macOS runners report a login session, so the guard failed to skip; the test then drove a live key window + first responder + field editor and SIGSEGV'd in AppKit window activation (before any assertion — exactly the failure the test's own comment predicts).

Fix: require the session to be active on the console (kCGSessionOnConsoleKey) and bail out under CI, where no usable WindowServer is guaranteed. The live-editor path still runs on developer machines with a real GUI session. (The test was already a no-op on CI by design; this just makes the skip actually fire there.)

Verification

  • Linux: reproduced locally and confirmed the fix — the 4 variants now finish in ~30–50 ms each (previously 94 s+ per variant on CI) with --gtest_print_time=1. The "fast""threadsafe" re-exec works fine under the Bazel test sandbox.
  • macOS: header-only/logic change (CoreGraphics already imported); the Apple-only target builds under the macOS CI job.

🤖 Generated with Claude Code

Both the "macOS: C++ & Platform Tests" and "Linux: C++ Tests" jobs have been
red on main for weeks. Neither is a product regression; both are test-harness
bugs.

Linux — Runtime_tests.cpp: AsyncStrictModeSyncCallAssertsOnMainThread uses the
default "fast" gtest death-test style, which fork()s in place. This fixture owns
a live runtime with several worker/JS threads, so the forked child inherits them
frozen mid-flight. On loaded CI runners the child stalls ~90s on an orphaned lock
before the assert's abort() completes; the parameterized variants (~94s each)
then blow the //valdi:test timeout. Switch this test to "threadsafe" death-test
style, which re-execs a fresh process for the death check so no locked mutexes
are inherited. Verified locally: the 4 variants now finish in ~30-50ms each
(previously 94s+ on CI).

macOS — SCValdiMacOSViewManagerTests.mm: testPhysicalTextEditingApplies...
gated its headless skip on CGSessionCopyCurrentDictionary() != NULL. GitHub's
macOS runners report a login session, so the guard failed to skip; the test then
drove a live key window + first responder + field editor and SIGSEGV'd in AppKit
window activation. Require the session to be active on the console and bail out
under CI, where no usable WindowServer is guaranteed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📊 PR Size: size/S

Total changes: 23 lines (2 files)

Top files changed:

  • valdi/test/macos/SCValdiMacOSViewManagerTests.mm: +12 -4
  • valdi/test/integration/Runtime_tests.cpp: +7 -0

Size calculated as additions + deletions. Labels: XS (<10), S (<50), M (<250), L (<1000), XL (1000+)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant