diff --git a/doc/admin-guide/configuration/hrw4u.en.rst b/doc/admin-guide/configuration/hrw4u.en.rst index 4a0fb3f6961..f7b8510ec08 100644 --- a/doc/admin-guide/configuration/hrw4u.en.rst +++ b/doc/admin-guide/configuration/hrw4u.en.rst @@ -115,6 +115,23 @@ This is particularly useful for build systems or when processing many configurat files at once. All files are processed in a single invocation, improving performance for large batches of files. +Exit Status +^^^^^^^^^^^ + +====== ========================================================================== +Status Meaning +====== ========================================================================== +0 Every input compiled. Warnings may still have been reported. +1 At least one input had an error, or the command line was invalid. +====== ========================================================================== + +A compile error does not stop the run: every input is still processed before +the status is decided, so one bad file in a multi-file or bulk run does not +skip the files after it. Fatal problems outside the compile itself, such as an +invalid command line, a missing or unreadable input, or an unwritable output, +still abort immediately. A failing compile writes its partial output; the exit +status is what marks that output untrustworthy. + Reverse Tool (u4wrh) ^^^^^^^^^^^^^^^^^^^^ diff --git a/tools/hrw4u/src/common.py b/tools/hrw4u/src/common.py index 15f1885f4bd..22ee24e7939 100644 --- a/tools/hrw4u/src/common.py +++ b/tools/hrw4u/src/common.py @@ -238,8 +238,12 @@ def generate_output( filename: str, args: Any, error_collector: ErrorCollector | None = None, - extra_kwargs: dict[str, Any] | None = None) -> None: - """Generate and print output based on mode with optional error collection.""" + extra_kwargs: dict[str, Any] | None = None) -> bool: + """Generate and print output based on mode with optional error collection. + + Returns True when the input produced errors, so the caller can set the exit + status after every input has been processed rather than aborting mid-run. + """ if args.ast: if tree is not None: print(tree.toStringTree(recog=parser_obj)) @@ -278,8 +282,8 @@ def generate_output( if error_collector and (error_collector.has_errors() or error_collector.has_warnings()): print(error_collector.get_error_summary(), file=sys.stderr) - if error_collector.has_errors() and not args.ast and tree is None: - sys.exit(1) + + return bool(error_collector and error_collector.has_errors()) def run_main( @@ -363,10 +367,12 @@ def run_main( emit_fatal_error(args.error_format, e) tree, parser_obj, error_collector = create_parse_tree( content, filename, lexer_class, parser_class, error_prefix, not args.stop_on_error, args.max_errors, args.error_format) - generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs) + if generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs): + sys.exit(1) return if any(':' in f for f in args.files): + failed = False for pair in args.files: if ':' not in pair: emit_fatal_message( @@ -398,12 +404,13 @@ def run_main( original_stdout = sys.stdout try: sys.stdout = output_file - generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs) + failed |= generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs) finally: sys.stdout = original_stdout except Exception as e: emit_fatal_message(args.error_format, f"Error writing to '{output_path}': {e}", filename=output_path) else: + failed = False for i, input_path in enumerate(args.files): if i > 0: print("# ---") @@ -426,4 +433,7 @@ def run_main( content, filename, lexer_class, parser_class, error_prefix, not args.stop_on_error, args.max_errors, args.error_format) - generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs) + failed |= generate_output(tree, parser_obj, visitor_class, filename, args, error_collector, extra_kwargs) + + if failed: + sys.exit(1) diff --git a/tools/hrw4u/tests/test_cli.py b/tools/hrw4u/tests/test_cli.py index 886728de052..ff2cfa95d8d 100644 --- a/tools/hrw4u/tests/test_cli.py +++ b/tools/hrw4u/tests/test_cli.py @@ -48,6 +48,14 @@ def run_hrw4u(args: list[str], stdin: str | None = None) -> subprocess.Completed return subprocess.run(cmd, capture_output=True, text=True, input=stdin, cwd=Path.cwd()) +def run_u4wrh(args: list[str], stdin: str | None = None) -> subprocess.CompletedProcess: + """Run u4wrh script with given arguments.""" + script = Path("scripts/u4wrh").resolve() + cmd = [sys.executable, str(script)] + args + + return subprocess.run(cmd, capture_output=True, text=True, input=stdin, cwd=Path.cwd()) + + def test_cli_single_file_to_stdout(sample_hrw4u_files: tuple[Path, Path, Path]) -> None: """Test compiling a single file to stdout.""" file1, _, _ = sample_hrw4u_files @@ -244,3 +252,64 @@ def test_cli_help_lists_error_format_flag() -> None: assert "--error-format" in result.stdout for choice in ("plain", "json", "markdown"): assert choice in result.stdout + + +# +# Exit-code contract: a compile error must fail the build. +# + + +def test_cli_exits_nonzero_on_syntax_error(tmp_path: Path) -> None: + """A syntax error must exit non-zero even though ANTLR recovers and yields a tree.""" + bad = tmp_path / "bad.hrw4u" + bad.write_text("REMAP {\n inbound.req.X-Foo = \n}\n") + + result = run_hrw4u([str(bad)]) + + assert result.returncode != 0 + assert ": error:" in result.stderr + + +def test_cli_exits_nonzero_on_semantic_error(tmp_path: Path) -> None: + """A semantic error must exit non-zero; the parse tree exists, so only sema catches it.""" + bad = tmp_path / "bad.hrw4u" + bad.write_text("REMAP {\n test::add-debug-header(\"foo\");\n}\n") + + result = run_hrw4u([str(bad)]) + + assert result.returncode != 0 + assert "unknown procedure" in result.stderr + + +def test_cli_collects_all_errors_and_still_exits_nonzero(tmp_path: Path) -> None: + """Multi-error mode must report every diagnostic AND fail; the two are not exclusive.""" + bad = tmp_path / "bad.hrw4u" + bad.write_text("REMAP {\n bogus.one = \"a\";\n bogus.two = \"b\";\n}\n") + + result = run_hrw4u([str(bad)]) + + assert result.returncode != 0 + assert result.stderr.count(": error:") >= 2 + + +def test_cli_multi_file_exits_nonzero_if_any_fails(sample_hrw4u_files: tuple[Path, Path, Path], tmp_path: Path) -> None: + """One bad file among good ones fails the run, but the good ones are still processed.""" + good, _, _ = sample_hrw4u_files + bad = tmp_path / "bad.hrw4u" + bad.write_text("REMAP {\n test::nope(\"x\");\n}\n") + + result = run_hrw4u([str(bad), str(good)]) + + assert result.returncode != 0 + assert "no-op" in result.stdout, "processing must continue past the failing file" + + +def test_cli_u4wrh_exits_nonzero_on_error(tmp_path: Path) -> None: + """u4wrh shares run_main(), so it must honor the same exit-status contract.""" + bad = tmp_path / "bad.conf" + bad.write_text("cond %{READ_REQUEST_HDR_HOOK}\n set-header X-Foo\n") + + result = run_u4wrh([str(bad)]) + + assert result.returncode != 0 + assert ": error:" in result.stderr diff --git a/tools/hrw4u/tests/test_common.py b/tools/hrw4u/tests/test_common.py index d17cdf6ad5d..1997f28cbed 100644 --- a/tools/hrw4u/tests/test_common.py +++ b/tools/hrw4u/tests/test_common.py @@ -164,14 +164,26 @@ def test_ast_mode_tree_none_with_errors(self, capsys): out = capsys.readouterr().out assert "Parse tree not available" in out - def test_error_collector_exits_on_parse_failure(self, capsys): - """When tree is None and errors exist in non-AST mode, should exit(1).""" - errors = ErrorCollector() - errors.add_error(Hrw4uSyntaxError("", 1, 0, "parse failed", "bad")) + def test_error_collector_reports_failure_to_caller(self): + """generate_output reports errors via its return value; run_main owns the exit status. + + The input parses, so ``tree`` is not None -- the exact shape the old + ``tree is None`` exit gate let through. + """ + tree, parser_obj, errors = create_parse_tree( + 'REMAP { test::nope("x"); }', "", hrw4uLexer, hrw4uParser, "hrw4u", collect_errors=True) args = SimpleNamespace(ast=False, debug=False, no_comments=False) - with pytest.raises(SystemExit) as exc_info: - generate_output(None, None, HRW4UVisitor, "", args, errors) - assert exc_info.value.code == 1 + + assert tree is not None + assert generate_output(tree, parser_obj, HRW4UVisitor, "", args, errors) is True + + def test_clean_input_reports_no_failure(self): + """A clean parse must report False so a multi-file run keeps exit status 0.""" + tree, parser_obj, errors = create_parse_tree( + 'REMAP { no-op(); }', "", hrw4uLexer, hrw4uParser, "hrw4u", collect_errors=True) + args = SimpleNamespace(ast=False, debug=False, no_comments=False) + + assert generate_output(tree, parser_obj, HRW4UVisitor, "", args, errors) is False def test_visitor_exception_collected(self, capsys): """When visitor.visit() raises, error is collected and reported."""