You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "masaori335 (via GitHub)" <gi...@apache.org> on 2026/09/10 07:29:52 UTC
[PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
masaori335 opened a new pull request, #13665:
URL: https://github.com/apache/trafficserver/pull/13665
ast_visitor.py is test-only today, and it cannot reproduce the file it parsed. Four things are dropped, each of which changes either the emitted .conf or the location a diagnostic can name.
A span names a file, line and column rather than a line alone. Procedure inlining splices nodes in from a second file, so a line number can no longer say where a diagnostic belongs. Node.line is replaced outright rather than kept as a property over span: nothing outside the tests reads it, and keeping it would let new code go on producing locations that cannot name their own file.
Comments, because five .conf goldens carry # comments at exact positions.
Parentheses, because a parenthesized factor emits cond %{GROUP}.
Set and iprange operands, which until now both became a bare tuple: `in [1.2.3.4]` and `in {1.2.3.4}` produced byte-identical ASTs. They are not interchangeable -- visitor.py emits `lhs (...)` for a set and `lhs {...}` for an iprange, and only the set path is sandbox-checked. Each keeps the source text the emitter already uses, so SetValue.raw matches visitor.py's set_().getText()[1:-1] and IpRangeValue.raw its iprange().getText().
Target.from_dotted guessed a namespace/field split that no caller wanted; every table lookup matches the whole dotted name.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5628946363
[approve ci autest 2of4]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3977070709
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -67,22 +87,22 @@ def _visit_section(self, ctx) -> VarSection | Section:
return self._visit_var_section(ctx.sessionVarSection(), "session")
name = ctx.name.text
body = self._visit_body(ctx.sectionBody())
Review Comment:
It's pre-existing code. Out of scope from this PR.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5863059970
Rebased on the latest master (`738cd651b3`). This is ready for review.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5627760294
[approve ci autest osx freebsd clang-analyzer]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5615513914
[approve ci centos]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976850168
##########
tools/hrw4u/tests/test_ast_nodes.py:
##########
@@ -15,27 +15,31 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from hrw4u.ast_nodes import Target
+import pytest
+from hrw4u.ast_nodes import Span
-class TestTarget:
- def test_dotted_path(self):
- t = Target.from_dotted("inbound.req.X-Foo")
- assert t.namespace == "inbound.req"
- assert t.field == "X-Foo"
+class TestSpan:
- def test_two_segments(self):
- t = Target.from_dotted("inbound.ip")
- assert t.namespace == "inbound"
- assert t.field == "ip"
+ def test_equality_is_by_value(self):
+ assert Span(file="a", line=1, column=0) == Span(file="a", line=1, column=0)
+ assert Span(file="a", line=1, column=0) != Span(file="b", line=1, column=0)
- def test_no_dots(self):
- t = Target.from_dotted("bool_0")
- assert t.namespace is None
- assert t.field == "bool_0"
+ def test_is_hashable_so_it_can_key_a_span_index(self):
+ first = Span(file="a", line=1, column=0)
+ assert {first: "node"}[Span(file="a", line=1, column=0)] == "node"
- def test_deep_namespace(self):
- t = Target.from_dotted("http.cntl.TXN_DEBUG")
- assert t.namespace == "http.cntl"
- assert t.field == "TXN_DEBUG"
+ def test_is_immutable(self):
+ span = Span(file="a", line=1, column=0)
+ with pytest.raises(Exception):
+ span.line = 2
+
+ def test_rejects_unknown_attributes(self):
+ # slots=True: a typo'd field must fail loudly rather than sit unread on the instance.
+ with pytest.raises(AttributeError):
+ Span(file="a", line=1, column=0).lineno = 2
+
+
+if __name__ == "__main__":
+ pytest.main([__file__, "-v"])
Review Comment:
Removed by 04133e312884032376e56d402ca602ebb784fe9e
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976835730
##########
tools/hrw4u/src/ast_nodes.py:
##########
@@ -119,11 +127,16 @@ class Break(Node):
pass
+@dataclass(frozen=True, kw_only=True)
+class Comment(Node):
+ text: str
+
+
@dataclass(frozen=True, kw_only=True)
class Comparison(Node):
left: IdentValue | FunctionCall
operator: str # "==", "!=", ">", "<", "~", "!~", "in", "!in"
- right: ValueExpr | RegexValue | tuple[ValueExpr, ...]
+ right: ValueExpr | RegexValue | SetValue
modifiers: tuple[str, ...]
Review Comment:
Same as above.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976832189
##########
tools/hrw4u/src/ast_nodes.py:
##########
@@ -77,33 +81,37 @@ class RegexValue:
raw: str
-ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, int, bool, tuple[IPValue, ...]]
+@dataclass(frozen=True, kw_only=True)
+class SetValue:
+ """An `in [...]` operand. Emitted as `(raw)`, so the brackets are stripped but quoting is not."""
+ raw: str
@dataclass(frozen=True, kw_only=True)
-class Node:
- line: int
+class IpRangeValue:
+ """An `in {...}` operand. Emitted verbatim, braces included."""
+ raw: str
-@dataclass(frozen=True)
-class Target:
- namespace: str | None
- field: str
+ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, int, bool, IpRangeValue]
Review Comment:
Not taking the shared type, because the asymmetry tracks the grammar rather than being an accident of modelling.
`hrw4u.g4`'s `value` rule admits `iprange` but not `set_`:
```
value
: number=NUMBER | str=STRING | TRUE | FALSE
| ident=IDENT | ip | iprange | paramRef
;
```
So an iprange is legal anywhere a value is — `_extract_value` returns `IpRangeValue` for a plain `value` context — while a set is only reachable after `in`. Folding both into `MembershipValue` and using it for `Comparison.right` would make the type admit a set in positions the grammar rejects, and would drop `IpRangeValue` from the one alias that correctly describes where it can appear.
The confusion you're naming is real though — nothing at the definition said why. Added a comment there:
```python
# IpRangeValue is a ValueExpr but SetValue is not, because the grammar's `value` rule admits
# `iprange` and not `set_`: an iprange is legal anywhere a value is, a set only after `in`.
```
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976838488
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -39,26 +55,26 @@ def visitProgram(self, ctx) -> HRW4UAST:
elif item.section() is not None:
items.append(self._visit_section(item.section()))
elif item.commentLine() is not None:
- pass
+ items.append(self._visit_comment(item.commentLine()))
else:
raise ValueError(f"Unhandled programItem alternative at line {item.start.line}")
Review Comment:
Fixed by 04133e312884032376e56d402ca602ebb784fe9e
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5692643694
I found other AST bugs, so please review this first. https://github.com/apache/trafficserver/pull/13699
I'll rebase this PR based on the AST fix.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976845372
##########
tools/hrw4u/tests/test_ast_nodes.py:
##########
@@ -15,27 +15,31 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from hrw4u.ast_nodes import Target
+import pytest
+from hrw4u.ast_nodes import Span
-class TestTarget:
- def test_dotted_path(self):
- t = Target.from_dotted("inbound.req.X-Foo")
- assert t.namespace == "inbound.req"
- assert t.field == "X-Foo"
+class TestSpan:
- def test_two_segments(self):
- t = Target.from_dotted("inbound.ip")
- assert t.namespace == "inbound"
- assert t.field == "ip"
+ def test_equality_is_by_value(self):
+ assert Span(file="a", line=1, column=0) == Span(file="a", line=1, column=0)
+ assert Span(file="a", line=1, column=0) != Span(file="b", line=1, column=0)
- def test_no_dots(self):
- t = Target.from_dotted("bool_0")
- assert t.namespace is None
- assert t.field == "bool_0"
+ def test_is_hashable_so_it_can_key_a_span_index(self):
+ first = Span(file="a", line=1, column=0)
+ assert {first: "node"}[Span(file="a", line=1, column=0)] == "node"
- def test_deep_namespace(self):
- t = Target.from_dotted("http.cntl.TXN_DEBUG")
- assert t.namespace == "http.cntl"
- assert t.field == "TXN_DEBUG"
+ def test_is_immutable(self):
+ span = Span(file="a", line=1, column=0)
+ with pytest.raises(Exception):
+ span.line = 2
Review Comment:
Fixed by 04133e312884032376e56d402ca602ebb784fe9e
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976838488
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -39,26 +55,26 @@ def visitProgram(self, ctx) -> HRW4UAST:
elif item.section() is not None:
items.append(self._visit_section(item.section()))
elif item.commentLine() is not None:
- pass
+ items.append(self._visit_comment(item.commentLine()))
else:
raise ValueError(f"Unhandled programItem alternative at line {item.start.line}")
Review Comment:
Fixed by 04133e312884032376e56d402ca602ebb784fe9e
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -39,26 +55,26 @@ def visitProgram(self, ctx) -> HRW4UAST:
elif item.section() is not None:
items.append(self._visit_section(item.section()))
elif item.commentLine() is not None:
- pass
+ items.append(self._visit_comment(item.commentLine()))
else:
raise ValueError(f"Unhandled programItem alternative at line {item.start.line}")
Review Comment:
Fixed by 04133e312884032376e56d402ca602ebb784fe9e
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976990729
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -67,22 +87,22 @@ def _visit_section(self, ctx) -> VarSection | Section:
return self._visit_var_section(ctx.sessionVarSection(), "session")
name = ctx.name.text
body = self._visit_body(ctx.sectionBody())
Review Comment:
`_visit_body` iterates over a list of body-item contexts (with `.statement()/.conditional()/.commentLine()` access). Passing `ctx.sectionBody()` (a rule context) is inconsistent with how `_visit_body` is used elsewhere and can break traversal depending on the generated parser API. Fix by passing the section body's item list (e.g., `ctx.sectionBody().blockItem()` or the specific `sectionBodyItem()` list from the grammar) so `_visit_body` always receives homogeneous body-item contexts.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5627930887
[approve ci freebsd]
[approve ci clang-analyzer]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5628118074
[approve ci clang-analyzer]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#issuecomment-5619148909
[approve ci autest 0of4 1of4 2of4]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: record spans, comments and operand forms in the AST [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13665:
URL: https://github.com/apache/trafficserver/pull/13665#discussion_r3976746401
##########
tools/hrw4u/src/ast_visitor.py:
##########
@@ -39,26 +55,26 @@ def visitProgram(self, ctx) -> HRW4UAST:
elif item.section() is not None:
items.append(self._visit_section(item.section()))
elif item.commentLine() is not None:
- pass
+ items.append(self._visit_comment(item.commentLine()))
else:
raise ValueError(f"Unhandled programItem alternative at line {item.start.line}")
Review Comment:
The new AST model is file/line/column-aware via `Span`, but these internal error messages still only mention `line`. Including `self.filename` and `item.start.column` (and similarly for the other `Unhandled ... at line ...` raises) would make failures much easier to pinpoint when parsing/inlining across files.
##########
tools/hrw4u/src/ast_nodes.py:
##########
@@ -77,33 +81,37 @@ class RegexValue:
raw: str
-ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, int, bool, tuple[IPValue, ...]]
+@dataclass(frozen=True, kw_only=True)
+class SetValue:
+ """An `in [...]` operand. Emitted as `(raw)`, so the brackets are stripped but quoting is not."""
+ raw: str
@dataclass(frozen=True, kw_only=True)
-class Node:
- line: int
+class IpRangeValue:
+ """An `in {...}` operand. Emitted verbatim, braces included."""
+ raw: str
-@dataclass(frozen=True)
-class Target:
- namespace: str | None
- field: str
+ValueExpr = Union[LiteralStringValue, IdentValue, IPValue, ParamRef, int, bool, IpRangeValue]
Review Comment:
The typing/modeling of membership RHS is asymmetric: `IpRangeValue` is treated as a general `ValueExpr`, while `SetValue` is a separate special-case type in `Comparison.right`. If these are both membership-only operands, consider introducing a shared type (e.g., `MembershipValue = SetValue | IpRangeValue`) and using that in `Comparison.right` (and in the visitor return hints). This keeps the API surface more uniform and reduces “why is iprange a value but set is not?” confusion for future maintainers.
##########
tools/hrw4u/src/ast_nodes.py:
##########
@@ -119,11 +127,16 @@ class Break(Node):
pass
+@dataclass(frozen=True, kw_only=True)
+class Comment(Node):
+ text: str
+
+
@dataclass(frozen=True, kw_only=True)
class Comparison(Node):
left: IdentValue | FunctionCall
operator: str # "==", "!=", ">", "<", "~", "!~", "in", "!in"
- right: ValueExpr | RegexValue | tuple[ValueExpr, ...]
+ right: ValueExpr | RegexValue | SetValue
modifiers: tuple[str, ...]
Review Comment:
The typing/modeling of membership RHS is asymmetric: `IpRangeValue` is treated as a general `ValueExpr`, while `SetValue` is a separate special-case type in `Comparison.right`. If these are both membership-only operands, consider introducing a shared type (e.g., `MembershipValue = SetValue | IpRangeValue`) and using that in `Comparison.right` (and in the visitor return hints). This keeps the API surface more uniform and reduces “why is iprange a value but set is not?” confusion for future maintainers.
##########
tools/hrw4u/tests/test_ast_nodes.py:
##########
@@ -15,27 +15,31 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from hrw4u.ast_nodes import Target
+import pytest
+from hrw4u.ast_nodes import Span
-class TestTarget:
- def test_dotted_path(self):
- t = Target.from_dotted("inbound.req.X-Foo")
- assert t.namespace == "inbound.req"
- assert t.field == "X-Foo"
+class TestSpan:
- def test_two_segments(self):
- t = Target.from_dotted("inbound.ip")
- assert t.namespace == "inbound"
- assert t.field == "ip"
+ def test_equality_is_by_value(self):
+ assert Span(file="a", line=1, column=0) == Span(file="a", line=1, column=0)
+ assert Span(file="a", line=1, column=0) != Span(file="b", line=1, column=0)
- def test_no_dots(self):
- t = Target.from_dotted("bool_0")
- assert t.namespace is None
- assert t.field == "bool_0"
+ def test_is_hashable_so_it_can_key_a_span_index(self):
+ first = Span(file="a", line=1, column=0)
+ assert {first: "node"}[Span(file="a", line=1, column=0)] == "node"
- def test_deep_namespace(self):
- t = Target.from_dotted("http.cntl.TXN_DEBUG")
- assert t.namespace == "http.cntl"
- assert t.field == "TXN_DEBUG"
+ def test_is_immutable(self):
+ span = Span(file="a", line=1, column=0)
+ with pytest.raises(Exception):
+ span.line = 2
Review Comment:
Catching a broad `Exception` can let unrelated failures pass (e.g., if an unexpected runtime error is raised). Since `Span` is a frozen dataclass, this can be made stricter by asserting the specific exception type raised for frozen instances (from `dataclasses`), which improves signal if the dataclass configuration changes.
##########
tools/hrw4u/tests/test_ast_nodes.py:
##########
@@ -15,27 +15,31 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from hrw4u.ast_nodes import Target
+import pytest
+from hrw4u.ast_nodes import Span
-class TestTarget:
- def test_dotted_path(self):
- t = Target.from_dotted("inbound.req.X-Foo")
- assert t.namespace == "inbound.req"
- assert t.field == "X-Foo"
+class TestSpan:
- def test_two_segments(self):
- t = Target.from_dotted("inbound.ip")
- assert t.namespace == "inbound"
- assert t.field == "ip"
+ def test_equality_is_by_value(self):
+ assert Span(file="a", line=1, column=0) == Span(file="a", line=1, column=0)
+ assert Span(file="a", line=1, column=0) != Span(file="b", line=1, column=0)
- def test_no_dots(self):
- t = Target.from_dotted("bool_0")
- assert t.namespace is None
- assert t.field == "bool_0"
+ def test_is_hashable_so_it_can_key_a_span_index(self):
+ first = Span(file="a", line=1, column=0)
+ assert {first: "node"}[Span(file="a", line=1, column=0)] == "node"
- def test_deep_namespace(self):
- t = Target.from_dotted("http.cntl.TXN_DEBUG")
- assert t.namespace == "http.cntl"
- assert t.field == "TXN_DEBUG"
+ def test_is_immutable(self):
+ span = Span(file="a", line=1, column=0)
+ with pytest.raises(Exception):
+ span.line = 2
+
+ def test_rejects_unknown_attributes(self):
+ # slots=True: a typo'd field must fail loudly rather than sit unread on the instance.
+ with pytest.raises(AttributeError):
+ Span(file="a", line=1, column=0).lineno = 2
+
+
+if __name__ == "__main__":
+ pytest.main([__file__, "-v"])
Review Comment:
This `__main__` runner is unusual in repository test files and can create inconsistent execution paths vs the normal test runner (and can confuse tooling that imports tests). If there isn’t a strong repo convention for these blocks, it’s typically better to remove it and rely on the standard test invocation.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org