Skip to content
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: Sync expected stream for Pytest' version #996

Merged
merged 1 commit into from Feb 28, 2022

Conversation

stanislavlevin
Copy link
Contributor

https://docs.pytest.org/en/7.0.x/changelog.html#breaking-changes:

pytest#8246: --version now writes version information to stdout rather than stderr.

Fixes: #995

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks! I left a suggestion.

Comment on lines 121 to 126
if result.errlines:
version_out = result.stderr
else:
# Pytest 7.0+
version_out = result.stdout

version_out.fnmatch_lines(["*This is pytest version*"])
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to use an explicit pytest version check, since then when we drop support for pytest<7 it is much easier to find and remove no longer needed code.

Suggested change
if result.errlines:
version_out = result.stderr
else:
# Pytest 7.0+
version_out = result.stdout
version_out.fnmatch_lines(["*This is pytest version*"])
if hasattr(pytest, 'version_info') and pytest.version_info >= (7, 0):
version_out = result.stdout
else:
version_out = result.stderr
version_out.fnmatch_lines(["*This is pytest version*"])

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is better! Thanks @bluetech

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bluetech, thank you for the suggestion! I thought about this.
I prefer to not rely on version-check but check for actual behavior. For future cleanup I added the comment in code. But of course, your pov as project's maintainer is higher than my own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.
Note: version_info => version_tuple (requires Pytest 7.0+).

Copy link
Member

@hramezani hramezani left a comment

Choose a reason for hiding this comment

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

Thanks @stanislavlevin 👍

https://docs.pytest.org/en/7.0.x/changelog.html#breaking-changes:
> [pytest#8246](pytest-dev/pytest#8246): --version now writes version information to stdout rather than stderr.

Fixes: pytest-dev#995
Signed-off-by: Stanislav Levin <slev@altlinux.org>
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.

self-tests fail against Pytest 7
3 participants