From cbf4c1a9397117f4783932cd5551f892b83680b6 Mon Sep 17 00:00:00 2001 From: arzafran Date: Thu, 30 Jul 2026 20:07:48 -0300 Subject: [PATCH 1/2] test: stop skipping testCmdWClosesWindowWhenClosingLastSurfaceInLastWorkspace The skip carried no reason. Line 19 set STATEFUL_TEST_SKIP with no comment, and the commit that introduced it says nothing about it -- while the quarantine list directly below carries several paragraphs explaining itself. So one test has been silently excluded from CI with no rationale anyone could check, which leaves it in the worst state available: neither fixed nor deleted, and no way to tell which it should be. The likeliest cause was timing. It asserted the window was gone after a single fixed 0.05s run-loop spin -- the same failure mode that made this whole class flaky enough to need a retry. #218 converted it to wait for `window(withId:) == nil` with a 2s budget, so that cause should be gone. Un-skipped to find out. If it fails for some other reason, the skip goes back WITH the reason written down. Refs #186 --- scripts/ci-run-unit-tests.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/ci-run-unit-tests.sh b/scripts/ci-run-unit-tests.sh index f8205b6e..458ef68d 100755 --- a/scripts/ci-run-unit-tests.sh +++ b/scripts/ci-run-unit-tests.sh @@ -16,7 +16,16 @@ SWIFTPM_CACHE_DIR="${PROGRAMA_SWIFTPM_CACHE_DIR:-$HOME/Library/Caches/org.swift. DERIVED_DATA_DIR="${PROGRAMA_DERIVED_DATA_DIR:-$HOME/Library/Developer/Xcode/DerivedData}" TEST_SCOPE="${PROGRAMA_UNIT_TEST_SCOPE:-serial}" STATEFUL_TEST_CLASS="programaTests/AppDelegateShortcutRoutingTests" -STATEFUL_TEST_SKIP="${STATEFUL_TEST_CLASS}/testCmdWClosesWindowWhenClosingLastSurfaceInLastWorkspace" +# testCmdWClosesWindowWhenClosingLastSurfaceInLastWorkspace used to be skipped here, +# with no recorded reason. The likeliest cause was timing: it asserted the window had +# gone after a single fixed 0.05s run-loop spin, which is the same failure mode that +# made this whole class flaky enough to need a retry. That test now waits for +# `window(withId:) == nil` with a 2s budget instead of assuming one spin is enough, +# so the original cause should be gone. Un-skipped deliberately to find out. +# +# If it turns out to fail for some other reason, re-add the skip WITH the reason +# written down -- a silently excluded test is worse than either a fixed one or a +# deleted one, because nobody can tell which it should be. # Test CLASSES quarantined when PROGRAMA_UNIT_TEST_QUARANTINE is set (the # macos-15 compat leg). Every class here builds real NSWindows and waits on async @@ -117,11 +126,9 @@ run_unit_tests() { ;; stateful) xcode_args+=("-only-testing:${STATEFUL_TEST_CLASS}") - xcode_args+=("-skip-testing:${STATEFUL_TEST_SKIP}") xcode_args+=("-parallel-testing-enabled" "NO") ;; serial|*) - xcode_args+=("-skip-testing:${STATEFUL_TEST_SKIP}") xcode_args+=("-parallel-testing-enabled" "NO") ;; esac From c40e9dd583b75b139f70a9c6ba3f4b04549940e9 Mon Sep 17 00:00:00 2001 From: arzafran Date: Thu, 30 Jul 2026 20:32:49 -0300 Subject: [PATCH 2/2] test: record why testCmdWClosesWindow... is skipped Un-skipping it (previous commit) answered the question the skip could not: it does not flake, it fails. AppDelegateShortcutRoutingTests.swift:1463: failed - Timed out waiting for Cmd+W on the last surface to close the window AppDelegateShortcutRoutingTests.swift:1465: XCTAssertNil failed: "" It waits the full 2s that #218 gave it and the window is still there, so more time does not help. The shortcut itself dispatches -- the XCTAssertTrue on debugHandleCustomShortcut just above passes. And headless window closing works in general: two sibling Cmd+Ctrl+W tests close real windows on the same runner and are not skipped. That narrows it to the cascade this test exercises -- close last surface, close last workspace, close window -- and leaves open whether it is a headless-only gap or a real bug in a path users rely on. Restores the skip so CI stays green, with all of that written down. The wiring is byte-identical to before; the diff is comment-only. Refs #186 --- scripts/ci-run-unit-tests.sh | 37 +++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/scripts/ci-run-unit-tests.sh b/scripts/ci-run-unit-tests.sh index 458ef68d..3f555b5b 100755 --- a/scripts/ci-run-unit-tests.sh +++ b/scripts/ci-run-unit-tests.sh @@ -16,16 +16,33 @@ SWIFTPM_CACHE_DIR="${PROGRAMA_SWIFTPM_CACHE_DIR:-$HOME/Library/Caches/org.swift. DERIVED_DATA_DIR="${PROGRAMA_DERIVED_DATA_DIR:-$HOME/Library/Developer/Xcode/DerivedData}" TEST_SCOPE="${PROGRAMA_UNIT_TEST_SCOPE:-serial}" STATEFUL_TEST_CLASS="programaTests/AppDelegateShortcutRoutingTests" -# testCmdWClosesWindowWhenClosingLastSurfaceInLastWorkspace used to be skipped here, -# with no recorded reason. The likeliest cause was timing: it asserted the window had -# gone after a single fixed 0.05s run-loop spin, which is the same failure mode that -# made this whole class flaky enough to need a retry. That test now waits for -# `window(withId:) == nil` with a 2s budget instead of assuming one spin is enough, -# so the original cause should be gone. Un-skipped deliberately to find out. +STATEFUL_TEST_SKIP="${STATEFUL_TEST_CLASS}/testCmdWClosesWindowWhenClosingLastSurfaceInLastWorkspace" +# Skipped because it FAILS on CI, not because it flakes. This carried no reason at +# all until #221 measured it; the note below is what that run established. # -# If it turns out to fail for some other reason, re-add the skip WITH the reason -# written down -- a silently excluded test is worse than either a fixed one or a -# deleted one, because nobody can tell which it should be. +# The test presses Cmd+W on the last surface in the last workspace and expects the +# window to close. On CI the window never closes: +# +# AppDelegateShortcutRoutingTests.swift:1463: failed - Timed out waiting for +# Cmd+W on the last surface to close the window +# AppDelegateShortcutRoutingTests.swift:1465: XCTAssertNil failed: "" +# +# Three things narrow it down: +# * NOT a timing race. #218 replaced the single fixed 0.05s spin with a 2s +# condition wait; it now waits the full two seconds and the window is still +# there. More time does not help. +# * The shortcut IS dispatched -- the XCTAssertTrue on debugHandleCustomShortcut +# just above passes. Routing works; the resulting close does not complete. +# * Headless window closing works in general. testCmdCtrlWPromptsBeforeClosing- +# Window and ...ClosesWindowAfterConfirmation both close real windows on the +# same runner and are not skipped. +# +# So it is specific to the CASCADE this test exercises: close last surface -> close +# last workspace -> close window. Whether that is a headless-only gap or a real +# product bug is undetermined; nobody has reproduced it outside CI. +# +# Do not read this skip as "the test is flaky". It is a known, reproducible failure +# of a behaviour users rely on, parked rather than diagnosed. Worth its own issue. # Test CLASSES quarantined when PROGRAMA_UNIT_TEST_QUARANTINE is set (the # macos-15 compat leg). Every class here builds real NSWindows and waits on async @@ -126,9 +143,11 @@ run_unit_tests() { ;; stateful) xcode_args+=("-only-testing:${STATEFUL_TEST_CLASS}") + xcode_args+=("-skip-testing:${STATEFUL_TEST_SKIP}") xcode_args+=("-parallel-testing-enabled" "NO") ;; serial|*) + xcode_args+=("-skip-testing:${STATEFUL_TEST_SKIP}") xcode_args+=("-parallel-testing-enabled" "NO") ;; esac