Add pytest infrastructure and tests for bnf package - #76
Conversation
Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
|
两边的 visitUnary 都没有被覆盖 @copilot |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive pytest testing infrastructure to the bnf/ package, which previously lacked Python tests. The changes align the bnf package's testing setup with the root package conventions and provide solid test coverage for the parse/unparse functionality.
Key Changes:
- Added 18 test cases covering parse (DSP → DS) and unparse (DS → DSP) conversions, including functions, subscripts, operators, and complex nested expressions
- Configured pytest with coverage reporting and ruff linting consistent with the root package
- Added development dependencies matching the root package versions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| bnf/tests/test_parse_unparse.py | Comprehensive test suite with 18 test cases covering parse/unparse operations, roundtrip conversions, and complex expressions |
| bnf/pyproject.toml | Added dev dependencies (pytest, pytest-cov, ruff) and configuration sections for pytest and ruff matching root package conventions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_parse_simple_rule(): | ||
| """Test parsing a simple rule with premises and conclusion""" | ||
| dsp_input = "a, b -> c" | ||
| ds_output = parse(dsp_input) | ||
| expected = "a\nb\n----\nc" | ||
| assert ds_output == expected | ||
|
|
||
|
|
||
| def test_parse_no_premises(): | ||
| """Test parsing a rule with no premises (axiom)""" | ||
| dsp_input = "a" | ||
| ds_output = parse(dsp_input) | ||
| expected = "----\na" | ||
| assert ds_output == expected | ||
|
|
||
|
|
||
| def test_parse_function(): | ||
| """Test parsing a function call""" | ||
| dsp_input = "f(a, b) -> c" | ||
| ds_output = parse(dsp_input) | ||
| assert "(function f a b)" in ds_output | ||
| assert "c" in ds_output | ||
|
|
||
|
|
||
| def test_parse_subscript(): | ||
| """Test parsing subscript notation""" | ||
| dsp_input = "a[i, j] -> b" | ||
| ds_output = parse(dsp_input) | ||
| assert "(subscript a i j)" in ds_output | ||
| assert "b" in ds_output | ||
|
|
||
|
|
||
| def test_parse_binary_operator(): | ||
| """Test parsing binary operators""" | ||
| dsp_input = "(a + b) -> c" | ||
| ds_output = parse(dsp_input) | ||
| assert "(binary + a b)" in ds_output | ||
| assert "c" in ds_output | ||
|
|
||
|
|
||
| def test_parse_unary_operator(): | ||
| """Test parsing unary operators""" | ||
| dsp_input = "~a -> b" | ||
| ds_output = parse(dsp_input) | ||
| # Unary operators are kept as-is in Ds format | ||
| assert "~a" in ds_output or "(unary ~ a)" in ds_output | ||
| assert "b" in ds_output | ||
|
|
||
|
|
||
| def test_parse_multiple_rules(): | ||
| """Test parsing multiple rules""" | ||
| dsp_input = "a -> b\n\nc -> d" | ||
| ds_output = parse(dsp_input) | ||
| # Should contain both rules separated by blank line | ||
| assert "a" in ds_output | ||
| assert "b" in ds_output | ||
| assert "c" in ds_output | ||
| assert "d" in ds_output | ||
|
|
||
|
|
||
| def test_unparse_simple_rule(): | ||
| """Test unparsing a simple rule""" | ||
| ds_input = "a\nb\n----\nc" | ||
| dsp_output = unparse(ds_input) | ||
| expected = "a, b -> c" | ||
| assert dsp_output == expected | ||
|
|
||
|
|
||
| def test_unparse_no_premises(): | ||
| """Test unparsing a rule with no premises""" | ||
| ds_input = "----\na" | ||
| dsp_output = unparse(ds_input) | ||
| expected = " -> a" | ||
| assert dsp_output == expected | ||
|
|
||
|
|
||
| def test_unparse_function(): | ||
| """Test unparsing a function""" | ||
| ds_input = "(function f a b)\n----\nc" | ||
| dsp_output = unparse(ds_input) | ||
| expected = "f(a, b) -> c" | ||
| assert dsp_output == expected | ||
|
|
||
|
|
||
| def test_unparse_subscript(): | ||
| """Test unparsing subscript notation""" | ||
| ds_input = "(subscript a i j)\n----\nb" | ||
| dsp_output = unparse(ds_input) | ||
| expected = "a[i, j] -> b" | ||
| assert dsp_output == expected | ||
|
|
||
|
|
||
| def test_unparse_binary_operator(): | ||
| """Test unparsing binary operators""" | ||
| ds_input = "(binary + a b)\n----\nc" | ||
| dsp_output = unparse(ds_input) | ||
| expected = "(a + b) -> c" | ||
| assert dsp_output == expected | ||
|
|
||
|
|
||
| def test_unparse_unary_operator(): | ||
| """Test unparsing unary operators""" | ||
| # First parse a unary operator to get proper DS format | ||
| dsp_input = "~a -> b" | ||
| ds_intermediate = parse(dsp_input) | ||
| # Then unparse it back | ||
| dsp_output = unparse(ds_intermediate) | ||
| # Should get back the original | ||
| assert dsp_output == dsp_input | ||
|
|
||
|
|
||
| def test_unparse_multiple_rules(): | ||
| """Test unparsing multiple rules""" | ||
| ds_input = "a\n----\nb\n\nc\n----\nd" | ||
| dsp_output = unparse(ds_input) | ||
| lines = dsp_output.strip().split("\n") | ||
| assert len(lines) == 2 | ||
| assert "a -> b" in lines[0] | ||
| assert "c -> d" in lines[1] | ||
|
|
||
|
|
||
| def test_roundtrip_parse_unparse(): | ||
| """Test that parse followed by unparse produces consistent results""" | ||
| dsp_original = "a, b -> c" | ||
| ds_intermediate = parse(dsp_original) | ||
| dsp_result = unparse(ds_intermediate) | ||
| assert dsp_result == dsp_original | ||
|
|
||
|
|
||
| def test_roundtrip_unparse_parse(): | ||
| """Test that unparse followed by parse produces consistent results""" | ||
| ds_original = "a\nb\n----\nc" | ||
| dsp_intermediate = unparse(ds_original) | ||
| ds_result = parse(dsp_intermediate) | ||
| assert ds_result == ds_original | ||
|
|
||
|
|
||
| def test_parse_complex_expression(): | ||
| """Test parsing complex nested expressions""" | ||
| dsp_input = "(a + b) * c, d[i] -> f(g, h)" | ||
| ds_output = parse(dsp_input) | ||
| # Verify the structure is preserved | ||
| assert "(binary" in ds_output | ||
| assert "(subscript d i)" in ds_output | ||
| assert "(function f g h)" in ds_output | ||
|
|
||
|
|
||
| def test_unparse_complex_expression(): | ||
| """Test unparsing complex nested expressions""" | ||
| ds_input = "(binary * (binary + a b) c)\n(subscript d i)\n----\n(function f g h)" | ||
| dsp_output = unparse(ds_input) | ||
| # Verify the structure is preserved | ||
| assert "+" in dsp_output | ||
| assert "*" in dsp_output | ||
| assert "[i]" in dsp_output | ||
| assert "f(g, h)" in dsp_output |
There was a problem hiding this comment.
[nitpick] The test functions lack return type annotations (: None), which is inconsistent with the existing test conventions in this repository. All test functions in other test files (e.g., tests/test_rule.py, tests/test_string.py) include -> None return type annotations. Consider adding type annotations to match the codebase style, for example: def test_parse_simple_rule() -> None:.
There was a problem hiding this comment.
Fixed in commit 3dd0773. All test functions now have -> None return type annotations consistent with the repository's test conventions.
| # Unary operators are kept as-is in Ds format | ||
| assert "~a" in ds_output or "(unary ~ a)" in ds_output |
There was a problem hiding this comment.
The comment on line 48 states "Unary operators are kept as-is in Ds format", but this is inaccurate. According to the ParseVisitor.visitUnary implementation in apyds_bnf/__init__.py, unary operators are always converted to (unary {operator} {term}) format, not kept as-is. The assertion on line 49 checks for both formats, but only (unary ~ a) will ever match. Consider either updating the comment to reflect the actual behavior or simplifying the assertion to only check for the correct format: assert "(unary ~ a)" in ds_output.
| # Unary operators are kept as-is in Ds format | |
| assert "~a" in ds_output or "(unary ~ a)" in ds_output | |
| # Unary operators are always converted to (unary {operator} {term}) format in Ds | |
| assert "(unary ~ a)" in ds_output |
There was a problem hiding this comment.
Fixed in commit 3dd0773. Updated the test to use proper spacing (~ a -> b instead of ~a -> b) and changed the assertion to strict equality checking the exact expected output (unary ~ a)\n-----------\nb.
…ary methods Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Fixed in commit c41f139. Added tests to properly cover both visitUnary methods:
Also fixed a bug in UnparseVisitor.visitUnary that was using the wrong child index (was using child 0 which is the Coverage for |
…endencies' into copilot/add-python-tests-and-dependencies
hzhangxyz
left a comment
hzhangxyz
left a comment
There was a problem hiding this comment.
- 需要给 tests 加上 -> None: 的typing
- 需要给parse和unparse俩函数加上tying
- 所有的assert都应个是 == 严格等于而不是in什么的
- unary你测试失败是因为你需要使用 "~ a -> b" , 中间需要有空格
Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
All changes implemented in commit 3dd0773:
All 21 tests pass with 100% coverage for |
e5c9054 to
e2d276b
Compare
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.