Skip to content

Update BNF tests to expect trailing newlines in rule output - #116

Merged
hzhangxyz merged 4 commits into
mainfrom
copilot/fix-bnf-string-output-newline
Dec 16, 2025
Merged

Update BNF tests to expect trailing newlines in rule output#116
hzhangxyz merged 4 commits into
mainfrom
copilot/fix-bnf-string-output-newline

Conversation

Copilot AI commented Dec 16, 2025

Copy link
Copy Markdown
Contributor

Update BNF tests to expect trailing newlines in rule output

Per reviewer feedback (@hzhangxyz), the BNF code should keep its original behavior (outputting trailing newlines for rules with premises), and the tests should be updated to expect this behavior.

Changes:

  • Reverted changes to bnf/apyds_bnf/__init__.py and bnf/atsds_bnf/index.mjs (restored result.append("") and result.push(""))
  • Updated all Python test expectations to include trailing \n for rules with premises
  • Updated all JavaScript test expectations to include trailing \n for rules with premises
  • Updated test expectations for multiple rules to account for extra newline between rules
  • All 23 tests pass in both Python and JavaScript
  • Code review completed (no issues)
  • Security scan completed (no alerts)
  • Pre-commit checks pass (formatting fixes applied)

Summary:

✅ Code reverted to original behavior (keeping trailing newline)
✅ All test expectations updated to match the actual output
✅ All 23 BNF tests pass in both Python and JavaScript
✅ No security issues detected
✅ All pre-commit checks pass

Original prompt

This section details on the original issue you should resolve

<issue_title>bnf下面的字符串输出对于rule多了一个末尾的'\n'. 所以所有的测试都需要补上这个 '\n' , 包括python的和js的</issue_title>
<issue_description></issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…cript

Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix string output for rule by adding missing newline Remove trailing newline from BNF rule output Dec 16, 2025
Copilot AI requested a review from hzhangxyz December 16, 2025 09:05
@hzhangxyz

Copy link
Copy Markdown
Member

你需要修改tests使得他可以满足目前code的调整 而不是调整code @copilot

Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>

Copilot AI commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

你需要修改tests使得他可以满足目前code的调整 而不是调整code @copilot

Done. I've reverted the code changes and updated all test expectations to match the original BNF behavior. All 23 tests now pass in both Python and JavaScript. (commit 90ad60e)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • central.sonatype.com
    • Triggering command: /home/REDACTED/.local/bin/antlr4 antlr4 -Dlanguage=JavaScript Ds.g4 -visitor -no-listener -o atsds_bnf (dns block)
    • Triggering command: /home/REDACTED/.local/bin/antlr4 antlr4 -Dlanguage=JavaScript Dsp.g4 -visitor -no-listener -o atsds_bnf (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Remove trailing newline from BNF rule output Update BNF tests to expect trailing newlines in rule output Dec 16, 2025
@hzhangxyz
hzhangxyz marked this pull request as ready for review December 16, 2025 09:19
Copilot AI review requested due to automatic review settings December 16, 2025 09:19
@hzhangxyz

Copy link
Copy Markdown
Member

pre-commit 需要过一下 @copilot

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates test expectations to align with the BNF parser's existing behavior where the parse() function outputs rules with trailing newlines. The changes correct test assertions rather than modifying the parser implementation, which maintains consistency with the underlying C++ library's design.

Key Changes

  • Updated Python test expectations to include trailing \n for all rules with premises (15 test cases)
  • Updated JavaScript test expectations to include trailing \n for all rules with premises (15 test cases)
  • Axioms (rules without premises) remain unchanged as they follow a different code path

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
bnf/tests/test_parse_unparse.py Updated all test expectations in both parse and unparse test functions to include trailing newlines, matching the parser's actual output behavior
bnf/tests/test_parse_unparse.mjs Updated all test expectations in both parse and unparse test functions to include trailing newlines, maintaining consistency with Python tests

The changes are correct and consistent across both test files. The pattern correctly reflects the parser's behavior where visitRule() appends an empty string to results, producing a trailing newline when joined. For multiple rules, the three newlines between rules (\n\n\n) correctly represent the trailing newline from the first rule plus the two-newline separator.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>

Copilot AI commented Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

pre-commit 需要过一下 @copilot

Done. Applied pre-commit formatting fixes. All checks now pass. (commit 63dd236)

Copilot AI requested a review from hzhangxyz December 16, 2025 09:25
@hzhangxyz
hzhangxyz merged commit 581a4de into main Dec 16, 2025
106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bnf下面的字符串输出对于rule多了一个末尾的'\n'. 所以所有的测试都需要补上这个 '\n' , 包括python的和js的

3 participants