mk-python-derivation: don't expose check args when doCheck = false#327264
Merged
natsukium merged 1 commit intoNixOS:stagingfrom Sep 15, 2024
Merged
mk-python-derivation: don't expose check args when doCheck = false#327264natsukium merged 1 commit intoNixOS:stagingfrom
natsukium merged 1 commit intoNixOS:stagingfrom
Conversation
github-actions
bot
added
the
6.topic: python
ofborg
bot
added
10.rebuild-darwin-stdenv
13 tasks
This change prevents rebuilds if we change check flags when doCheck = false. It's useful, for example, when tests are separated in `passthru.tests`.
natsukium
force-pushed
the
python/rebuild-less-check-args
branch
from
d15b112 to
2e2f188
Compare
Member
Author
|
I didn't see any regression in my hydra job, so I'm going to rebase and merge this weekend. |
pbsds
approved these changes
Sep 14, 2024
Member
pbsds
left a comment
pbsds
left a comment
There was a problem hiding this comment.
Change LGTM and should achieve the intended effect. Good idea!
Comment on lines
+349
to
+355
| # `escapeShellArgs` should be used as well as `disabledTestPaths`, | ||
| # but some packages rely on existing raw strings. | ||
| disabledTests = attrs.disabledTests; | ||
| } // optionalAttrs (attrs ? pytestFlagsArray) { | ||
| pytestFlagsArray = attrs.pytestFlagsArray; | ||
| } // optionalAttrs (attrs ? unittestFlagsArray) { | ||
| unittestFlagsArray = attrs.unittestFlagsArray; |
Member
There was a problem hiding this comment.
The flags arrays should also in the future switch over to escapeShellArgs (or structured attrs), maybe a comment for those as well would be nice. I myself have done workarounds like [ "-m" "'not flaky'" ]. Not a blocker
Member
Author
There was a problem hiding this comment.
I think it is clear that similar actions are needed for other attributes.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
This change prevents rebuilds if we change check flags when doCheck = false. It's useful, for example, when tests are separated in
passthru.tests.Perhaps this concern will be addressed.
#272177 (comment)
Note: This PR should target staging, but I'm targeting master to reduce rebuilds during the review.
before
get caches from cache.nixos.org (no rebuild)
rebuild
sudachipyeven though pytest is not runafter
rebuild
sudachipywith some dependenciesNo rebuild even if overridden to add
pytestFlagsArray.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.