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.

Sign up for GitHub

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

fix: pytest 8.1 compat #16

Merged
merged 2 commits into from
Mar 10, 2024
Merged

fix: pytest 8.1 compat #16

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d488950
fix: pytest 8 compat
hairmare Mar 4, 2024
107ecfe
Test on yanked pytest 8.1.0
iurisilvio Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions pytest_ruff/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@

from ruff.__main__ import find_ruff_bin

from pytest_ruff._pytest_compat import get_stash, make_path_kwargs, set_stash
from pytest_ruff._pytest_compat import (
get_stash,
make_path_kwargs,
set_stash,
PYTEST_VER,
)

HISTKEY = "ruff/mtimes"

Expand All @@ -26,15 +31,29 @@
set_stash(config, config.cache.get(HISTKEY, {}))


def pytest_collect_file(path, parent, fspath=None):
config = parent.config
if not config.option.ruff:
return
if PYTEST_VER >= (7, 0):

if path.ext != ".py":
return
def pytest_collect_file(file_path, parent, fspath=None):
config = parent.config
if not config.option.ruff:
return

Check warning on line 39 in pytest_ruff/__init__.py

View check run for this annotation

Codecov / codecov/patch

pytest_ruff/__init__.py#L39 

Added line #L39 was not covered by tests

if file_path.suffix != ".py":
return

return RuffFile.from_parent(parent, **make_path_kwargs(file_path))

else:

def pytest_collect_file(path, parent, fspath=None):
config = parent.config
if not config.option.ruff:
return

Check warning on line 51 in pytest_ruff/__init__.py

View check run for this annotation

Codecov / codecov/patch

pytest_ruff/__init__.py#L51 

Added line #L51 was not covered by tests

if path.ext != ".py":
return

Check warning on line 54 in pytest_ruff/__init__.py

View check run for this annotation

Codecov / codecov/patch

pytest_ruff/__init__.py#L54 

Added line #L54 was not covered by tests

return RuffFile.from_parent(parent, **make_path_kwargs(path))
return RuffFile.from_parent(parent, **make_path_kwargs(path))
Copy link

Choose a reason for hiding this comment

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

For the sake of code deduplication, why not:

def _pytest_collect_file(
        file_path: Path, parent: pytest.Collector,
) -> RuffFile:
    config = parent.config
    if not config.option.ruff:
        return

    if path.ext != ".py":
        return

    return RuffFile.from_parent(parent, **make_path_kwargs(path))


if PYTEST_VER >= (7, 0):

    def pytest_collect_file(file_path, parent):
        return _pytest_collect_file(Path(file_path), parent)

else:

    def pytest_collect_file(path, parent):
        return _pytest_collect_file(path, parent)

For clarity, I also removed unused fspath and added type hints. My main point is to deduplicate code into a function that looks like the modern version and then call it via a small wrapper for old versions. I feel like this makes it much easier to understand what is going on and what is the actual difference between those two interfaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gave this a try but seem to be stumbling over the make_path_kwargs function when doing so. I'll have to dig deeper later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to reduce duplication too, but it is difficult to make it right, these args are a bit confusing.

I added a tox test with pytest 8.1.0 pinned and it works great.

Released in 0.3.1.



def pytest_sessionfinish(session, exitstatus):
Expand Down
Loading