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

Don't splice NixOS tests into packages #222

Open
3 of 4 tasks
fricklerhandwerk opened this issue May 3, 2024 · 3 comments
Open
3 of 4 tasks

Don't splice NixOS tests into packages #222

fricklerhandwerk opened this issue May 3, 2024 · 3 comments
Assignees
Labels
maintenance Cleanup, refactoring, improving discoverability, tending to continuos integration

Comments

@fricklerhandwerk
Copy link
Collaborator

fricklerhandwerk commented May 3, 2024

Problem

The code in flake.nix is tying knots, which makes it more mysterious than necessary:

ngipkgs/flake.nix

Lines 83 to 93 in 425f4bc

importPackages = pkgs: let
nixosTests = pick.tests (importProjects {pkgs = pkgs // allPackages;});
callPackage = pkgs.newScope (
allPackages // {inherit callPackage nixosTests;}
);
allPackages = import ./pkgs/by-name {
inherit (pkgs) lib;
inherit callPackage dream2nix pkgs;
};

and also happens to produce redundant test jobs:

ngipkgs/flake.nix

Lines 212 to 223 in 425f4bc

hydraJobs = let
passthruTests = concatMapAttrs (name: value:
if value ? passthru.tests
then {${name} = value.passthru.tests;}
else {})
nonBrokenPkgs;
in {
packages.${system} = nonBrokenPkgs;
tests.${system} = {
passthru = passthruTests;
nixos = pick.tests (importProjects {pkgs = pkgs // nonBrokenPkgs;});
};

The value of binding NixOS tests to derivation builds is questionable, as in this example

tests = {
inherit (nixosTests.Kbin) kbin;
};

because we're collecting all of them for the top-level projects anyway. And we're also primarily interested in "projects" working as a whole, which almost always requires complex test setups that aren't provided upstream, and have little to do with "the derivation builds".

Proposal

  • Remove nixosTests from package recipe inputs
  • Only run "regular" (non-VM) tests in passthru.tests
  • Simplify the top-level wiring
  • Add contributor documentation that says where to put which kind of test and what the trade-offs are
@fricklerhandwerk fricklerhandwerk added the maintenance Cleanup, refactoring, improving discoverability, tending to continuos integration label May 3, 2024
@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented May 3, 2024

I agree. But I fail to understand "produce redundant test jobs". NixOS tests are not under .#checks, but they are under .#hydraJobs. The lines you are referring to is just the code that exports NixOS tests under .#hydraJobs. Only the mechanism to collect them should be changed to use projects/* directly instead of passthrus, but we need to export them somewhere, right?

Just for reference: #28 is about the introduction of NixOS tests. I initially wanted to put them in .#checks, the argument against it is that "nix check should be fast". Then there was no standard output for NixOS tests and we opted for passthrus, as this is closer to nixpkgs. (Note that I am not arguing against the issue at hand, just trying to give some context.)

@wegank
Copy link
Member

wegank commented May 3, 2024

Yes. I suggest .#nixosTests, as in Nixpkgs.

Opting for passthru.test is probably not closer to Nixpkgs, since according to pkgs/README.md,

  • nixosTests.some-project are for NixOS module tests, which spawn NixOS VMs;
  • some-package.passthru.tests are for NixOS package tests, which do not.

While there's indeed a section on

It is purely optional, packages like Guix don't follow that.

People only do that because in Nixpkgs, ofborg does @ofborg build some-package some-package.passthru.tests by default, and if one doesn't link module tests to the package, they'll have to manually invoke @ofborg test some-project for them, so that makes a difference. But this doesn't apply here.

@lorenzleutgeb
Copy link
Member

While there's indeed a section on

Yup, and before we had /projects it made somewhat sense to attach NixOS tests to the "main" package for a project. Anyway, I'm all for it :)

@fricklerhandwerk fricklerhandwerk assigned cleeyv and unassigned wegank Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Cleanup, refactoring, improving discoverability, tending to continuos integration
Projects
None yet
Development

No branches or pull requests

4 participants