pytestCheckHook: support inclusion and exclusion of path globs, test items, keywords, and markers by ShamrockLee · Pull Request #386513 · NixOS/nixpkgs

ShamrockLee · 2025-03-03T00:09:48Z

If applied, pytestCheckHook will accept

enabledTestPaths and disabledTestPaths to include/exclude either path globs or test items (<file path>::<specifier>)
enabledTests and disabledTests to include/exclude keywords
enabledTestMarks and disabledTestMarks to include/exclude markers

The inclusion corresponds to the positional arguments of pytest, as described by the PyTest documentation.

The path globs inside enabledTestPaths is expanded using an in-line Python script powered by the glob module from the Python Standard Library. enabledTestPaths is crucial for deprecating pytestFlagsArray, or we'll have to specify globbed paths inside preCheck. Other attributes also enhance the structural selection of the test cases.

This enhancement is essential to the tree-wide transition to deprecate the non __structuredAttrs-compatible pytestFlagsArray flags. See the previous discussion: #347194 (comment)

The keyword and markers expressions shares the same form ((inc1) or (inc2)) and not (exc1) and not (exc2), and are passed to -k and -m respectively.

I have considered renaming them to includeTests and excludeTests, but I'm unsure if tree-wide renaming from disabledTests to excludeTests would be worthwhile.

Things done

Add a 👍 reaction to pull requests you find important.

…bledTestPaths flags

…edTestPaths Pass disabledTestPaths elements containing double colons (::) to --deselect instead of --ignore-glob.

ShamrockLee · 2025-03-09T14:24:33Z

Do you think we should rename from disabledTests to excludeTests and disabledTestPaths to excludeTestPaths?

This way, we can call the whole set of arguments:

includeTestPaths, excludeTestPaths
includeTests, excludeTests
includeTestMarks, excludeTestMarks

Ping @wolfgangwalther

mweinelt

Cool.

doc/languages-frameworks/python.section.md

ShamrockLee · 2025-03-15T14:26:16Z

I improved the example based on pytest's default test discovery documented in its reference manual.

See the discussion comment for detail: #386513 (comment)

pkgs/development/interpreters/python/mk-python-derivation.nix

pkgs/development/interpreters/python/hooks/pytest-check-hook.sh

MattSturgeon

A few minor suggestions, nothing blocking

doc/languages-frameworks/python.section.md

ShamrockLee · 2025-03-23T09:47:50Z

@MattSturgeon, I addressed the suggestions and adopted the definition-list syntax.

I also add pytestFlags attribute description to the list, and merge its pytestFlags example into the main example. I moved the __structuredAttrs part to a new paragraph below the main example.

…ssions Wrap each elements of disabledTests with parenthesis so that when __structuredAttrs = true, people could use sub-expressions an element. E.g. ```nix { disabledTests = [ "ClassFoo and test_foo" "test_bar" ]; }

wolfgangwalther

Did not do any builds or tests, but code LGTM.

(short of the current CI failure for nixfmt)

…dTestMarks

Co-authored-by: Sandro <[email protected]> Co-authored-by: Matt Sturgeon <[email protected]> Co-authored-by: Wolfgang Walther <[email protected]>

ShamrockLee · 2025-03-23T15:53:53Z

BTW, the current enable* not-an-empty-list assertions only guard the buildPythonPackage (and overridePythonAttrs) arguments but not the overridden attributes by overrideAttrs, since the latter is tricky to implement.

ShamrockLee · 2025-03-23T15:57:58Z

Did not do any builds or tests, but code LGTM.

Let's see if python3Packages.pytestCheckHook.tests and python3Packages.xyzservices build.

@ofborg build nixpkgs-manual

ShamrockLee · 2025-03-23T15:59:07Z

(short of the current CI failure for nixfmt)

Addressed.

ShamrockLee · 2025-03-23T16:30:29Z

The OfBorg CI test building xyzservices on aarch64-linux fails because an inline test of perl5 doesn't pass. This is unrelated to this PR, but might hint that it's a bit unrealistic to expect python3Packages.xyzservices to build smoothly on the staging branch.

The tests should be lighter:
@ofborg build python3Packages.pytestCheckHook.tests

ShamrockLee · 2025-03-23T23:15:25Z

I cherry-picked the documentation changes onto nixos-unstable-small and the Nixpkgs Manual builds fine.

OfBorg's failures to build nixpkgs-manual on aarch64-linux seem to caused by a Perl plugin failing to pass three of its tests. That's not related to the issue.

Partial log:

error: builder for '/nix/store/bpl8lqqw67kpqd7jcqy0km6fn3rs67i9-perl5.40.0-XML-Parser-2.46.drv' failed with exit code 2;
       last 25 log lines:
       > PERL_DL_NONLAZY=1 "/nix/store/ds9x5fnzhspvcndr32i9qf5d95a3xd57-perl-5.40.0/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
       > t/astress.t ........... ok
       > t/cdata.t ............. ok
       > t/decl.t .............. ok
       > t/defaulted.t ......... ok
       > t/encoding.t .......... ok
       > t/external_ent.t ...... ok
       > t/file.t .............. ok
       > t/file_open_scalar.t .. ok
       > t/finish.t ............ ok
       > t/namespaces.t ........ ok
       > t/parament.t .......... ok
       > t/partial.t ........... Failed 1/3 subtests
       > t/skip.t .............. ok
       > t/stream.t ............ ok
       > t/styles.t ............ ok
       >
       > Test Summary Report
       > -------------------
       > t/partial.t         (Wstat: 0 Tests: 3 Failed: 1)
       >   Failed test:  3
       > Files=15, Tests=140,  2 wallclock secs ( 0.06 usr  0.03 sys +  0.73 cusr  0.49 csys =  1.31 CPU)
       > Result: FAIL
       > Failed 1/15 test programs. 1/140 subtests failed.
       > make: *** [Makefile:981: test_dynamic] Error 255

That is unrelated to this issue.

Given that python3Packages.pytestCheckHook.tests builds on aarch64-linux, and that the functionality is platform-agnostic, I'm going to merge this PR.

wolfgangwalther · 2025-05-01T19:39:46Z

pkgs/development/interpreters/python/hooks/pytest-check-hook.sh

+        else
+            # The `|| kill "$$"` trick propagates the errors from the process substitutiton subshell,
+            # which is suggested by a StackOverflow answer: https://unix.stackexchange.com/a/217643
+            readarray -t -O"${#flagsArray[@]}" flagsArray < <(@pythonCheckInterpreter@ - "$path" <<EOF || kill "$$")


I'm afraid this doesn't work as expected,

In a random hydra log, I found this error:

/nix/store/l3797z8ybpgk14ara7z0ld4hsnrpnvyv-pytest-check-hook/nix-support/setup-hook: line 41: warning: command substitution: 1 unterminated here-document

This makes sense - the here doc starts in the subshell, but ends outside.

What I am confused about even more is the fact that this doesn't break the build at this stage, yet.

I'm afraid this doesn't work as expected,

In a random hydra log, I found this error:

/nix/store/l3797z8ybpgk14ara7z0ld4hsnrpnvyv-pytest-check-hook/nix-support/setup-hook: line 41: warning: command substitution: 1 unterminated here-document

This makes sense - the here doc starts in the subshell, but ends outside.

I'll fix it this weekend.

What I am confused about even more is the fact that this doesn't break the build at this stage, yet.

I guess it's because the syntax error require a missing EOF for the heredoc, which is only valid when taking account the whole line. Since it the doesn't happen before the || operator, it isn't caught by the kill command.

I guess it's because the syntax error require a missing EOF for the heredoc, which is only valid when taking account the whole line. Since it the doesn't happen before the || operator, it isn't caught by the kill command.

Ah, ok! So it's a demonstration of why we need the kill:) (for other, non-syntax errors, that is)

@wolfgangwalther Here is the fix: #403973.

ShamrockLee · 2025-07-14T19:54:02Z

I proposed in #425191 to add a default value enabledTestPaths = [ "." ] to simplify overriding (making it always specified as a non-empty list). What do you think about the change?

ShamrockLee requested a review from wolfgangwalther March 3, 2025 00:09

github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 3, 2025

ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 3, 2025

ShamrockLee force-pushed the build-python-include-tests branch from 4501097 to c5c9dab Compare March 7, 2025 08:58

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 7, 2025

ShamrockLee force-pushed the build-python-include-tests branch 2 times, most recently from d9e5f0f to 80b35f2 Compare March 7, 2025 10:48

ShamrockLee added 2 commits March 9, 2025 22:20

python3Packages.pytestCheckHook: append disableTests flags after disa…

6bdb735

…bledTestPaths flags

python3Packages.pytestCheckHook: support deselecting tests via disabl…

16f8b50

…edTestPaths Pass disabledTestPaths elements containing double colons (::) to --deselect instead of --ignore-glob.

ShamrockLee force-pushed the build-python-include-tests branch from 80b35f2 to a238b9d Compare March 9, 2025 14:20

ShamrockLee force-pushed the build-python-include-tests branch from a238b9d to 8c64725 Compare March 9, 2025 16:07

github-actions bot added the 8.has: documentation This PR adds or changes documentation label Mar 9, 2025

ShamrockLee force-pushed the build-python-include-tests branch from 8c64725 to 8c0090b Compare March 9, 2025 16:08

ShamrockLee marked this pull request as ready for review March 9, 2025 16:12

nix-owners bot requested review from autra, imincik, l0b0, mweinelt, natsukium, nh2, nialov, sikmir and willcohen March 9, 2025 16:13

mweinelt reviewed Mar 9, 2025

View reviewed changes

doc/languages-frameworks/python.section.md Outdated Show resolved Hide resolved

mweinelt requested a review from fabaff March 9, 2025 16:35

wolfgangwalther reviewed Mar 16, 2025

View reviewed changes

pkgs/development/interpreters/python/mk-python-derivation.nix Outdated Show resolved Hide resolved

pkgs/development/interpreters/python/mk-python-derivation.nix Outdated Show resolved Hide resolved

pkgs/development/interpreters/python/hooks/pytest-check-hook.sh Outdated Show resolved Hide resolved

MattSturgeon approved these changes Mar 16, 2025

View reviewed changes

wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 16, 2025

ShamrockLee force-pushed the build-python-include-tests branch from 75b99b9 to 9417ba1 Compare March 23, 2025 09:43

ShamrockLee requested a review from MattSturgeon March 23, 2025 09:48

ShamrockLee force-pushed the build-python-include-tests branch from 9417ba1 to 05a9d04 Compare March 23, 2025 15:09

ShamrockLee added 2 commits March 23, 2025 23:10

python3Packages.pytestCheckHook: support enabledTestPaths

4174f6e

ShamrockLee force-pushed the build-python-include-tests branch from 05a9d04 to 8ba0944 Compare March 23, 2025 15:10

wolfgangwalther approved these changes Mar 23, 2025

View reviewed changes

ShamrockLee and others added 4 commits March 23, 2025 23:51

python3Packages.pytestCheckHook: support enabledTests

f7aeb5a

python3Packages.pytestCheckHook: support enabledTestMarks and disable…

e1e072b

…dTestMarks

python3Packages.xyzservices: use disabledTestMarks

8ebcc32

doc: python.section.md: partial rewrites for pytest and pytestCheckHook

bb76d74

Co-authored-by: Sandro <[email protected]> Co-authored-by: Matt Sturgeon <[email protected]> Co-authored-by: Wolfgang Walther <[email protected]>

ShamrockLee force-pushed the build-python-include-tests branch from 8ba0944 to bb76d74 Compare March 23, 2025 15:51

wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 23, 2025

MattSturgeon approved these changes Mar 23, 2025

View reviewed changes

MattSturgeon added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Mar 23, 2025

ShamrockLee merged commit d3a452a into NixOS:staging Mar 23, 2025
31 of 36 checks passed

wolfgangwalther reviewed May 1, 2025

View reviewed changes

ShamrockLee mentioned this pull request May 3, 2025

python3Packages.pytestCheckHook: fix Bash heredoc EOF outside subshell #403973

Merged

13 tasks

ShamrockLee mentioned this pull request Jul 14, 2025

python3Packages.buildPythonPackage: default enabledTestPaths = [ "." ] #425191

Merged

13 tasks

Uh oh!

Conversation

ShamrockLee commented Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

ShamrockLee commented Mar 9, 2025

Uh oh!

mweinelt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShamrockLee commented Mar 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MattSturgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShamrockLee commented Mar 23, 2025

Uh oh!

wolfgangwalther left a comment

Choose a reason for hiding this comment

Uh oh!

ShamrockLee commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShamrockLee commented Mar 23, 2025

Uh oh!

ShamrockLee commented Mar 23, 2025

Uh oh!

ShamrockLee commented Mar 23, 2025

Uh oh!

ShamrockLee commented Mar 23, 2025

Uh oh!

Uh oh!

wolfgangwalther May 1, 2025

Choose a reason for hiding this comment

Uh oh!

ShamrockLee May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolfgangwalther May 2, 2025

Choose a reason for hiding this comment

Uh oh!

ShamrockLee May 3, 2025

Choose a reason for hiding this comment

Uh oh!

ShamrockLee commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ShamrockLee commented Mar 3, 2025 •

edited

Loading

ShamrockLee commented Mar 23, 2025 •

edited

Loading

ShamrockLee May 2, 2025 •

edited

Loading

ShamrockLee commented Jul 14, 2025 •

edited

Loading