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

Jump to bottom

--import-mode=importlib causes tests to import a class with a different id #10341

Closed
4 tasks done
rectalogic opened this issue Oct 5, 2022 · 2 comments · Fixed by #11148
Closed
4 tasks done

--import-mode=importlib causes tests to import a class with a different id #10341

rectalogic opened this issue Oct 5, 2022 · 2 comments · Fixed by #11148
Labels
topic: collection related to the collection phase type: bug problem that needs to be addressed

Comments

@rectalogic
Copy link

  • a detailed description of the bug or problem you are having
  • output of pip list from the virtual environment you are using
  • pytest and operating system versions
  • minimal example if possible

Here is a minimal repo that reproduces this issue https://github.com/rectalogic/pytest-importlib-bug
It is configured for poetry, you can poetry install then poetry run pytest -s to reproduce. See the README at the above repo for example output.

When using both --doctest-modules and --import-mode=importlib with pytest, it imports the same class as two different classes.

In the test repo above, you can see the id() of the class imported in test_google_workspace.py is different than the one in google_workspace.py - and then the test assertion fails because the two Event classes are actually different.

Remove either of the above pytest options and tests pass.
Running the test in isolation also passes.

pytest 7.1.3
macOS 12.6

pip list output:

Package    Version Editable project location
---------- ------- -----------------------------------------------------------------------
attrs      22.1.0
iniconfig  1.1.1
packaging  21.3
pip        22.2.2
pluggy     1.0.0
py         1.11.0
pybug      0.1.0   /Users/aw/Projects/cureatr/dev/cureatr/experiments/pytest-importlib-bug
pyparsing  3.0.9
pytest     7.1.3
setuptools 65.3.0
tomli      2.0.1
wheel      0.37.1
@rectalogic
Copy link
Author

It works with pytest@7.1.2, so I suspect this PR is what broke it #10088

@RonnyPfannschmidt
Copy link
Member

Most likely, that patch shoehorning namespaces in a funky way is a potential problem

@Zac-HD Zac-HD added type: bug problem that needs to be addressed topic: collection related to the collection phase labels Oct 16, 2022
@jaraco jaraco mentioned this issue Dec 22, 2022
Closed
@jaraco jaraco mentioned this issue Jun 25, 2023
Closed
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 29, 2023
@nicoddemus
Fix duplicated imports with importlib mode and doctest-modules
ef8e9df 
The initial implementation (in pytest-dev#7246) introduced the `importlib` mode, which
never added the imported module to `sys.modules`, so it included a test
to ensure calling `import_path` twice would yield different modules.

That proved problematic, so we started adding the imported module to `sys.modules`
in pytest-dev#7870, but failed to realize that given we are now changing `sys.modules`, we might
as well avoid importing it more than once.

Then pytest-dev#10088 came along, passing `importlib` also when importing application modules
(as opposed to only test modules before), which caused problems due to imports
having side-effects and the expectation being that they are imported only once.

With this PR, `import_path` returns the module immediately if already in
`sys.modules`.

Fix pytest-dev#10811, pytest-dev#10341
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jun 29, 2023
@nicoddemus
Fix duplicated imports with importlib mode and doctest-modules
86fb698 
The initial implementation (in pytest-dev#7246) introduced the `importlib` mode, which
never added the imported module to `sys.modules`, so it included a test
to ensure calling `import_path` twice would yield different modules.

Not adding modules to `sys.modules` proved problematic, so we began to add the imported module to `sys.modules`
in pytest-dev#7870, but failed to realize that given we are now changing `sys.modules`, we might
as well avoid importing it more than once.

Then pytest-dev#10088 came along, passing `importlib` also when importing application modules
(as opposed to only test modules before), which caused problems due to imports
having side-effects and the expectation being that they are imported only once.

With this PR, `import_path` returns the module immediately if already in
`sys.modules`.

Fix pytest-dev#10811, pytest-dev#10341
@nicoddemus nicoddemus mentioned this issue Jun 29, 2023
Merged
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jul 1, 2023
@nicoddemus
Fix duplicated imports with importlib mode and doctest-modules
7335b76 
The initial implementation (in pytest-dev#7246) introduced the `importlib` mode, which
never added the imported module to `sys.modules`, so it included a test
to ensure calling `import_path` twice would yield different modules.

Not adding modules to `sys.modules` proved problematic, so we began to add the imported module to `sys.modules`
in pytest-dev#7870, but failed to realize that given we are now changing `sys.modules`, we might
as well avoid importing it more than once.

Then pytest-dev#10088 came along, passing `importlib` also when importing application modules
(as opposed to only test modules before), which caused problems due to imports
having side-effects and the expectation being that they are imported only once.

With this PR, `import_path` returns the module immediately if already in
`sys.modules`.

Fix pytest-dev#10811, pytest-dev#10341
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jul 1, 2023
@nicoddemus
Fix duplicated imports with importlib mode and doctest-modules
d2ae7d3 
The initial implementation (in pytest-dev#7246) introduced the `importlib` mode, which
never added the imported module to `sys.modules`, so it included a test
to ensure calling `import_path` twice would yield different modules.

Not adding modules to `sys.modules` proved problematic, so we began to add the imported module to `sys.modules`
in pytest-dev#7870, but failed to realize that given we are now changing `sys.modules`, we might
as well avoid importing it more than once.

Then pytest-dev#10088 came along, passing `importlib` also when importing application modules
(as opposed to only test modules before), which caused problems due to imports
having side-effects and the expectation being that they are imported only once.

With this PR, `import_path` returns the module immediately if already in
`sys.modules`.

Fix pytest-dev#10811, pytest-dev#10341
nicoddemus added a commit that referenced this issue Jul 1, 2023
@nicoddemus
Fix duplicated imports with importlib mode and doctest-modules (#11148)
b77d0de 
The initial implementation (in #7246) introduced the `importlib` mode, which
never added the imported module to `sys.modules`, so it included a test
to ensure calling `import_path` twice would yield different modules.

Not adding modules to `sys.modules` proved problematic, so we began to add the imported module to `sys.modules`
in #7870, but failed to realize that given we are now changing `sys.modules`, we might
as well avoid importing it more than once.

Then #10088 came along, passing `importlib` also when importing application modules
(as opposed to only test modules before), which caused problems due to imports
having side-effects and the expectation being that they are imported only once.

With this PR, `import_path` returns the module immediately if already in
`sys.modules`.

Fix #10811
Fix #10341
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: collection related to the collection phase type: bug problem that needs to be addressed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix duplicated imports with importlib mode and doctest-modules nicoddemus/pytest
3 participants
@rectalogic @RonnyPfannschmidt @Zac-HD