-
Notifications
You must be signed in to change notification settings - Fork 249
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: housekeeping, test_authentication.py #7509
Conversation
f25866a
to
80c539f
Compare
"This commit does not belong to any branch on this repository" |
80c539f
to
0a3acc9
Compare
@alexey-tikhonov fixed. |
housekeeping, the following is looked at and may have been done: * fixed typos and standardized formatting * renamed test cases to improve the clarity of what the test does * improved docstring language, setup, steps and expected results * synced code with the docstring order * removed necessary configuration relevant to the test * added pytest.mark.importance to test cases noteable changes: * big rename on the test case names, after discussing that some cases will have the positive and negative test, it no longers to be specified (cherry picked from commit 7716d13)
0a3acc9
to
4aaef80
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.
Code-wise it looks good.
I would like to see more descriptive "Expected result" (Feel free to disagree ;-) ).
Tom
CI failures are not related to the PR |
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.
ACK
@jakub-vavra-cz, can we merge it? |
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.
LGTM
housekeeping, the following is looked at and may have been done:
noteable changes:
(cherry picked from commit 7716d13)