-
Notifications
You must be signed in to change notification settings - Fork 1
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 broken argument parser test #56
Conversation
@@ -11,7 +11,7 @@ | |||
|
|||
def test_make_inference_arg_parser(): | |||
parser = make_inference_arg_parser() | |||
args = parser.parse_args() | |||
args = parser.parse_known_args()[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could test this by passing in the command line arguments to parse explicitly. This way, you could test different inputs without needing to run from the command line (and ignore any other conflicting arguments from the pytest invocation). Something like parser.parse_args([])
.
Then if you wanted to test specific inputs you'd do something like parser.parse_args(["--flag", "test_value"])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to match your suggestion.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
========================================
+ Coverage 1.23% 4.97% +3.73%
========================================
Files 19 19
Lines 2908 3056 +148
========================================
+ Hits 36 152 +116
- Misses 2872 2904 +32
☔ View full report in Codecov by Sentry. |
Use
parse_known_args
instead ofparse_args
so the coverage tests do not fail.parse_known_args
returns a tuple of (known, unknown) args, so we take the first element of that tuple.