Add pkgs.nixosTest #47684

roberth · 2018-10-02T20:44:56Z

Motivation for this change

This makes writing NixOS tests easier and faster for project that use Nixpkgs, because it reuses the applied pkgs.

(The fast aspect was available since config.nixpkgs.pkgs but its use is unlikely to have spread much)

This now also includes a test for pkgs.nixos.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

xeji · 2018-10-02T21:25:11Z

cc @aszlig

roberth · 2018-10-02T21:32:23Z

~~I also have a test for this and pkgs.nixos but those tests depend on #47430~~

roberth · 2018-10-23T15:53:04Z

@GrahamcOfBorg build tests.nixos-functions.nixosTest-test tests.nixos-functions.nixos-test

GrahamcOfBorg · 2018-10-23T15:54:59Z

Success on x86_64-linux (full log)

Attempted: tests.nixos-functions.nixosTest-test, tests.nixos-functions.nixos-test

Partial log (click to expand)

machine# [   15.629935] systemd[1]: Listening on Load/Save RF Kill Switch Status /dev/rfkill Watch.
machine# [   15.652585] systemd[1]: Started Name Service Cache Daemon.
machine: exit status 0
test script finished in 17.99s
cleaning up
killing machine (pid 597)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/fqml68ffajk250dqj3rq4gfpd0nzsn35-vm-test-run-nixosTest-test
/nix/store/clq7f02ijzsdallwvf0999qgh4ccb93h-nixos-system-nixos-19.03.git.bba4655

GrahamcOfBorg · 2018-10-23T15:57:09Z

Success on aarch64-linux (full log)

Attempted: tests.nixos-functions.nixosTest-test, tests.nixos-functions.nixos-test

Partial log (click to expand)

building '/nix/store/2qq81mmf4kc75d2vz1vjza0w2cln5jsg-unit-polkit.service.drv'...
building '/nix/store/7vrv7477qqwpbgnw2ka19y47hf5nz6w4-unit-systemd-fsck-.service.drv'...
building '/nix/store/5ny207c93rri58nds5a85g1hydiqz2w8-unit-dbus.service.drv'...
building '/nix/store/r3ywvinmx5x2vfh3s38mjg87hq4b68n9-unit-dbus.service.drv'...
building '/nix/store/daiily46ya44kk4q5h5dgs2y1vry9p97-system-units.drv'...
building '/nix/store/zaflv7ffp4sj4p7jznb77rnr0cp663vy-user-units.drv'...
building '/nix/store/zmnn9a8gpwq1yi73f32fxy3nwnylpvdj-etc.drv'...
building '/nix/store/vmbf1zkrqza5ikpvwb12l207q4mifcz6-nixos-system-nixos-19.03.git.bba4655.drv'...
/nix/store/1l8lgk0ix55yqczawszyaaiiwpwkg6f0-vm-test-run-nixosTest-test
/nix/store/qgfi5vyy2dncq5cvyk98xgxmj7x5zqk4-nixos-system-nixos-19.03.git.bba4655

Profpatsch

I’m a bit out of my league when it comes to full evaluation of test frameworks and nixos configs, but I highlighted parts where I see potential problems.
You should get one or two reviewers that are more familiar with this parts of nixpkgs, @edolstra certainly is.

nixos/lib/testing.nix

pkgs/test/nixos-functions/default.nix

pkgs/top-level/all-packages.nix

Profpatsch · 2018-10-25T12:46:19Z

pkgs/top-level/all-packages.nix

+                       else test;
+          calledTest = if pkgs.lib.isFunction loadedTest
+                       then callPackage loadedTest {}
+                       else loadedTest;


Are these really necessary? The user can add the import and callPackage if needed. Plus, if I see this correctly, the global callPackage from all-packages.nix is used here, which is scoped to a specific version of pkgs.

This is desirable to be compatible with testing.nix/makeTest that people are used to.
Using a specific version ('the version') of pkgs is the point of this function. It's called as pkgs.nixosTest. If you want to use another pkgs, you call otherPkgs.nixosTest or do the deep import of testing.nix or similar that people have been doing.

Profpatsch · 2018-10-25T12:48:40Z

pkgs/top-level/all-packages.nix

+                       then callPackage loadedTest {}
+                       else loadedTest;
+        in
+          nixosTesting.makeTest calledTest;


Hm, the function only accepts one test. I don’t know how much sharing is done internally, this might mean a hefty evaluation for each test if you want to run multiple tests. cc @aszlig maybe.

This reuses an applied Nixpkgs (pkgs), so that will be efficient. Evaluation nixos configurations is inevitable because each machine in a nixos test is unique.

Thanks Profpatsch, I believe I have addressed your review. Would you mind having a look at my comments?

aszlig

While I really appreciate making a few things a bit more overridable, applying nixpkgs.pkgs to all machines really is a blocker here, because it makes tests behave differently than by using <nixpkgs/nixos/tests/make-test.nix> or even <nixpkgs/nixoslib/testing.nix>.

aszlig · 2018-10-27T03:11:20Z

pkgs/top-level/all-packages.nix

+          inherit pkgs;
+          extraConfigurations = [(
+            { lib, ... }: {
+              config.nixpkgs.pkgs = lib.mkDefault pkgs;


This is a bit tricky here, because the default in the nixpkgs NixOS module is:

nixpkgs/nixos/modules/misc/nixpkgs.nix

Lines 68 to 70 in 591ba73

default = import ../../.. {

inherit (cfg) config overlays localSystem crossSystem;

};

Setting this to pkgs here has the effect that overlays and config now no longer work and test machines will be built with whatever overlays were set outside of their machine definition.

I'd say this not only leads to surprising behavior when overlays are no longer working but also could lead to tests failing because you have some pretty weird override somewhere in your ~/.config/nixpkgs/overlays.nix when going the impure import route (which is the default right now).

@aszlig I have found a solution for the overlay problem in #49256.

This function does not introduce import impurities (NIX_PATH); it should actually make it easier to avoid accidental import impurities because it calls the NixOS files with the right arguments that avoid impure defaults.

Thanks aszlig for your review. You have made it very clear that I needed to improve the documentation wrt handling of pkgs. The goal is not to replace anything in NixOS but to add a new way to invoke NixOS tests that is both more accessible and more efficient.

aszlig · 2018-10-27T23:52:56Z

Documenting this limitation (which is still the case, not only for config but because just appending overlays won't suddenly make things more deterministic as the existing overlays are still in effect) will not suddenly prevent people from using this and once that lands in nixpkgs, people probably will build things around this, so I think we have to be really careful whether we really want something like this.

What would be the actual use cases where importing <nixpkgs/nixos/tests/make-test.nix> or <nixpkgs/nixos/lib/testing.nix> wouldn't work?

roberth · 2018-10-28T00:54:04Z

just appending overlays won't suddenly make things more deterministic as the existing overlays are still in effect

The logic in this PR is entirely deterministic. If someone wants to have a fresh Nixpkgs, they should just import it. Having pkgs.nixosTest ignore pkgs defeats the purpose of putting it in pkgs.

so I think we have to be really careful whether we really want something like this.

It makes sense for a project to use Nixpkgs as a central part of their Nix expressions. Having functions like nixos and nixosTest there makes it easy to create images and tests based on the project-specific pkgs. Setting this up used to require a deep dive into the Nixpkgs/NixOS directory tree, which is a waste and likely results in ad-hoc solutions that are worse than this.

What would be the actual use cases where importing <nixpkgs/nixos/tests/make-test.nix> or <nixpkgs/nixos/lib/testing.nix> wouldn't work?

These don't work if you only have a pkgs attrset with custom stuff inside. With master, you have to go out of your way to insert it into the NixOS configs.
Another use case is where you have many machines that need to be evaluated. With master, if you use those imports, you will re-apply Nixpkgs for each machine, which will waste a lot of time producing the same pkgs over and over.

roberth · 2018-10-28T01:10:24Z

Another very important reason to have this function is to make it easy for beginners to get started with writing NixOS tests for their own projects. It's really a 'killer feature' of the Nix ecosystem so it should be easy to get started with.
Would you really want to point people at these weird, undocumented deep imports?

This function is easy to get started with and it scales well into 'serious projects' and setups with a company overlay, for example.

The good news is that it worked as expected.

nixos/lib/build-vms.nix

matthewbauer

Looks good! I still think the extraConfigurations thing is awkward but it’s okay for now

edolstra · 2018-11-15T16:16:35Z

These kinds of attributes should not be included in all-packages.nix! They confuse tools like nix-env and Hydra (see e.g. #50405) and are IMHO confusing to users (e.g. why is something that is not a package in something called pkgs?).

Would you really want to point people at these weird, undocumented deep imports?

It may be undocumented but so is an attribute in all-packages.nix. And importing a file is less weird than getting a function for testing from something called pkgs. (It's also no weirder than, say, #include <stdlib.h>.)

BTW, the flake mechanism should provide a better way to handle this by giving tools like nix-env a dedicated entry point into Nixpkgs.

roberth · 2018-11-15T17:21:40Z

Same applies to runCommand, I'd say.

I hope flakes aren't too heavy a solution. How are those coming along?

edolstra · 2018-11-15T17:44:50Z

Yes, runCommand was also a mistake. But let's not use that as a reason for making things worse.

domenkozar · 2018-11-15T17:49:16Z

Shouldn't there be a way to tell nix-env to exclude something to be installable? We have a way to do that for Hydra.

Mic92 · 2018-11-15T18:16:31Z

This also breaks nixops:

error: anonymous function at /home/joerg/git/nixpkgs/nixos/lib/testing.nix:1:1 called without required argument 'pkgs', at /home/joerg/git/nixops/nix/eval-machine-info.nix:9:6
error: evaluation of the deployment specification failed

Mic92 · 2018-11-15T18:20:22Z

nixops breakage documented here: #50419

roberth · 2018-11-15T19:51:08Z

No, this doesn't break NixOps. #50233 broke NixOps.

Everything in Nixpkgs is a 'public interface'.

Mic92 · 2018-11-15T19:54:12Z

yes unfortunately.

roberth · 2022-07-11T06:42:47Z

BTW, the flake mechanism should provide a better way to handle this by giving tools like nix-env a dedicated entry point into Nixpkgs.

It's ~4 years later and this can be discussed now.

packages' lack of nesting makes it unsuitable as a replacement for "pkgs", yet we don't have a place for functions that have pkgs in their closure.

roberth added 0.kind: enhancement 6.topic: nixos 8.has: documentation 6.topic: user experience labels Oct 2, 2018

roberth requested a review from domenkozar October 2, 2018 20:44

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 8.has: documentation labels Oct 2, 2018

roberth removed the request for review from domenkozar October 23, 2018 15:06

roberth force-pushed the nixpkgs-nixosTest branch from 2aa1e86 to bba4655 Compare October 23, 2018 15:39

roberth requested review from Profpatsch and aszlig October 23, 2018 15:50

GrahamcOfBorg added 8.has: package (new) 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 labels Oct 23, 2018

Profpatsch previously requested changes Oct 25, 2018

View reviewed changes

aszlig previously requested changes Oct 27, 2018

View reviewed changes

roberth mentioned this pull request Oct 27, 2018

NixOS: use overlays when nixpkgs.pkgs is set #49256

Merged

9 tasks

roberth added 2 commits November 13, 2018 10:43

Add pkgs.nixosTest

cdca66d

Add tests for pkgs.nixos and pkgs.nixosTest

933c95c

roberth added 4 commits November 13, 2018 10:43

tests: Don't use pkgs.extend. OfBorg will reject it.

5d594d7

The good news is that it worked as expected.

pkgs.nixosTest: format

d82e152

pkgs/test/nixos-functions: Add inline doc

3783f2d

pkgs.nixosTest: Emphasize propagation of pkgs

5a8bddf

roberth force-pushed the nixpkgs-nixosTest branch from 886e72f to 5a8bddf Compare November 13, 2018 09:54

roberth mentioned this pull request Nov 13, 2018

pkgs.nixosTests doesn't make sense #50301

Closed

matthewbauer reviewed Nov 13, 2018

View reviewed changes

nixos/lib/build-vms.nix Show resolved Hide resolved

matthewbauer approved these changes Nov 14, 2018

View reviewed changes

matthewbauer merged commit 0874ee8 into NixOS:master Nov 14, 2018

Mic92 mentioned this pull request Nov 15, 2018

nixops broken in master #50419

Closed

roberth mentioned this pull request Jul 11, 2022

Provide explanation for usage of pkgs.nixos instead of lib.nixosSystem hercules-ci/flake-parts#47

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pkgs.nixosTest #47684

Add pkgs.nixosTest #47684

roberth commented Oct 2, 2018 •

edited

xeji commented Oct 2, 2018

roberth commented Oct 2, 2018 •

edited

roberth commented Oct 23, 2018

GrahamcOfBorg commented Oct 23, 2018

GrahamcOfBorg commented Oct 23, 2018

Profpatsch left a comment

Profpatsch Oct 25, 2018

roberth Oct 25, 2018

Profpatsch Oct 25, 2018

roberth Oct 25, 2018

aszlig left a comment

aszlig Oct 27, 2018

roberth Oct 27, 2018 •

edited

aszlig commented Oct 27, 2018

roberth commented Oct 28, 2018

roberth commented Oct 28, 2018

matthewbauer left a comment

edolstra commented Nov 15, 2018

roberth commented Nov 15, 2018

edolstra commented Nov 15, 2018

domenkozar commented Nov 15, 2018

Mic92 commented Nov 15, 2018

Mic92 commented Nov 15, 2018

roberth commented Nov 15, 2018

Mic92 commented Nov 15, 2018

roberth commented Jul 11, 2022

	default = import ../../.. {
	inherit (cfg) config overlays localSystem crossSystem;
	};

Add pkgs.nixosTest #47684

Add pkgs.nixosTest #47684

Conversation

roberth commented Oct 2, 2018 • edited

Motivation for this change

Things done

xeji commented Oct 2, 2018

roberth commented Oct 2, 2018 • edited

roberth commented Oct 23, 2018

GrahamcOfBorg commented Oct 23, 2018

GrahamcOfBorg commented Oct 23, 2018

Profpatsch left a comment

Choose a reason for hiding this comment

Profpatsch Oct 25, 2018

Choose a reason for hiding this comment

roberth Oct 25, 2018

Choose a reason for hiding this comment

Profpatsch Oct 25, 2018

Choose a reason for hiding this comment

roberth Oct 25, 2018

Choose a reason for hiding this comment

aszlig left a comment

Choose a reason for hiding this comment

aszlig Oct 27, 2018

Choose a reason for hiding this comment

roberth Oct 27, 2018 • edited

Choose a reason for hiding this comment

aszlig commented Oct 27, 2018

roberth commented Oct 28, 2018

roberth commented Oct 28, 2018

matthewbauer left a comment

Choose a reason for hiding this comment

edolstra commented Nov 15, 2018

roberth commented Nov 15, 2018

edolstra commented Nov 15, 2018

domenkozar commented Nov 15, 2018

Mic92 commented Nov 15, 2018

Mic92 commented Nov 15, 2018

roberth commented Nov 15, 2018

Mic92 commented Nov 15, 2018

roberth commented Jul 11, 2022

roberth commented Oct 2, 2018 •

edited

roberth commented Oct 2, 2018 •

edited

roberth Oct 27, 2018 •

edited