diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 5c00c398..c0c8659a 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2088,6 +2088,7 @@ def _run_manifest_mode( else: variants = [(args.source, args.required_only)] + failed: list[str] = [] for source, required_only in variants: for group_platforms, representative in groups: print(f"\nGenerating manifest for {representative} [source={source}]...") @@ -2155,6 +2156,13 @@ def _run_manifest_mode( print(f" {alias_path}: alias of {representative}") except (FileNotFoundError, OSError, yaml.YAMLError) as e: print(f" ERROR: {e}") + failed.append(representative) + + # A platform whose manifest could not be built has none, and install.py + # serves whatever an older run left there: that is a failed run. + if failed: + print(f"ERROR: no manifest for {', '.join(sorted(set(failed)))}") + sys.exit(1) @contextlib.contextmanager @@ -2265,6 +2273,7 @@ def _run_platform_packs( else: variants = [(args.source, args.required_only)] + failed: list[str] = [] for source, required_only in variants: for group_platforms, representative in groups: aliases = [p for p in group_platforms if p != representative] @@ -2346,6 +2355,7 @@ def _run_platform_packs( print(f" Renamed -> {os.path.basename(new_path)}") except (FileNotFoundError, OSError, yaml.YAMLError) as e: print(f" ERROR: {e}") + failed.append(representative) print("\nVerifying packs and generating manifests...") skip_conf = bool(system_filter or args.split) @@ -2369,6 +2379,11 @@ def _run_platform_packs( regions=getattr(args, "regions", None), ) all_ok = all_ok and ok + if failed: + # Verification only sees the ZIPs that exist; a pack that was never + # written does not fail it. + print(f"ERROR: no pack for {', '.join(sorted(set(failed)))}") + sys.exit(1) if not all_ok: print("WARNING: some packs have verification errors") sys.exit(1) diff --git a/tests/test_pack_failures.py b/tests/test_pack_failures.py new file mode 100644 index 00000000..9d6c897d --- /dev/null +++ b/tests/test_pack_failures.py @@ -0,0 +1,65 @@ +"""A platform whose pack or manifest raised fails the run. + +Both loops printed ERROR and moved on. The exit code came from the packs +present on disk, so a pack never written, or a manifest install.py would +keep serving from an older run, left the run green. +""" + +from __future__ import annotations + +import argparse +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import generate_pack as gp # noqa: E402 + + +def _boom(*args, **kwargs): + raise OSError(28, "No space left on device") + + +class FailedPlatformFailsTheRun(unittest.TestCase): + def setUp(self): + previous = os.getcwd() + os.chdir(REPO_ROOT) + self.addCleanup(os.chdir, previous) + (REPO_ROOT / "tmp").mkdir(exist_ok=True) + self.tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(self.tmp.cleanup) + + def _args(self) -> argparse.Namespace: + return argparse.Namespace( + all_variants=False, source="full", required_only=False, + platforms_dir="platforms", target=None, split=False, + include_extras=False, emulators_dir="emulators", regions=[], + one_per_slot=False, offline=True, bios_dir="bios", + output_dir=self.tmp.name, verify_packs=False, + ) + + def test_pack_mode(self): + with mock.patch.object(gp, "generate_pack", _boom), \ + mock.patch.object(gp, "verify_and_finalize_packs", return_value=True): + with self.assertRaises(SystemExit) as ctx: + gp._run_platform_packs( + self._args(), [(["retropie"], "retropie")], {}, {}, {}, {}, {}, None + ) + self.assertEqual(ctx.exception.code, 1) + + def test_manifest_mode(self): + with mock.patch.object(gp, "generate_manifest", _boom): + with self.assertRaises(SystemExit) as ctx: + gp._run_manifest_mode( + self._args(), [(["retropie"], "retropie")], {}, {}, {}, {} + ) + self.assertEqual(ctx.exception.code, 1) + + +if __name__ == "__main__": + unittest.main()