Copilot commented on code in PR #13656:
URL: https://github.com/apache/trafficserver/pull/13656#discussion_r3968159721
##########
tools/hrw4u/tests/test_common.py:
##########
@@ -164,14 +164,21 @@ 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)."""
+ def test_error_collector_reports_failure_to_caller(self, capsys):
+ """generate_output reports errors via its return value; run_main owns
the exit status."""
errors = ErrorCollector()
errors.add_error(Hrw4uSyntaxError("<test>", 1, 0, "parse failed",
"bad"))
args = SimpleNamespace(ast=False, debug=False, no_comments=False)
- with pytest.raises(SystemExit) as exc_info:
- generate_output(None, None, HRW4UVisitor, "<test>", args, errors)
- assert exc_info.value.code == 1
+
+ assert generate_output(None, None, HRW4UVisitor, "<test>", args,
errors) is True
+
+ def test_clean_input_reports_no_failure(self, capsys):
+ """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(); }', "<test>", hrw4uLexer, hrw4uParser, "hrw4u",
collect_errors=True)
+ args = SimpleNamespace(ast=False, debug=False, no_comments=False)
+
+ assert generate_output(tree, parser_obj, HRW4UVisitor, "<test>", args,
errors) is False
Review Comment:
`capsys` is unused in these new tests, and `generate_output(...,
parser_obj=None, ...)` relies on current control flow to avoid touching
`parser_obj`. To make the tests more robust (and less misleading), either (a)
remove the unused `capsys` fixture, and/or (b) pass a minimal stub/mock for
`parser_obj` (or a typed cast) so a future refactor that touches `parser_obj`
doesn’t turn this test into an unexpected `AttributeError`.
##########
tools/hrw4u/tests/test_cli.py:
##########
@@ -244,3 +244,53 @@ 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
Review Comment:
These assertions depend on the exact text shape of diagnostics (e.g., `\":
error:\"`), which can vary with `--error-format` defaults. To reduce
brittleness, consider invoking the CLI in tests with an explicit
`--error-format plain` (or whichever format the assertions expect) so the tests
remain stable if the default output format changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
