Skip to content

Conversation

@PGijsbers
Copy link
Contributor

@PGijsbers PGijsbers commented Jul 25, 2024

SubRequest was removed, but it was really used to denote a FixtureRequest, so I updated that part and now everything works on the latest versions again.

edit: with pytest 8+, test ordering changed which exposed abuse of global state in tests, so I had to remove that too.

@PGijsbers PGijsbers added the pip dependencies update dependency version label Jul 25, 2024
@PGijsbers PGijsbers requested review from jsmatias and removed request for jsmatias July 25, 2024 14:53
@PGijsbers
Copy link
Contributor Author

Whoops, I accidentally branched off the wrong branch.. rebasing.



class TestUserType(enum.Enum):
__test__ = False
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise Pytest will complain it cannot collect from this class since it has an (implicit) __init__ method.

@PGijsbers
Copy link
Contributor Author

I am a bit confused why some tests failed.. they pass locally. Trying to look into it. Have you experienced this before? Are there flaky tests (on CI)?

@PGijsbers
Copy link
Contributor Author

Closing and reopening to see if that triggers CI.

@PGijsbers PGijsbers closed this Nov 19, 2024
@PGijsbers PGijsbers reopened this Nov 19, 2024
@PGijsbers
Copy link
Contributor Author

PGijsbers commented Nov 19, 2024

The test fails when run in the entire suite, not when run in isolation. Have narrowed down to the specific version change of pytest 7.4.4->8.0.0 with pytest-asyncio fixed at 2.3.8.

@PGijsbers
Copy link
Contributor Author

I'm still not entirely sure what's happening, it looks like a different responses mock is used than intended in the authentication tests... and the tests results depend on execution order. I started down this path since it was a breaking change of pytest 8.0.0, verified by renaming test_authorization.py to auth_test.py so it is executed earlier (alphabetical order..), we verify the tests do work then. So the tests somehow violate independence, but I am not sure yet how since the auth tests seem OK (all work is within the scope of a context manager).

@PGijsbers
Copy link
Contributor Author

The problem was practically every other test modifying global state.
I patched it for now to where they at least revert the change after the test is ran, but perhaps we should redesign the use of global variables. That would be a separate issue.

@PGijsbers PGijsbers requested review from Taniya-Das and removed request for Taniya-Das and jsmatias November 20, 2024 07:59
Copy link
Collaborator

@Taniya-Das Taniya-Das left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me.

@PGijsbers PGijsbers merged commit 7c4a80e into develop Nov 23, 2024
4 checks passed
@PGijsbers PGijsbers deleted the bump-pytest branch November 23, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pip dependencies update dependency version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants