diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index cd4a2005..780a8037 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -117,6 +117,48 @@ jobs: - name: Validate platform configs and emulator profiles run: python scripts/validate_schemas.py --source-only + # A profile written from a clone can cite lines that do not hold the value + # it declares; schemas and tests pass it. Each profile a change adds is + # confronted with its upstream, both roads into main. An edited profile is + # not held to it: the backlog of older refs is reviewed by hand. + check-new-profiles: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 + with: + python-version: "3.12" + + - name: Install dependencies + run: pip install pyyaml + + - name: Confront added profiles with their upstream + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BEFORE: ${{ github.event.before }} + run: | + # A pull request is checked out as a merge commit whose first + # parent is the base; a push names the commit it moved main from. + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then + base=HEAD^1 + elif [ -z "$BEFORE" ] || [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then + echo "first push of the branch: no base to diff against" + exit 0 + else + base=$BEFORE + fi + status=0 + for file in $(git diff --name-only --diff-filter=A "$base" HEAD -- 'emulators/*.yml'); do + name=$(basename "$file" .yml) + echo "::group::$name" + python scripts/profile_sync.py --emulator "$name" --strict || status=1 + echo "::endgroup::" + done + exit $status + run-tests: runs-on: ubuntu-latest steps: diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 47bf171e..76df322e 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -3251,6 +3251,11 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--stale-before", help="keep profiles profiled before a date") parser.add_argument("--limit", type=int, help="stop after N profiles") parser.add_argument("--changed-only", action="store_true") + parser.add_argument( + "--strict", + action="store_true", + help="exit 1 when a profile has refs to review, as for a new profile in CI", + ) parser.add_argument("--json", action="store_true", dest="as_json") parser.add_argument("--markdown", action="store_true") parser.add_argument( @@ -3600,6 +3605,7 @@ def _refuse_flag_combinations(args: argparse.Namespace) -> None: ("--fetch-plan", args.fetch_plan), ("--triage", args.triage), ("--changed-only", args.changed_only), + ("--strict", args.strict), ("--check-version", args.check_version), ("--detect-new-files", args.detect_new_files), ("--watch-hashes", args.watch_hashes), @@ -3731,7 +3737,7 @@ def main() -> None: if not args.changed_only or report.needs_review() or report.name in failed ] _emit(args, selected, reports, shown) - if failed: + if failed or (args.strict and any(report.needs_review() for report in reports)): raise SystemExit(1) diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 29914055..72ee842c 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -230,6 +230,21 @@ class WorkflowRegressions(unittest.TestCase): f"{job} carries a condition; the suite must run on push and on PR", ) + def test_added_profiles_meet_their_upstream_on_both_roads(self): + """A new profile's refs were confronted only if its author remembered.""" + workflow = self._workflow("validate.yml") + runs = [ + (name, str(step.get("run", ""))) + for name, body in workflow["jobs"].items() + for step in body.get("steps", []) + if "profile_sync.py" in str(step.get("run", "")) + ] + self.assertTrue(runs, "no job confronts profiles with their upstream") + for name, body in runs: + self.assertIn("--strict", body) + self.assertIn("--diff-filter=A", body) + self.assertIsNone(workflow["jobs"][name].get("if"), f"{name} must run on push too") + def test_both_events_watch_the_same_paths(self): triggers = self._workflow("validate.yml")["on"] self.assertEqual( diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 13123f31..6244f660 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -3804,6 +3804,19 @@ class TestOutputModesApplyTheirFlags(unittest.TestCase): self.assertEqual(code, 0) self.assertEqual([r["name"] for r in json.loads(out)], ["drift"]) + def test_strict_fails_on_refs_to_review(self): + """A new profile whose refs miss their value used to pass CI green.""" + code, out, _ = self._main("--changed-only", "--strict") + self.assertEqual(code, 1) + self.assertIn("drift", out) + + def test_strict_passes_when_nothing_is_to_review(self): + from unittest import mock + + with mock.patch.object(ProfileReport, "needs_review", return_value=0): + code, _, _ = self._main("--strict") + self.assertEqual(code, 0) + def test_extras_are_refused_with_json(self): code, _, err = self._main("--json", "--check-version") self.assertEqual(code, 1) @@ -3872,7 +3885,7 @@ class TestRealignFlagMatrix(unittest.TestCase): def test_output_flags_are_refused(self): for flag in ( "--json", "--markdown", "--fetch-plan", "--triage", - "--changed-only", "--check-version", "--detect-new-files", + "--changed-only", "--strict", "--check-version", "--detect-new-files", "--watch-hashes", "--full-diff", "--tree-diff", "--accept-changed", ):