From 6b7d93fc9ea287c0dad4bf4693f7d866855243b8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:56:17 +0200 Subject: [PATCH] fix: report profiles the forge failed to serve --- scripts/profile_sync.py | 37 ++++++++++++++++++++++++++++--------- tests/test_profile_sync.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index c31b36d7..4364f8da 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -3427,6 +3427,9 @@ def main() -> None: if writes and not args.dry_run and not args.force: _refuse_dirty_tree(args.emulators_dir) + # A profile that could not be compared or written is not a profile with + # nothing to review: it stays in every output and fails the run. + failed: set[str] = set() if args.realign_prose: for name in selected: profile_path = Path(args.emulators_dir) / f"{name}.yml" @@ -3441,8 +3444,12 @@ def main() -> None: raise except upstream.UpstreamError as exc: print(f"{name}: {exc}", file=sys.stderr) + failed.add(name) except YamlWriteError as exc: print(f"{name}: write refused: {exc}", file=sys.stderr) + failed.add(name) + if failed: + raise SystemExit(1) return reports = [] @@ -3470,6 +3477,7 @@ def main() -> None: report = ProfileReport( name=name, entries=[], counts={}, skipped=f"upstream error: {exc}" ) + failed.add(name) reports.append(report) if writes: try: @@ -3477,13 +3485,26 @@ def main() -> None: except YamlWriteError as exc: # The guard already left the file untouched; keep going. print(f"{name}: write refused: {exc}", file=sys.stderr) + failed.add(name) # --changed-only narrows every output, not the text report alone: the # documented `--all --changed-only --json` listed every profile. shown = [ report for report in reports - if not args.changed_only or report.needs_review() + if not args.changed_only or report.needs_review() or report.name in failed ] + _emit(args, selected, reports, shown) + if failed: + raise SystemExit(1) + + +def _emit( + args: argparse.Namespace, + selected: dict, + reports: list[ProfileReport], + shown: list[ProfileReport], +) -> None: + """Write the reports in the form the flags ask for.""" if args.as_json: print(json.dumps([report_to_dict(r) for r in shown], indent=2)) return @@ -3501,23 +3522,21 @@ def main() -> None: if args.triage: _print_triage(args, selected, shown) return - printed = 0 - for report in reports: - if args.changed_only and not report.needs_review(): - continue - printed += 1 + for report in shown: print(format_report(report, args.changed_only)) _print_extras(args, selected[report.name], report) print() if args.changed_only: - _print_elided(reports, printed) + _print_elided(reports, len(shown), frozenset(report.name for report in shown)) -def _print_elided(reports: list[ProfileReport], printed: int) -> None: +def _print_elided( + reports: list[ProfileReport], printed: int, listed: frozenset[str] = frozenset() +) -> None: """Account for what --changed-only left out, grouped by reason.""" reasons: dict[str, int] = {} for report in reports: - if report.needs_review(): + if report.needs_review() or report.name in listed: continue key = report.skipped or "nothing to review" reasons[key] = reasons.get(key, 0) + 1 diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 145a688a..d71e5925 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -3698,6 +3698,34 @@ class TestOutputModesApplyTheirFlags(unittest.TestCase): self.assertEqual(code, 1) self.assertIn("--check-version", err) + def test_a_profile_the_forge_failed_is_listed_and_fails_the_run(self): + """Dropped by --changed-only, a 502 read as nothing to review.""" + from unittest import mock + + def build(name, *args, **kwargs): + if name == "drift": + raise profile_sync.upstream.UpstreamError("https://x: HTTP 502") + return ProfileReport(name=name) + + out = io.StringIO() + saved = sys.argv + try: + sys.argv = ["profile_sync.py", "--all", "--changed-only", "--json"] + with mock.patch.object(profile_sync, "load_emulator_profiles", return_value={}), \ + mock.patch.object(profile_sync, "select_profiles", + return_value={"clean": {}, "drift": {}}), \ + mock.patch.object(profile_sync, "_check_quota"), \ + mock.patch.object(profile_sync, "build_report", side_effect=build), \ + contextlib.redirect_stdout(out), contextlib.redirect_stderr(io.StringIO()): + with self.assertRaises(SystemExit) as caught: + profile_sync.main() + finally: + sys.argv = saved + self.assertEqual(caught.exception.code, 1) + (row,) = json.loads(out.getvalue()) + self.assertEqual(row["name"], "drift") + self.assertIn("upstream error", row["skipped"]) + class TestReconcilePerFile(unittest.TestCase): def test_an_anchored_part_settles_only_its_own_file(self):