mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-23 20:29:06 -05:00
fix: print pack and lock errors on stderr
The lock refusal, "no pack for", "no manifest for" and "no pack to verify" went to stdout, so a run redirected to a log exited 1 with nothing on the terminal. A test reads both scripts and refuses an ERROR line printed to stdout.
This commit is contained in:
1 parent
a160a5bc6b
commit
4fa598645e
4 files changed
+42
-12
No files matched your search
@@ -2547,13 +2547,13 @@ def _run_manifest_mode(
|
||||
_write_manifest_if_changed(alias_path, alias_manifest)
|
||||
print(f" {alias_path}: alias of {representative}")
|
||||
except (FileNotFoundError, OSError, yaml.YAMLError) as e:
|
||||
print(f" ERROR: {e}")
|
||||
print(f" ERROR: {e}", file=sys.stderr)
|
||||
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)))}")
|
||||
print(f"ERROR: no manifest for {', '.join(sorted(set(failed)))}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
@@ -2564,7 +2564,7 @@ def _pack_output_lock(output_dir: str, exclusive: bool = True) -> Iterator[None]
|
||||
with artifact_lock(output_dir, exclusive=exclusive):
|
||||
yield
|
||||
except ArtifactLockBusy as exc:
|
||||
print(f"ERROR: {exc}")
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
|
||||
@@ -2667,7 +2667,7 @@ def _run_verify_packs(args: argparse.Namespace) -> None:
|
||||
if not verified:
|
||||
# Every platform skipped is no check at all: a mistyped or purged
|
||||
# output directory read as a passed integrity check.
|
||||
print(f"ERROR: no pack to verify in {args.output_dir}")
|
||||
print(f"ERROR: no pack to verify in {args.output_dir}", file=sys.stderr)
|
||||
all_ok = False
|
||||
if not all_ok:
|
||||
sys.exit(1)
|
||||
@@ -2829,7 +2829,7 @@ def _run_platform_packs(
|
||||
pack_name=group_name,
|
||||
)
|
||||
except (FileNotFoundError, OSError, yaml.YAMLError) as e:
|
||||
print(f" ERROR: {e}")
|
||||
print(f" ERROR: {e}", file=sys.stderr)
|
||||
failed.append(representative)
|
||||
else:
|
||||
# A builder that declined (no system left under the target)
|
||||
@@ -2867,7 +2867,7 @@ def _run_platform_packs(
|
||||
if failed:
|
||||
# Verification only sees the ZIPs that exist, so a pack that was never
|
||||
# written does not fail it.
|
||||
print(f"ERROR: no pack for {', '.join(sorted(set(failed)))}")
|
||||
print(f"ERROR: no pack for {', '.join(sorted(set(failed)))}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
if not all_ok:
|
||||
sys.exit(1)
|
||||
|
||||
+2
-2
@@ -467,7 +467,7 @@ def main() -> None:
|
||||
try:
|
||||
held.enter_context(hold_artifact_lock(args.output_dir))
|
||||
except ArtifactLockBusy as exc:
|
||||
print(f"ERROR: {exc}")
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
all_ok = True
|
||||
@@ -655,7 +655,7 @@ def main() -> None:
|
||||
if stale:
|
||||
print(f"Purged {len(stale)} stale pack file(s) from {out_dir}/")
|
||||
except ArtifactLockBusy as exc:
|
||||
print(f"ERROR: {exc}")
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
pack_cmd = [
|
||||
|
||||
@@ -108,7 +108,7 @@ class PackLockCliTest(unittest.TestCase):
|
||||
]
|
||||
)
|
||||
self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
|
||||
self.assertIn("is in use by another run", proc.stdout)
|
||||
self.assertIn("is in use by another run", proc.stderr)
|
||||
|
||||
def test_every_writing_mode_refuses_a_locked_output(self):
|
||||
"""Two of seven modes took the lock, and five wrote straight into it.
|
||||
@@ -153,7 +153,7 @@ class PackLockCliTest(unittest.TestCase):
|
||||
]
|
||||
)
|
||||
self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
|
||||
self.assertIn("is in use by another run", proc.stdout)
|
||||
self.assertIn("is in use by another run", proc.stderr)
|
||||
|
||||
def test_pipeline_refuses_locked_output(self):
|
||||
with artifact_lock(self.dir):
|
||||
@@ -167,7 +167,7 @@ class PackLockCliTest(unittest.TestCase):
|
||||
]
|
||||
)
|
||||
self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
|
||||
self.assertIn("is in use by another run", proc.stdout)
|
||||
self.assertIn("is in use by another run", proc.stderr)
|
||||
|
||||
|
||||
class HeldLockTest(unittest.TestCase):
|
||||
|
||||
@@ -5192,7 +5192,7 @@ class ANamedPackThatIsAbsentFails(unittest.TestCase):
|
||||
cwd=ROOT, capture_output=True, text=True, timeout=600,
|
||||
)
|
||||
self.assertEqual(result.returncode, 1)
|
||||
self.assertIn("no pack to verify", result.stdout)
|
||||
self.assertIn("no pack to verify", result.stderr)
|
||||
|
||||
|
||||
|
||||
@@ -5945,5 +5945,35 @@ class HashInputNamesWhatItAccepts(unittest.TestCase):
|
||||
self.assertEqual(sha1_for_hash("md5", entry["md5"][:29], db), sha1)
|
||||
|
||||
|
||||
class ErrorsGoToStderr(unittest.TestCase):
|
||||
"""The lock refusal and "no pack for" went to stdout: a run redirected to
|
||||
a log showed nothing on the terminal and exited 1."""
|
||||
|
||||
@staticmethod
|
||||
def _leading_text(node: ast.expr) -> str:
|
||||
if isinstance(node, ast.Constant) and isinstance(node.value, str):
|
||||
return node.value
|
||||
if isinstance(node, ast.JoinedStr) and node.values:
|
||||
return ErrorsGoToStderr._leading_text(node.values[0])
|
||||
return ""
|
||||
|
||||
def test_no_error_line_is_printed_to_stdout(self):
|
||||
for script in ("generate_pack.py", "pipeline.py"):
|
||||
tree = ast.parse((ROOT / "scripts" / script).read_text(encoding="utf-8"))
|
||||
seen = 0
|
||||
for node in ast.walk(tree):
|
||||
if not (isinstance(node, ast.Call) and getattr(node.func, "id", "") == "print"
|
||||
and node.args):
|
||||
continue
|
||||
text = self._leading_text(node.args[0])
|
||||
# Detail lines under a pack's own verdict stay with it.
|
||||
if not text.lstrip().startswith("ERROR") or text.startswith(" "):
|
||||
continue
|
||||
seen += 1
|
||||
with self.subTest(script=script, line=node.lineno):
|
||||
self.assertIn("file", [k.arg for k in node.keywords])
|
||||
self.assertGreater(seen, 0, script)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user