Repository navigation
compass(gates): gate_cpu.sh verdict line carries its reason through a pipe #276
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.
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
Merged
jgong5
merged 2 commits into
feature/atomcompass_new
from
compass/issue-191-gate-rc-through-pipe
Sep 23, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """`gate_cpu.sh`'s verdict has to survive being piped, and say why. | ||
|
|
||
| A pipeline's status is its last command's, so `gate_cpu.sh | tail -6` hands | ||
| the caller tail's 0 whatever the gate exited. The script cannot change that. | ||
| What it controls is the text: its last line is the verdict, and that line has | ||
| to carry the reason, because the three common pipes keep different halves of | ||
| the output -- `2>/dev/null | tail -6` drops every stderr line, which on a run | ||
| that exits 98 leaves the verdict number under pytest's green summary. | ||
|
|
||
| Each case runs the real script over a throwaway tree: a one-test suite, an | ||
| empty exclusion list, and a trigger list that the stamped diff touches, so the | ||
| run is green and then exits 98 for the GPU tier. Nothing needs a driver. | ||
| """ | ||
|
|
||
| import os | ||
| import shutil | ||
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| REPO = Path(__file__).resolve().parents[2] | ||
| BASH = shutil.which("bash") | ||
| BLIND = "atom/blind.py" | ||
|
|
||
| pytestmark = pytest.mark.skipif( | ||
| BASH is None or shutil.which("git") is None, reason="needs bash and git" | ||
| ) | ||
|
|
||
|
|
||
| def _tree(root, stamped): | ||
| """A minimal checkout the gate accepts, green, touching one GPU trigger.""" | ||
| shutil.copytree(REPO / "scripts" / "compass", root / "scripts" / "compass") | ||
| (root / "scripts" / "compass" / "cpu_gate_exclude.txt").write_text("") | ||
| (root / "scripts" / "compass" / "gpu_gate_triggers.txt").write_text(BLIND + "\n") | ||
| (root / "atom").mkdir() | ||
| (root / "atom" / "__init__.py").write_text("") | ||
| (root / "tests").mkdir() | ||
| (root / "tests" / "test_ok.py").write_text("def test_ok():\n pass\n") | ||
| if stamped: | ||
| (root / ".compass-changed").write_text(BLIND + "\n") | ||
| (root / ".compass-commit").write_text("0" * 40 + "\n") | ||
| return root | ||
|
|
||
|
|
||
| def _run(root, command): | ||
| env = {k: v for k, v in os.environ.items() if not k.startswith("COMPASS_")} | ||
| env["PATH"] = os.path.dirname(sys.executable) + os.pathsep + env["PATH"] | ||
| # A checkout above the temp dir must not make the bare tree look like git. | ||
| env["GIT_CEILING_DIRECTORIES"] = str(root.parent) | ||
| return subprocess.run( | ||
| [BASH, "-c", command.format(gate="scripts/compass/gate_cpu.sh")], | ||
| cwd=root, | ||
| env=env, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| timeout=300, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def gpu_tree(tmp_path_factory): | ||
| return _tree(tmp_path_factory.mktemp("gate") / "ATOM", stamped=True) | ||
|
|
||
|
|
||
| def test_unpiped_the_gate_exits_98(gpu_tree): | ||
| # The control: the status every piped case below loses. | ||
| out = _run(gpu_tree, "{gate}") | ||
| assert out.returncode == 98, out.stdout + out.stderr | ||
|
|
||
|
|
||
| def test_the_verdict_key_is_printed_exactly_once(gpu_tree): | ||
| # A reader taking the first match must land on the verdict, not on prose | ||
| # that mentions the key. | ||
| out = _run(gpu_tree, "{gate} 2>&1") | ||
| keyed = [line for line in out.stdout.splitlines() if "GATE_CPU_RC=" in line] | ||
| assert len(keyed) == 1, keyed | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "pipe", ["| tail -6", "2>&1 | tail -6", "2>/dev/null | tail -6"] | ||
| ) | ||
| def test_a_piped_run_still_reads_as_not_passed_and_says_why(gpu_tree, pipe): | ||
| out = _run(gpu_tree, "{gate} " + pipe) | ||
| assert out.returncode == 0, "the pipe kept the gate's status; recheck the premise" | ||
| kept = out.stdout.splitlines() | ||
| assert kept, "the pipe kept nothing" | ||
| verdict = kept[-1] | ||
| assert verdict.startswith("GATE_CPU_RC=98 NOT PASSED"), kept | ||
| assert "gate_gpu.sh" in verdict, ( | ||
| f"`{pipe}` keeps a verdict with no reason: {verdict!r}. Beside the " | ||
| "pipe's own 0 and pytest's green summary, a bare 98 is all the reader " | ||
| "has, so the reason has to travel on the verdict line." | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "orphan, cause", | ||
| [(False, "resolves to no commit"), (True, "shares no commit with HEAD")], | ||
| ) | ||
| def test_a_git_checkout_is_not_called_unstamped(tmp_path, orphan, cause): | ||
| root = _tree(tmp_path / "ATOM", stamped=False) | ||
| git = ["git", "-C", str(root), "-c", "user.name=t", "-c", "user.email=t@t"] | ||
| subprocess.run(git + ["init", "-q"], check=True) | ||
| subprocess.run(git + ["add", "-A"], check=True) | ||
| subprocess.run(git + ["commit", "-qm", "t"], check=True) | ||
| if orphan: | ||
| # An integration branch with no history in common with HEAD. | ||
| tree = git + ["commit-tree", "HEAD^{tree}", "-m", "o"] | ||
| sha = subprocess.run(tree, check=True, capture_output=True, text=True) | ||
| branch = ["branch", "feature/atomcompass_new", sha.stdout.strip()] | ||
| subprocess.run(git + branch, check=True) | ||
| out = _run(root, "{gate}") | ||
| gpu = [line for line in out.stdout.splitlines() if line.startswith("gpu:")] | ||
| assert out.returncode == 98 and gpu, out.stdout + out.stderr | ||
| assert "never stamped" not in gpu[0], gpu[0] | ||
| assert f"feature/atomcompass_new {cause}" in gpu[0], gpu[0] | ||
|
|
||
|
|
||
| def test_a_tree_with_no_git_and_no_stamp_is_still_called_unstamped(tmp_path): | ||
| out = _run(_tree(tmp_path / "ATOM", stamped=False), "{gate}") | ||
| gpu = [line for line in out.stdout.splitlines() if line.startswith("gpu:")] | ||
| assert out.returncode == 98 and gpu, out.stdout + out.stderr | ||
| assert "never stamped" in gpu[0], gpu[0] |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking (principle 8): no test reaches this arm, one of the three causes the PR says it now names.
The mutation below survives the whole new file and
test_gate_cpu_pipe_identity.py: 8 passed, and the file stays at 268 lines.test_a_git_checkout_is_not_called_unstampedonly builds a checkout where the ref does not resolve.I checked the arm by hand, and it is correct. I built a git checkout with an orphan
feature/atomcompass_newthat shares no commit withmaster, and ran it on node 18:Given that you are already pushing for the header line, a third case in the same test file would close the gap. It needs about 3 extra git calls:
checkout --orphan feature/atomcompass_new; commit; checkout master.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Covered in
c4509d1d9.test_a_git_checkout_is_not_called_unstampedis now parametrized over two cases:[False-resolves to no commit]: an unresolvable ref, as before.[True-shares no commit with HEAD]: an orphanfeature/atomcompass_newmade withgit commit-tree HEAD^{tree}plusgit branch, so there is no checkout switching.Your mutation R3, applied at head (268 → 268 lines), now gives 1 failed:
test_a_git_checkout_is_not_called_unstamped[True-shares no commit with HEAD]. The null control gives 10 passed.