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

doctest: fix autouse fixtures possibly not getting picked up #11941

Merged
merged 2 commits into from Feb 7, 2024

Conversation

bluetech
Copy link
Member

@bluetech bluetech commented Feb 6, 2024

Fix #11929. There is also a prerequisite commit, please see the commit message.

Currently, `DoctestModule` does `import_path` on its own. This changes
it to use `importtestmodule` as `Module` does. The behavioral changes
are:

- Much better error messages on import errors.

- Handles a few more error cases (see `importtestmodule`). This
  technically expands the cover of `--doctest-ignore-import-errors` but
  I think it makes sense.

- Considers `pytest_plugins` in the module.

- Populates `self.obj` as properly (without double-imports) as is
  expected from a `PyCollector`.

This is also needed for the next commit.
@bluetech bluetech added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Feb 6, 2024
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, appreciate the separate commits (as always).

I noticed this is touching the doctest import part mentioned in #11475...

@flying-sheep any chance of you testing this branch with your reproducer, and see how it affects it?

Fix pytest-dev#11929.

Figured out what's going on. We have the following collection tree:

```
<Dir pyspacewar>
  <Dir src>
    <Package pyspacewar>
      <Package tests>
        <DoctestModule test_main.py>
          <DoctestItem pyspacewar.tests.test_main.doctest_main>
```

And the `test_main.py` contains an autouse fixture (`fake_game_ui`) that
`doctest_main` needs in order to run properly. The fixture doesn't run!
It doesn't run because nothing collects the fixtures from (calls
`parsefactories()` on) the `test_main.py` `DoctestModule`.

How come it only started happening with commit
ab63ebb? Turns out it mostly only
worked accidentally. Each `DoctestModule` is also collected as a normal
`Module`, with the `Module` collected after the `DoctestModule`. For
example, if we add a non-doctest test to `test_main.py`, the collection
tree looks like this:

```
<Dir pyspacewar>
  <Dir src>
    <Package pyspacewar>
      <Package tests>
        <DoctestModule test_main.py>
          <DoctestItem pyspacewar.tests.test_main.doctest_main>
        <Module test_main.py>
          <Function test_it>
```

Now, `Module` *does* collect fixtures. When autouse fixtures are
collected, they are added to the `_nodeid_autousenames` dict.

Before ab63ebb, `DoctestItem` consults
`_nodeid_autousenames` at *setup* time. At this point, the `Module` has
collected and so it ended up picking the autouse fixture (this relies on
another "accident", that the `DoctestModule` and `Module` have the same
node ID).

After ab63ebb, `DoctestItem` consults
`_nodeid_autousenames` at *collection* time (= when it's created). At
this point, the `Module` hasn't collected yet, so the autouse fixture is
not picked out.

The fix is simple -- have `DoctestModule.collect()` call
`parsefactories`. From some testing I've done it shouldn't have negative
consequences (I hope).
@bluetech
Copy link
Member Author

bluetech commented Feb 7, 2024

@nicoddemus I'm pretty sure #11475 is a different issue from this one.

@bluetech bluetech merged commit 6c0b6c2 into pytest-dev:main Feb 7, 2024
24 checks passed
@bluetech bluetech deleted the doctest-parsefactories branch February 7, 2024 20:09
@bluetech bluetech added backport 8.0.x and removed needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch labels Feb 7, 2024
bluetech added a commit to bluetech/pytest that referenced this pull request Feb 7, 2024
…sefactories

doctest: fix autouse fixtures possibly not getting picked up
(cherry picked from commit 6c0b6c2)
nicoddemus pushed a commit that referenced this pull request Feb 8, 2024
#11948)

doctest: fix autouse fixtures possibly not getting picked up
(cherry picked from commit 6c0b6c2)
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.

pytest 8.0.0 ignores autouse fixtures in doctest modules when collecting packages
2 participants