Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/ci-guards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand Down
4 changes: 4 additions & 0 deletions tests/test-execution.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
83 changes: 83 additions & 0 deletions tests/test_ci_guard_tests_emit_no_workflow_commands.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
#!/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_parallel_artifact_transport.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"^\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]:
"""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:
# 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.
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)
2 changes: 1 addition & 1 deletion tests/test_ci_helper_prebuild_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,4 @@ def test_cancellation_kills_helper_descendants(self):


if __name__ == "__main__":
unittest.main()
unittest.main(buffer=True)
2 changes: 1 addition & 1 deletion tests/test_ci_late_placement.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/test_ci_main_regression_bisect.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,4 +479,4 @@ def test_advances_on_a_schedule_and_after_each_report(self):


if __name__ == "__main__":
unittest.main()
unittest.main(buffer=True)
2 changes: 1 addition & 1 deletion tests/test_ci_parallel_artifact_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/test_ci_pr_media.py
Original file line number Diff line number Diff line change
Expand Up @@ -722,4 +722,4 @@ def test_adopts_on_matches_the_product_family(self) -> None:


if __name__ == "__main__":
unittest.main()
unittest.main(buffer=True)
2 changes: 1 addition & 1 deletion tests/test_ci_prune_pr_media.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/test_ci_ui_tests_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion tests/test_ios_upload_batching.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,4 +408,4 @@ def test_batching_steps_continue_on_error(self):


if __name__ == "__main__":
unittest.main()
unittest.main(buffer=True)
Loading