pytestCheckHook: Switch from python -m pytest to simply pytest?

[doronbehar]

Summary of issue

Many packages in Nixpkgs that use cython extension modules, demonstrate some issues with running tests via pytestCheckHook, credit to @natsukium for helping figuring that out.

Generally, the problem is that the extension modules are installed at the installCheckPhase at $out, and are not $PWD. This for instance may cause differences between running pytest and running python -m pytest - hence the issue title. What's rather peculiar, is the fact that all packages that manage to overcome that use different schemes. The most common of them all is:

preCheck = ''
  cd $out
'';

Below is a list of such packages, and PRs that improved the situation a bit:

• python3.pkgs.xdot: 1.2 -> 1.3 #255254
• python3.pkgs.iniconfig: improve doCheck = false comment #255256
• python3.pkgs.primer3: enable tests #255260
• python3.pkgs.msprime: use pytestCheckHook #255471
• python3.pkgs.astropy: 5.2.1 -> 5.3.3 & more #255124
• python3Packages.numpy: use pytestCheckHook & disable failing tests #251923
• python3.pkgs.scipy: use pytestCheckHook && python3.pkgs.numpy: small cleanup #255607
• python311Packages.shapely: 2.0.1 → 2.0.2 #263989 (comment)
• python3.pkgs.orange3 No PR was made yet to workaround the issue.
• python311Packages.shapely: 2.0.1 → 2.0.2 #263989 (review)
• fontbakery: init at 0.10.40 #228099 (comment)
• python3.pkgs.matplotlib, the common workaround doesn't work due to the directory structure of it, and no other workaround has been found yet (running pytest instead of python -m pytest has also been tried). See also python310Packages.matplotlib: multiple cleanup commits #279317 .
• python3.pkgs.freud (added at python311Packages.freud: init at 3.0.0 #313079) has tests that don't pass due to a circular ImportError, but do pass if you use pytest and not python -m pytest. cd $out didn't help for pytestCheckHook.

Below is the original issue description, opened before further details were investigated. Hence the issues there may be a bit outdated.

Original issue description

Describe the bug

While trying to enable tests for pyerfa, I noticed that I got circular imports errors as explained upstream here. I then ran git grep circular import and learned that another Python package, pyrevolve is experiencing the same issue on NixOS:

nixpkgs/pkgs/development/python-modules/pyrevolve/default.nix

Lines 36 to 46 in 46688f8

	nativeCheckInputs = [ pytest ];
	# Using approach bellow bcs the tests fail with the pytestCheckHook, throwing the following error
	# ImportError: cannot import name 'crevolve' from partially initialized module 'pyrevolve'
	# (most likely due to a circular import)
	checkPhase = ''
	pytest
	'';
	pythonImportsCheck = [
	"pyrevolve"
	];

Steps To Reproduce

For each package that comes out in a git grep circular import search, I'll list what I learned, from most interesting, to less interesting:

python3.pkgs.pyrevolve

Try to build it with this patch:

diff --git i/pkgs/development/python-modules/pyrevolve/default.nix w/pkgs/development/python-modules/pyrevolve/default.nix
index 754baf91ad38..33df5d177f4d 100644
--- i/pkgs/development/python-modules/pyrevolve/default.nix
+++ w/pkgs/development/python-modules/pyrevolve/default.nix
@@ -38,7 +38,7 @@ buildPythonPackage rec {
   # ImportError: cannot import name 'crevolve' from partially initialized module 'pyrevolve'
   # (most likely due to a circular import)
   checkPhase = ''
-    pytest
+    python -m pytest
   '';
 
   pythonImportsCheck = [

Which does the same as replacing pytest with pytestCheckHook, and removing the overriding checkPhase. Note how the tests now fail.

python3.pkgs.orange3

Seems like a very similar case to what happens in python3.pkgs.pyrevolve, but replacing python -m pytest with pytest doesn't resolve the issue. I also haven't opened an issue upstream because I'm not sure whether this is our fault or theirs.

I tested this issue using this patch

diff --git i/pkgs/development/python-modules/orange3/default.nix w/pkgs/development/python-modules/orange3/default.nix
index cff4a603c846..4a9051548655 100644
--- i/pkgs/development/python-modules/orange3/default.nix
+++ w/pkgs/development/python-modules/orange3/default.nix
@@ -24,6 +24,7 @@
 , pyqtgraph
 , pyqtwebengine
 , python
+, pytest
 , python-louvain
 , pythonOlder
 , pyyaml
@@ -104,8 +105,13 @@ let
       baycomp
     ];
 
-    # FIXME: ImportError: cannot import name '_variable' from partially initialized module 'Orange.data' (most likely due to a circular import) (/build/source/Orange/data/__init__.py)
-    doCheck = false;
+    doCheck = true;
+    nativeCheckInputs = [
+      pytest
+    ];
+    checkPhase = ''
+      pytest
+    '';
 
     pythonImportsCheck = [ "Orange" "Orange.data._variable" ];
 

python3.pkgs.primer3

The circular import issue is mentioned there in a comment, but that comment seems maybe inaccurate to me, and the same testing failure is observed when adding pytest to nativeCheckInputs, and when running manually pytest in checkPhase. Comment improvement is in #255260

python3.pkgs.xdot

Don't know why was the purpose of the circular import comment, maybe it was outdated. Now a fix for tests and update is available in #255254 .

python3.pkgs.iniconfig

Not exactly this issue, but rather it is a form of #63168 . Improvement for the comment near it's doCheck = false; is in #255256 .

Expected behavior

No difference between pytest and python -m pytest.

Additional context

Problem was investigated also in https://discourse.nixos.org/t/python-package-testing-circular-import-in-a-version-py/33019/ .

Notify maintainers

@FRidh . Maybe python3.pkgs.orange3, and python3.pkgs.pyrevolve maintainers @lucasew and @AtilaSaraiva will be also interested.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/python-package-testing-circular-import-in-a-version-py/33019/5

[natsukium]

It is not a circular import, but this is a problem often encountered in extension modules generated by cython and others.

some examples

#240183 (review)

• use pytest directly with a flag --import-mode append

#238058 (comment)

• change the current directory

#102729 (comment)

• remove source directory

python -m pytest adds the current directory to the head of the search path, so python imports packages in $src/ instead of packages installed in $out/{python.sitePackages}/.
https://docs.pytest.org/en/7.1.x/explanation/pythonpath.html#invoking-pytest-versus-python-m-pytest
I don't think this is the expected behavior of pytestCheckHook.

[lucasew]

Just for reference:

pytestCheckHook is defined in pkgs/development/interpreters/python/hooks/pytest-check-hook.sh

And I think that -m behavior is to allow using a interpreter specific for testing. I don't know the usecases of this approach exactly as I am not the author of the hook.

Do you think that an option to disable this behaviour and use pytest directly would be better?

Like instead of that eval "@pythonCheckInterpreter@ $args" some kind of check that if a flag is enabled then pytest $args?

[doronbehar]

Like instead of that eval "@pythonCheckInterpreter@ $args" some kind of check that if a flag is enabled then pytest $args?

Definitely that would be nice, but I'm not sure that's the only thing that is required here. At astropy/astropy#15316 (comment) I summarized some my findings regarding pytest vs python -m pytest.

It is not a circular import, but this is a problem often encountered in extension modules generated by cython and others.

some examples

Thanks for the examples! Indeed for the packages I tried to update and enable tests (pyerfa and astropy) was using cd $out in preCheck. See also #255471 .

On the other hand over at #255260 a different solution was implemented eventually.