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

Refactor default parser test #294

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

sigee
Copy link
Contributor

@sigee sigee commented Jul 29, 2024

  • Satisfy the TODO comment
  • Fill out the missing cases
    • Long option quote handling DEFAULT behavior
    • Short option quote handling DEFAULT behavior
    • Short option concatenated quote handling WITH strip
    • Short option concatenated quote handling WITHOUT strip

@garydgregory
Copy link
Member

This PR has an empty description. Please tell us what this fixes.

@sigee
Copy link
Contributor Author

sigee commented Jul 30, 2024

The main purpose was to satisfy the TODO comment in the code. "TODO Needs a rework using JUnit parameterized tests."
While moving the test cases into the argument provider, I also filled out some missing cases, because in the provider it is more conspicuous which cases are covered and which are not.
Unfortunatelly the parent abstract class also requires a refactor, to eliminate the two disabled inherrited test cases, because their behavior differs from the other parsers, but could be covered by the parameterized test.

@garydgregory
Copy link
Member

@sigee
Thank you for your PR.
Please run mvn before you push and fix any build failures.

@sigee sigee force-pushed the refactor-DefaultParserTest branch from f3ab157 to 4b95700 Compare August 5, 2024 14:46
@sigee
Copy link
Contributor Author

sigee commented Aug 5, 2024

@garydgregory
I usually do run verification at least before the pull request, but this time I missed it for some reason I couldn't remember.
Anyway, I fixed the checkstyle issues.

@sigee sigee force-pushed the refactor-DefaultParserTest branch from 4b95700 to aa77a92 Compare August 5, 2024 17:15
@sigee sigee force-pushed the refactor-DefaultParserTest branch from aa77a92 to 71d3cd3 Compare October 18, 2024 14:06
@garydgregory garydgregory merged commit 3bac60a into apache:master Oct 18, 2024
8 of 9 checks passed
@garydgregory
Copy link
Member

@sigee
Merged; TY!

garydgregory added a commit that referenced this pull request Oct 18, 2024
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