fix(ci): remove two platform/environment couplings from the pipeline tests - #4
Merged
Merged
Conversation
…tests CI on ubuntu-latest went red on four tests that pass on the Windows dev machine. Neither was a git-identity problem; the repo fixture already sets a repo-local user.name/user.email and every test that makes real commits passed. 1. firewall._normalise did not treat a backslash as a separator. This is a REAL GATE HOLE, not a test artifact: on POSIX, Path(r"config\risk.yaml") is one filename containing a backslash, so it never matched the config/risk.yaml entry and the firewall ALLOWED a frozen path. A gate whose verdict depends on the OS evaluating it is not a gate. The separator is now normalised in the string before Path sees it, and _normalise is pinned directly by a parametrised test rather than inferred from a verdict. 2. test_allowed_evidence_paths_are_accepted asserted .exists() on reports/*, which is gitignored, so a fresh CI clone has none of it. The test was reading the developer local data rather than the code behaviour. It now asserts what assert_allowed actually promises: the path lies in the permitted read set and resolves under the repo root. Whether the artifact has been generated is the caller problem, and render_inventory already reports absent sources as ABSENT. 672 pass locally; ruff and mypy clean.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CI went red on
main(PR #2's merge commit) and onprop/2with the same four failures. Not a git-identity problem — see below.Cause 1 — a real gate hole, not a test artifact
firewall._normalisedid not treat a backslash as a separator. On POSIX,Path(r"config\risk.yaml")is a single filename containing a backslash, so it never matched theconfig/risk.yamlentry and the firewall ALLOWED a frozen path. The same assertion passed on Windows, wherePathsplits on backslash. A gate whose verdict depends on which OS evaluates it is not a gate.Fixed by normalising the separator in the string before
Pathsees it, plus a parametrised test that pins_normalisedirectly rather than inferring it from a verdict, and a mixed-separator case.Cause 2 — a test reading the developer's data
test_allowed_evidence_paths_are_acceptedasserted.exists()onreports/*, which is gitignored (.gitignore:6), so a fresh CI clone has none of it. It now asserts whatassert_allowedactually promises — the path is inside the permitted read set and resolves under the repo root. Whether the artifact has been produced is the caller's problem, andrender_inventoryalready reports absent sources as ABSENT.Refuting the git-identity hypothesis
No identity error appears anywhere in the logs; the
repofixture already sets repo-localuser.email/user.name(no--global); and every test that makes real git commits passed in CI.672 pass locally; ruff and mypy clean. CI is the real verdict here — the POSIX fix cannot be proven on a Windows dev machine.