Skip to content

Conversation

@dheerajck
Copy link
Contributor

No description provided.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks @dheerajck!

Please see my suggestions.

Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
@dheerajck dheerajck requested a review from nicoddemus April 7, 2024 13:18
@dheerajck
Copy link
Contributor Author

also can I ask why was a ut failing before the latest commit but not right now

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

See my suggestion to preserve the original PYTEST_VERSION, so pytester.runpytest() is more robust.

…t run by the calling process, we set it back after pytest run

Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
@dheerajck dheerajck requested a review from nicoddemus April 9, 2024 14:20
@dheerajck
Copy link
Contributor Author

Do we need any more changes

@dheerajck
Copy link
Contributor Author

@nicoddemus

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@nicoddemus nicoddemus merged commit 48b6d18 into pytest-dev:main Apr 18, 2024
Copy link
Member

@adamchainz adamchainz left a comment

Choose a reason for hiding this comment

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

Thank you very much. This is a clean approach and may have other use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants