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

Fix collection failures due to permission errors when using --pyargs #12043

Merged
merged 5 commits into from
Mar 2, 2024

Conversation

bluetech
Copy link
Member

@bluetech bluetech commented Mar 1, 2024

This should fix #11904. It does not fix all permission error situations that popped up with pytest 8.0.0 but most complaints were about --pyargs and others are mostly misconfigurations as far I could tell, so let's start with this to unblock pytest 8.1.0 and see about the others as the come.

The PR has some preparatory commits, here is the description of the main change (last commit):

In pytest<8, the collection tree for pyargs arguments in an invocation like this:

pytest --collect-only --pyargs pyflakes.test.test_undefined_names

looked like this:

<Package test>
  <Module test_undefined_names.py>
    <UnitTestCase Test>
      <TestCaseFunction test_annotationUndefined>
      ... snipped ...

The pytest 8 collection improvements changed it to this:

<Dir pytest>
  <Dir .tox>
    <Dir venv>
      <Dir lib>
        <Dir python3.11>
          <Dir site-packages>
            <Package pyflakes>
              <Package test>
                <Module test_undefined_names.py>
                  <UnitTestCase Test>
                    <TestCaseFunction test_annotationUndefined>
                    ... snipped ...

Besides being egregious (and potentially even worse than the above, going all the way to the root, for system-installed packages, as is apparently common in CI), this also caused permission errors when trying to probe some of those intermediate directories.

This change makes --pyargs arguments no longer try to add parent directories to the collection tree according to the --confcutdir like they're regular arguments. Instead, only add the parents that are in the import path. This now looks like this:

<Package .tox/venv/lib/python3.11/site-packages/pyflakes>
  <Package test>
    <Module test_undefined_names.py>
      <UnitTestCase Test>
        <TestCaseFunction test_annotationUndefined>
        ... snipped ...

Used in `resolve_collection_argument`. It's implicitly imported by some
other import, but some type checkers don't recognize this.
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!

I specially liked the CollectionArgument dataclass, this is something that also caught my eye in the past.

Missing a CHANGELOG entry; I would add an example showing the old and the new collection tree, as it helps understand the change better.

testing/test_collection.py Outdated Show resolved Hide resolved
src/_pytest/main.py Outdated Show resolved Hide resolved
bluetech added 4 commits March 2, 2024 20:25
This is available when the argument is a `--pyargs` argument (resolved
from a python module path). Will be used in an upcoming commit.
No logical change, preparation for the next commit.
…llection arguments

(diff better viewed ignoring whitespace)

In pytest<8, the collection tree for `pyargs` arguments in an invocation
like this:

    pytest --collect-only --pyargs pyflakes.test.test_undefined_names

looked like this:

```
<Package test>
  <Module test_undefined_names.py>
    <UnitTestCase Test>
      <TestCaseFunction test_annotationUndefined>
      ... snipped ...
```

The pytest 8 collection improvements changed it to this:

```
<Dir pytest>
  <Dir .tox>
    <Dir venv>
      <Dir lib>
        <Dir python3.11>
          <Dir site-packages>
            <Package pyflakes>
              <Package test>
                <Module test_undefined_names.py>
                  <UnitTestCase Test>
                    <TestCaseFunction test_annotationUndefined>
                    ... snipped ...
```

Besides being egregious (and potentially even worse than the above,
going all the way to the root, for system-installed packages, as is
apparently common in CI), this also caused permission errors when trying
to probe some of those intermediate directories.

This change makes `--pyargs` arguments no longer try to add parent
directories to the collection tree according to the `--confcutdir` like
they're regular arguments. Instead, only add the parents that are in the
import path. This now looks like this:

```
<Package .tox/venv/lib/python3.11/site-packages/pyflakes>
  <Package test>
    <Module test_undefined_names.py>
      <UnitTestCase Test>
        <TestCaseFunction test_annotationUndefined>
        ... snipped ...
```

Fix pytest-dev#11904.
@bluetech
Copy link
Member Author

bluetech commented Mar 2, 2024

Missing a CHANGELOG entry; I would add an example showing the old and the new collection tree, as it helps understand the change better.

I never forget to forget to git add the file, added now. I didn't add the collection tree comparisons, it seems like giving too much space for something not too significant. Instead, I linked to this PR for those interested in more info.

@bluetech bluetech merged commit 6ed0051 into pytest-dev:main Mar 2, 2024
25 checks passed
@bluetech bluetech deleted the pyargs-root branch March 2, 2024 19:12
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.

8.0.0 + pyargs + permissions causes failures
2 participants