Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatter test setup and ignore condition #132

Merged
merged 2 commits into from
Jan 20, 2024

Conversation

phelrine
Copy link
Contributor

While extending the parser functionality, I encountered some issues with the existing formatter test suite:

  1. The formatter_test.go was sharing a formatEnvironment across test cases. This led to state-related bugs due to shared state between tests. Now, a new formatEnvironment is created for each test to ensure independence and prevent errors that could arise from shared state.

  2. There was a typo in the .ignore file naming (003.ignroe), and the ignore condition logic was incorrect. The tests incorrectly reported failures when an .ignore file was present and an output mismatch occurred. This has been remedied so that now, presence of an .ignore file will result in the output being logged instead of causing a failing test.

This commit addresses the issue where the test logic wrongfully inverted the ignore rule condition:

1. A file naming typo (`003.ignroe` corrected to `003.ignore`) has been fixed, ensuring `.ignore` files are now properly recognized. Previously, the misnamed file caused certain test cases to be compared against expected outputs when they should have been ignored.

2. The test logic within `formatter_test.go` that determined whether to ignore discrepancies based on `.ignore` file presence was inverted. Before the fix, the presence of an `.ignore` file led to a test error upon output mismatch, while its absence merely logged the difference without failing the test. The corrected logic now reflects the intended behavior: when an `.ignore` file is present, the differing outputs are logged for review rather than causing the test to fail.
This commit updates `formatter_test.go` to address an issue where each test case was not receiving a fresh `formatEnvironment`. Previously, a single `formatEnvironment` instance was reused across multiple test cases, leading to potential dependencies on prior tests' states, such as indentation levels, that could result in unintended test failures.
@mattn
Copy link
Member

mattn commented Jan 20, 2024

thank you

@mattn mattn merged commit d0751d8 into sqls-server:master Jan 20, 2024
1 check passed
@phelrine phelrine deleted the fix-formatter-test branch January 20, 2024 08:28
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.

2 participants