-
Notifications
You must be signed in to change notification settings - Fork 126
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
tests: config: Fix assertions checks with pytest.raises #777
Conversation
@carlescufi #768 might change the message with the grouped argument change, this PR can go before or after yours but might need changes accordingly. |
62a7753
to
41f201b
Compare
Updated after merging #768 with updated error messages. |
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 took a longer look and realized now that this does much more than moving the asserts : this also fixes the test code which of course did not work because it never ran!
That's great, but now I see a lot of repetition. Have you considered a new, generic function for negative tests? Something like:
def run_bad_cmd(cmd_str_or_list, expected_exception_type):
with pytest.raises(expected_exception_type) as e:
...
return err_msg
emsg = run_bad_cmd("config pytest", subprocess.CalledProcessError)
assert 'invalid ..." in emsg
Create a helper function that captures an expected exception and returns the output together with stderr data. Dedent assertion checks as they were currently skipped, and make sure the correct error message is validated. Signed-off-by: Pieter De Gendt <pieter.degendt@basalte.be>
41f201b
to
5b12276
Compare
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.
Thanks!
Dedent assertion checks as they were currently skipped, and make sure the correct error message is validated.
Fixes #776