Resolve relative test-selection args before matching rsync roots - #1366
Open
agu2347 wants to merge 2 commits into
Open
Resolve relative test-selection args before matching rsync roots#1366agu2347 wants to merge 2 commits into
agu2347 wants to merge 2 commits into
Conversation
make_reltoroot() (used to translate a test-selection arg like "tests/test_sample.py" into a path relative to an rsync root before sending it to a remote worker) constructed fspath = Path(parts[0]) directly from the given arg. py.path.local, used here prior to the project's migration to pathlib, transparently resolved a relative path against the current working directory as part of its own construction; plain pathlib.Path does not do this. As a result, a relative path given on the command line (e.g. running `pytest -d --tx socket=... tests/test_sample.py` from the project root) never compared as a match or subpath of any of the (absolute) rsync roots via relative_to(), even when it does in fact point inside one of them once resolved against the cwd -- raising "arg ... not relative to an rsync root" and breaking test selection entirely for any relative path, which worked correctly before the pathlib migration. Resolve fspath explicitly (Path.resolve()) before the existence check and root-matching loop, restoring the old py.path.local behavior. Verified directly against the exact reported scenario: calling make_reltoroot() with a relative path arg and an absolute root previously raised the exact "not relative to an rsync root" error; with the fix, it resolves correctly. Also verified: an absolute path arg (the case that already worked) is unaffected; an arg with a "::test_id" suffix is preserved correctly; and a non-existent relative path still passes through unchanged rather than raising. Added regression tests covering all of the above using pytester's tmp_path/monkeypatch.chdir, matching this test file's existing style. Confirmed the relative-path tests fail with the original code (reproducing the exact reported ValueError) and pass with the fix. Ran the full existing test_workermanage.py suite (23 passed: 19 baseline + 4 new; 4 pre-existing, unrelated warning-serialization failures confirmed identical with and without this change via a fixed random seed on a clean checkout). Fixes pytest-dev#971
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #971.
make_reltoroot()(used to translate a test-selection arg liketests/test_sample.pyinto a path relative to an rsync root before sending it to a remote worker) constructedfspath = Path(parts[0])directly from the given arg.py.path.local, used here prior to the project's migration to pathlib, transparently resolved a relative path against the current working directory as part of its own construction; plainpathlib.Pathdoes not do this.As a result, a relative path given on the command line (e.g. running
pytest -d --tx socket=... tests/test_sample.pyfrom the project root) never compared as a match or subpath of any of the (absolute) rsync roots viarelative_to(), even when it does in fact point inside one of them once resolved against the cwd -- raising"arg ... not relative to an rsync root"and breaking test selection entirely for any relative path, which worked correctly before the pathlib migration -- exactly as diagnosed in the issue, including the exact commit that introduced the regression.Fix: resolve
fspathexplicitly (Path.resolve()) before the existence check and root-matching loop, restoring the oldpy.path.localbehavior.Testing: verified directly against the exact reported scenario: calling
make_reltoroot()with a relative path arg and an absolute root previously raised the exact"not relative to an rsync root"error; with the fix, it resolves correctly. Also verified: an absolute path arg (the case that already worked) is unaffected; an arg with a"::test_id"suffix is preserved correctly; and a non-existent relative path still passes through unchanged rather than raising.Added regression tests covering all of the above using pytester's
tmp_path/monkeypatch.chdir, matching this test file's existing style. I confirmed the relative-path tests fail with the original code (reproducing the exact reportedValueError) and pass with the fix. Ran the full existingtest_workermanage.pysuite (23 passed: 19 baseline + 4 new; 4 pre-existing, unrelated warning-serialization failures confirmed identical with and without this change via a fixed random seed on a clean checkout).