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

packaging: move tests to passthru #12063

Merged
merged 1 commit into from
Dec 17, 2024
Merged

packaging: move tests to passthru #12063

merged 1 commit into from
Dec 17, 2024

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Dec 15, 2024

Motivation

This is unnecessary build tests in some builds, where we don't actually need it.
It is useful to build nix without having to run tests.
Before when running vm tests we were also building unittests.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VM tests already run with

        nix.package = nixpkgsFor.${system}.native.nixComponents.nix-cli;

and nix-cli does not include tests.
(It's an input to everything.nix, not its return value)

Another possibility would be to override the doCheck and doInstallCheck attrs to false where desired.

I don't think we should do this before we have something like

@Mic92
Copy link
Member Author

Mic92 commented Dec 16, 2024

The VM tests already run with

        nix.package = nixpkgsFor.${system}.native.nixComponents.nix-cli;

They still seem to build all those tests for some reasons. Why do we need to define tests as check inputs? I don't see the advantage. If someone wants to do nix run github:NixOS/nix they shouldn't have to build tests. This is an anti-pattern that we inherited from nixpkgs, but now we could avoid.

@roberth roberth marked this pull request as draft December 16, 2024 14:11
@Mic92 Mic92 marked this pull request as ready for review December 16, 2024 14:12
@Mic92
Copy link
Member Author

Mic92 commented Dec 16, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Dec 16, 2024

queue

🛑 The pull request has been synchronized by a user

mergify bot added a commit that referenced this pull request Dec 16, 2024
mergify bot added a commit that referenced this pull request Dec 16, 2024
mergify bot added a commit that referenced this pull request Dec 16, 2024
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-12-16-nix-team-meeting-minutes-203/57483/1

@NixOS NixOS deleted a comment from mergify bot Dec 17, 2024
@NixOS NixOS deleted a comment from mergify bot Dec 17, 2024
@NixOS NixOS deleted a comment from mergify bot Dec 17, 2024
@Mic92
Copy link
Member Author

Mic92 commented Dec 17, 2024

@mergify rebase

@NixOS NixOS deleted a comment from mergify bot Dec 17, 2024
Copy link
Contributor

mergify bot commented Dec 17, 2024

rebase

✅ Branch has been successfully rebased

@mergify mergify bot merged commit 8117f16 into NixOS:master Dec 17, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants