From 17bc4f716a9b18b7db5162e00295c8e4ded81610 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 11:08:48 -0400 Subject: [PATCH 1/4] test(ci): passing guard tests must print no workflow commands Red: test_ci_ui_tests_dispatch, test_ci_pr_media, test_ci_late_placement, test_ci_main_regression_bisect, test_ci_prune_pr_media and test_ios_upload_batching print ::error/::warning/::notice from their fixtures, which the runner turns into annotations on a passing run. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci-guards.yml | 1 + ...i_guard_tests_emit_no_workflow_commands.py | 80 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100755 tests/test_ci_guard_tests_emit_no_workflow_commands.py diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 9ca07a3f9116..4394b04f5f34 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -618,6 +618,7 @@ jobs: python3 tests/test_ci_focused_test_selectors.py python3 tests/test_ci_ui_tests_dispatch.py python3 tests/test_ci_pr_media.py + python3 tests/test_ci_guard_tests_emit_no_workflow_commands.py - name: Validate manual macOS package cache recovery if: ${{ matrix.group == 'app-host-execution' }} diff --git a/tests/test_ci_guard_tests_emit_no_workflow_commands.py b/tests/test_ci_guard_tests_emit_no_workflow_commands.py new file mode 100755 index 000000000000..06786324c3fd --- /dev/null +++ b/tests/test_ci_guard_tests_emit_no_workflow_commands.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +"""Passing guard tests print no GitHub workflow commands. + +The runner reads every line of a step's output that starts with `::` as a +workflow command, so a fixture that makes a script under test print +`::error::...` becomes a red annotation on the pull request's run page even +though the test passed. PR 15160's run 36420353579 showed "No ci-ui-tests.yml +run titled 'UI tests for CI run 100 attempt 1' appeared" and "https://x/900 +ended success without running the UI tests" from test_ci_ui_tests_dispatch.py, +which read as real CI failures. + +Each module below exercises code that annotates on purpose. It runs here the +way the guard job runs it, with GITHUB_ACTIONS set, and its output must hold +no line the runner would parse as a command. A module that fails is left to +its own step to report: a failing test's captured output may annotate. +""" + +from __future__ import annotations + +import os +import re +import subprocess +import sys +import tempfile +import unittest +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + +# Guard tests whose scripts under test print annotations. +ANNOTATING_MODULES = ( + "tests/test_ci_helper_prebuild_lifecycle.py", + "tests/test_ci_late_placement.py", + "tests/test_ci_main_regression_bisect.py", + "tests/test_ci_pr_media.py", + "tests/test_ci_prune_pr_media.py", + "tests/test_ci_ui_tests_dispatch.py", + "tests/test_ios_upload_batching.py", +) + +# unittest's progress characters share stderr with the script's output, so a +# command may follow them on one line of the combined log; the runner reads +# stdout and stderr as separate streams, where it starts the line. +COMMAND = re.compile(r"^[.EFsxu]*::(?:error|warning|notice|group|endgroup|debug|add-mask|stop-commands)\b") + + +def run_module(relative: str) -> tuple[int, str]: + """Exit code and combined output. Files, not pipes: a descendant a test + leaves behind would hold a pipe open and hang the read.""" + env = {**os.environ, "GITHUB_ACTIONS": "true"} + env.pop("GITHUB_OUTPUT", None) + env.pop("GITHUB_STEP_SUMMARY", None) + with tempfile.TemporaryFile("w+") as output: + code = subprocess.run([sys.executable, str(ROOT / relative)], cwd=ROOT, env=env, stdin=subprocess.DEVNULL, + stdout=output, stderr=output, timeout=600, check=False).returncode + output.seek(0) + return code, output.read() + + +class GuardTestsEmitNoWorkflowCommandsTests(unittest.TestCase): + def test_passing_guard_tests_print_no_workflow_commands(self) -> None: + with ThreadPoolExecutor(max_workers=len(ANNOTATING_MODULES)) as pool: + results = dict(zip(ANNOTATING_MODULES, pool.map(run_module, ANNOTATING_MODULES))) + checked = 0 + for relative, result in results.items(): + code, output = result + with self.subTest(module=relative): + if code != 0: + continue + checked += 1 + leaked = [line for line in output.splitlines() if COMMAND.match(line)] + # repr() quotes each line, so this report is never read as a command itself. + self.assertFalse(leaked, f"{relative} printed workflow commands:\n" + + "\n".join(repr(line) for line in leaked[:10])) + self.assertGreater(checked, 0, "no annotating guard test passed, so nothing was checked") + + +if __name__ == "__main__": + unittest.main(buffer=True) From 7709909bb7b0027e1e9f78c957846a2ee5d57bf4 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 11:09:15 -0400 Subject: [PATCH 2/4] fix(ci): buffer guard-test output so passing fixtures annotate nothing unittest.main(buffer=True) captures each test's stdout and stderr and prints it only when that test fails or errors, so a fixture's ::error line never reaches the step log of a passing run. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_ci_helper_prebuild_lifecycle.py | 2 +- tests/test_ci_late_placement.py | 2 +- tests/test_ci_main_regression_bisect.py | 2 +- tests/test_ci_pr_media.py | 2 +- tests/test_ci_prune_pr_media.py | 2 +- tests/test_ci_ui_tests_dispatch.py | 2 +- tests/test_ios_upload_batching.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_ci_helper_prebuild_lifecycle.py b/tests/test_ci_helper_prebuild_lifecycle.py index 460796b336c3..107f66fe3726 100644 --- a/tests/test_ci_helper_prebuild_lifecycle.py +++ b/tests/test_ci_helper_prebuild_lifecycle.py @@ -101,4 +101,4 @@ def test_cancellation_kills_helper_descendants(self): if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ci_late_placement.py b/tests/test_ci_late_placement.py index 8f5b84c248d5..ac96cd3c5bf6 100644 --- a/tests/test_ci_late_placement.py +++ b/tests/test_ci_late_placement.py @@ -439,4 +439,4 @@ def test_moved_jobs_leave_the_marker_the_rescue_watch_looks_for(self): if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ci_main_regression_bisect.py b/tests/test_ci_main_regression_bisect.py index 988d99a32000..75459b505cf2 100644 --- a/tests/test_ci_main_regression_bisect.py +++ b/tests/test_ci_main_regression_bisect.py @@ -479,4 +479,4 @@ def test_advances_on_a_schedule_and_after_each_report(self): if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ci_pr_media.py b/tests/test_ci_pr_media.py index ee5284f81191..e051cc116679 100644 --- a/tests/test_ci_pr_media.py +++ b/tests/test_ci_pr_media.py @@ -722,4 +722,4 @@ def test_adopts_on_matches_the_product_family(self) -> None: if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ci_prune_pr_media.py b/tests/test_ci_prune_pr_media.py index 8a657e166515..55265df0d997 100644 --- a/tests/test_ci_prune_pr_media.py +++ b/tests/test_ci_prune_pr_media.py @@ -154,4 +154,4 @@ def test_only_main_prunes_and_a_dry_run_is_the_default(self) -> None: if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ci_ui_tests_dispatch.py b/tests/test_ci_ui_tests_dispatch.py index cb135071428c..424a8f371e97 100644 --- a/tests/test_ci_ui_tests_dispatch.py +++ b/tests/test_ci_ui_tests_dispatch.py @@ -458,4 +458,4 @@ def test_the_dispatching_workflow_runs_from_the_default_branch(self) -> None: if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) diff --git a/tests/test_ios_upload_batching.py b/tests/test_ios_upload_batching.py index a9d9d5b0bbb1..8bfea6e439e6 100644 --- a/tests/test_ios_upload_batching.py +++ b/tests/test_ios_upload_batching.py @@ -408,4 +408,4 @@ def test_batching_steps_continue_on_error(self): if __name__ == "__main__": - unittest.main() + unittest.main(buffer=True) From 5f9d1ebbfb57b9aacdfb3826de0db7f7e5dd5f4d Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 11:15:22 -0400 Subject: [PATCH 3/4] ci: register the workflow-command guard test Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test-execution.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 69dcb0e8028f..03da13d385a0 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -1429,3 +1429,7 @@ lane = "linux-guard" [[test]] path = "tests/test_ci_pr_media.py" lane = "linux-guard" + +[[test]] +path = "tests/test_ci_guard_tests_emit_no_workflow_commands.py" +lane = "linux-guard" From 016d7a28d923601816194cc849a94b840605d805 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Mon, 28 Sep 2026 11:35:25 -0400 Subject: [PATCH 4/4] ci: quiet the artifact transport test; widen the command check and report skipped modules test_ci_parallel_artifact_transport printed seven ::warning:: lines on a pass (ci-artifact-transport.yml); it now buffers too. The check matches leading whitespace and every runner command, and a module that fails locally shows as skipped rather than silently unchecked. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_ci_guard_tests_emit_no_workflow_commands.py | 7 +++++-- tests/test_ci_parallel_artifact_transport.py | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_ci_guard_tests_emit_no_workflow_commands.py b/tests/test_ci_guard_tests_emit_no_workflow_commands.py index 06786324c3fd..9b9c038bd9f9 100755 --- a/tests/test_ci_guard_tests_emit_no_workflow_commands.py +++ b/tests/test_ci_guard_tests_emit_no_workflow_commands.py @@ -33,6 +33,7 @@ "tests/test_ci_helper_prebuild_lifecycle.py", "tests/test_ci_late_placement.py", "tests/test_ci_main_regression_bisect.py", + "tests/test_ci_parallel_artifact_transport.py", "tests/test_ci_pr_media.py", "tests/test_ci_prune_pr_media.py", "tests/test_ci_ui_tests_dispatch.py", @@ -42,7 +43,8 @@ # unittest's progress characters share stderr with the script's output, so a # command may follow them on one line of the combined log; the runner reads # stdout and stderr as separate streams, where it starts the line. -COMMAND = re.compile(r"^[.EFsxu]*::(?:error|warning|notice|group|endgroup|debug|add-mask|stop-commands)\b") +COMMAND = re.compile(r"^\s*[.EFsxu]*::(?:error|warning|notice|debug|group|endgroup|add-mask|stop-commands|echo" + r"|set-output|save-state|set-env|add-path|add-matcher|remove-matcher)\b") def run_module(relative: str) -> tuple[int, str]: @@ -67,7 +69,8 @@ def test_passing_guard_tests_print_no_workflow_commands(self) -> None: code, output = result with self.subTest(module=relative): if code != 0: - continue + # Its own step reports the failure; say which module this check skipped. + self.skipTest(f"{relative} failed here (exit {code}), so its output was not checked") checked += 1 leaked = [line for line in output.splitlines() if COMMAND.match(line)] # repr() quotes each line, so this report is never read as a command itself. diff --git a/tests/test_ci_parallel_artifact_transport.py b/tests/test_ci_parallel_artifact_transport.py index 23d33c9b0d9d..942439c757dd 100644 --- a/tests/test_ci_parallel_artifact_transport.py +++ b/tests/test_ci_parallel_artifact_transport.py @@ -458,4 +458,4 @@ def test_unknown_kind_cannot_choose_a_destination(self): if __name__ == "__main__": - unittest.main(verbosity=2) + unittest.main(verbosity=2, buffer=True)