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

Update path argument #89

Merged
merged 11 commits into from Dec 21, 2021
Merged

Update path argument #89

merged 11 commits into from Dec 21, 2021

Conversation

zero323
Copy link
Contributor

@zero323 zero323 commented Dec 21, 2021

This PR introduces compatibility with pytest 7.0.0rc1. As described in the link included in the original issue. fspath with LocalPath is deprecated in favor of path with pathlib.Path.

Closes #88

Comment on lines 163 to 166
def pytest_collect_file(file_path: pathlib.Path, parent: Node) -> Optional[YamlTestFile]:
if file_path.suffix in {".yaml", ".yml"} and file_path.name.startswith(("test-", "test_")):
return YamlTestFile.from_parent(parent, path=file_path)
return None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If only this part is implemented, pluggy fails with:

pluggy._manager.PluginValidationError: Plugin 'pytest-mypy-plugins' for hook 'pytest_collect_file'
hookimpl definition: pytest_collect_file(file_path: pathlib.Path, parent: _pytest.nodes.Node) -> Optional[pytest_mypy_plugins.collect.YamlTestFile]
Argument(s) {'file_path'} are declared in the hookimpl but can not be found in the hookspec

@zero323 zero323 marked this pull request as draft December 21, 2021 01:55
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

Do we need some additional test cases with pytest==7.0 and pytest<7.0?

@zero323
Copy link
Contributor Author

zero323 commented Dec 21, 2021

Do we need some additional test cases with pytest==7.0 and pytest<7.0?

For starters, I am thinking about adding a matrix of pytest versions to the workflow to be sure things work across versions.

Beyond that, I am not sure TBH. Initially I completely missed reportinfo return type ‒ it passed the tests without warnings, as well as mypy==0.910 tests and was caught only in CI, with mypy==0.920. But current tests don't go that deep into internals.

@zero323
Copy link
Contributor Author

zero323 commented Dec 21, 2021

Beyond that, I am not sure TBH. Initially I completely missed reportinfo return type

Huh, so it seems like it is unrelated to issue

@sobolevn
Copy link
Member

Anything else? It is still marked as WIP 🙂

@zero323 zero323 marked this pull request as ready for review December 21, 2021 15:07
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Great work!

@sobolevn sobolevn merged commit e59bda7 into typeddjango:master Dec 21, 2021
@zero323
Copy link
Contributor Author

zero323 commented Dec 21, 2021

Thanks @sobolevn @lovasoa!

@zero323 zero323 deleted the fspath-update branch December 21, 2021 15:16
@sinback
Copy link

sinback commented Jan 10, 2022

Yay, this is cool. It's causing an issue in the test framework for one of lovasoa's repositories which has also been bothering me for a while, and blocking a pull request which would fix it. It doesn't look like there's been a release since this branch was merged, do you know when we might get one? :)

@sobolevn
Copy link
Member

Today! 🙂

@sobolevn
Copy link
Member

Done! https://pypi.org/project/pytest-mypy-plugins/1.9.3/

Thanks for the reminder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants