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

Breaks with pytest 7 due to private imports #533

Closed
The-Compiler opened this issue Dec 21, 2021 · 15 comments · Fixed by #535
Closed

Breaks with pytest 7 due to private imports #533

The-Compiler opened this issue Dec 21, 2021 · 15 comments · Fixed by #535

Comments

@The-Compiler
Copy link

In pytest-dev/pytest#9415, we discovered that this plugin breaks with pytest 7.0.0rc1:

  File ".../pytest_postgresql/factories/process.py", line 27, in <module>
    from _pytest.tmpdir import TempdirFactory
ImportError: cannot import name 'TempdirFactory' from '_pytest.tmpdir' (/usr/lib/python3.10/site-packages/_pytest/tmpdir.py)

This is due to this class moving to a different place in pytest internally. Other places use similar private imports which still work, for now:

tests/test_postgres_options_plugin.py
3:from _pytest.pytester import Pytester

src/pytest_postgresql/config.py
1:from _pytest.fixtures import FixtureRequest

src/pytest_postgresql/plugin.py
22:from _pytest.config.argparsing import Parser

tests/test_executor.py
5:from _pytest.fixtures import FixtureRequest

src/pytest_postgresql/factories/client.py
22:from _pytest.fixtures import FixtureRequest

src/pytest_postgresql/factories/noprocess.py
23:from _pytest.fixtures import FixtureRequest

src/pytest_postgresql/factories/process.py
26:from _pytest.fixtures import FixtureRequest
27:from _pytest.tmpdir import TempdirFactory

With pytest 7.0.0, those types become publicly available, so it's recommended to import them from pytest directly instead, perhaps via a small compat layer which can be dropped once support for pytest<7 isn't relevant anymore.

@The-Compiler
Copy link
Author

Trivial fix which makes the test work (but the other private imports should be avoided even if they continue to work):

From 64d8b7135cb6cd582b06dbabe4da1ba3d19c8a73 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Tue, 21 Dec 2021 17:43:47 +0100
Subject: [PATCH] Fix pytest 7 compatibility

---
 src/pytest_postgresql/factories/process.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/pytest_postgresql/factories/process.py b/src/pytest_postgresql/factories/process.py
index 0f002c8..017b25c 100644
--- a/src/pytest_postgresql/factories/process.py
+++ b/src/pytest_postgresql/factories/process.py
@@ -23,10 +23,15 @@
 from typing import Union, Callable, List, Iterator, Optional, Tuple, Set
 
 import pytest
-from _pytest.fixtures import FixtureRequest
-from _pytest.tmpdir import TempdirFactory
 from port_for import get_port
 
+try:
+    # pytest 7
+    from pytest import FixtureRequest, TempdirFactory
+except ImportError:
+    from _pytest.fixtures import FixtureRequest
+    from _pytest.tmpdir import TempdirFactory
+
 from pytest_postgresql.config import get_config
 from pytest_postgresql.executor import PostgreSQLExecutor
 from pytest_postgresql.janitor import DatabaseJanitor
-- 
2.34.1

@fizyk
Copy link
Member

fizyk commented Dec 21, 2021

Took a look at pytest changelog, and we can import TmpdirFactory and FixtureRequest from pytest since pytest 6.2.0 Without try...except block.

I'll try to update the imports tomorrow and release a new version that hard requires at least pytest >= 6.2.0

More https://docs.pytest.org/en/6.2.x/changelog.html#improvements

fizyk added a commit that referenced this issue Dec 22, 2021
fizyk added a commit that referenced this issue Dec 22, 2021
fizyk added a commit that referenced this issue Dec 22, 2021
@fizyk
Copy link
Member

fizyk commented Dec 22, 2021

@The-Compiler I'm not able to import Parser from anywhere else though

@The-Compiler
Copy link
Author

Yep, looks like pytest.Parser will only be exposed with the upcoming 7.0.0 release, so you will probably need a try/except for that one for now. Well spotted that the others were available with 6.2.0 already, I missed that one!

@fizyk
Copy link
Member

fizyk commented Dec 22, 2021

@The-Compiler 7.0.x only deprecates the Parser usage.

I'd come back to this after the 7.0 will get released https://docs.pytest.org/en/7.0.x/changelog.html#deprecations

@The-Compiler
Copy link
Author

It deprecates creating the class, which is not something you are doing. It also exposes the class publicly as pytest.Parser for type annotations:

image

@fizyk fizyk reopened this Dec 22, 2021
@fizyk
Copy link
Member

fizyk commented Dec 22, 2021

@The-Compiler the same was the case for TempdirFactory and FixtureRequest in pytest 6.2: https://docs.pytest.org/en/6.2.x/changelog.html#deprecations

7.0 exposes Parser in similar fashion the 6.2 exposes the two types above https://docs.pytest.org/en/7.0.x/changelog.html#features

I'm puzzled though, as it seems that the TempdirFactory disappeared from the pytest 7.0.0.rc1

fizyk added a commit that referenced this issue Dec 22, 2021
@fizyk
Copy link
Member

fizyk commented Dec 22, 2021

okay, need to replace it with Temppath factory...

@The-Compiler
Copy link
Author

7.0 exposes Parser in similar fashion the 6.2 exposes the two types above https://docs.pytest.org/en/7.0.x/changelog.html#features

Yes, that's what I've been trying to say above 😉

I'm puzzled though, as it seems that the TempdirFactory disappeared from the pytest 7.0.0.rc1

Disappeared how? Works as expected for me when using the public import. What has changed is that it's set by an internal pytest plugin, so it won't be immediately available - but it should be there once the test files run.

In your run above, I see that it means the module isn't importable anymore, and that mypy complains. Not too happy about that. I might open an issue about it.

@fizyk fizyk closed this as completed in 2ae96bc Dec 22, 2021
fizyk added a commit that referenced this issue Dec 22, 2021
Replace tmpdir with tmo_path fixtures - closes #533
@fizyk
Copy link
Member

fizyk commented Dec 22, 2021

@The-Compiler The Parser issue, I'll tackle when the pytest 7.0.0 will be released, so the pytest-postgresql should be ready before pytest 7.1... hopefully.

@The-Compiler
Copy link
Author

FYI I opened pytest-dev/pytest#9432 to track the TmpdirFactory regression.

@fizyk
Copy link
Member

fizyk commented Dec 23, 2021

@The-Compiler the 4.1 version should work with pytest 7 (at least rc1)

@The-Compiler
Copy link
Author

Great, thank you! cc @hroncok

fizyk added a commit that referenced this issue Feb 14, 2022
fizyk added a commit that referenced this issue Feb 14, 2022
fizyk added a commit that referenced this issue Feb 14, 2022
Replace tmpdir with tmo_path fixtures - closes #533
@BeebBenjamin
Copy link

When I try to run this in gitlab CI with an uncapped version of pytest > 7.3.1 the check used in the compat.py gives a false negative. I think because the import check from that file fails? I reduced the pytest version to 6.2.0 and I no longer get the No module named psycopg. Please install psycopg. exception.

@fizyk
Copy link
Member

fizyk commented May 16, 2023

@BeebBenjamin that's something new and rather unrelated to private imports from inside pytest package. Could you report new/fresh issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants