-
Notifications
You must be signed in to change notification settings - Fork 6
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
Parser tests: replace shell scripts with pytest scripts #48
Conversation
005e1eb
to
511ea66
Compare
Please rebase/merge 🙂 |
tests/test_parser.py
Outdated
text=True | ||
) | ||
|
||
assert len(output.stderr) == 0, output.stderr |
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.
Maybe check the exit code here as well (just in case)? Should be non-zero on error or crash
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.
Yep, that was pretty dumb of me. Completely forgot about crashes. Thanks, will do.
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.
Was it the case that the --parse-file
would return exit code 0 previously on errors? Because I have no clue why the shell script used stderr output and not the exit code for tests. Maybe it was me just being dumb, hmm.
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.
Now that I think about it, it was probably just me shortcutting it, since both errors and crashes write to the stderr. Anyways, I added a check for the return code.
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.
You are right that stderr would likely catch 99.9% of all errors, but status code should catch the remaining 0.1% 🙂
Replaces the shell scripts for parser testing with pytest tests.
Runnable with: