-
Notifications
You must be signed in to change notification settings - Fork 335
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
[ENG-2435] Upgrade pytest and others to fix broken tests #9551
[ENG-2435] Upgrade pytest and others to fix broken tests #9551
Conversation
17677b4
to
dfa0d35
Compare
dfa0d35
to
d936a6d
Compare
266fe52
to
0ce6b5b
Compare
bf60095
to
5ce53cf
Compare
5ce53cf
to
0ceed2a
Compare
@pytest.mark.parametrize('sanction_fixture', [registration_approval, embargo, retraction]) | ||
def test_approve_after_reject_fails(self, sanction_fixture): | ||
@pytest.mark.parametrize('sanction_object', [registration_approval, embargo, retraction]) | ||
def test_approve_after_reject_fails(self, sanction_object): | ||
# using fixtures in parametrize returns the function |
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.
nit: This comment is now unnecessary/misleading (in all locations)
Can you explain in the description why updating pytest fixed the test linked in the ticket? Also, more detail on what, specifically, pytest doesn't like about these fixtures now (seemingly it doesn't support using fixtures that aren't fully evaluated in test setup, a la via parametrize). |
bb4c9d3
to
3ba26e7
Compare
@jwalz ok I made some comments in the description |
3ba26e7
to
d8f706b
Compare
d8f706b
to
2252890
Compare
@@ -135,7 +135,6 @@ def test_rejection_flow(self, sanction_object, initial_state, end_state): | |||
|
|||
@pytest.mark.parametrize('sanction_object', [registration_approval, embargo, retraction]) | |||
def test_approve_after_reject_fails(self, sanction_object): | |||
# using fixtures in parametrize returns the function |
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.
It's not a huge deal, but if you could go through the file and delete all iterations of this comment, that would be appreciated
Purpose
This is a booster shot of updated decencies for the OSF for pytest, these such keep us more up to date with the current test infrastructure and hopefully fix some probabilistic test fails.
You can read about the bug here which indicates updating pytest will fix the issue. I picked pytest 5.0.0 because it is recent and only requires few changes. The biggest change is pytest no long allows passing of fixtures the same way. Read about it here. So that had to be changed to release this.
Changes
QA Notes
Please make verification statements inspired by your code and what your code touches.
What are the areas of risk?
Any concerns/considerations/questions that development raised?
Documentation
Side Effects
None that I know of.
Ticket
https://openscience.atlassian.net/browse/ENG-2435