Skip to content

Comments

nixos/testing: Fix release.nix tests evaluation#193485

Merged
K900 merged 1 commit intoNixOS:masterfrom
hercules-ci:nixos-lib-runTest-return-test
Sep 29, 2022
Merged

nixos/testing: Fix release.nix tests evaluation#193485
K900 merged 1 commit intoNixOS:masterfrom
hercules-ci:nixos-lib-runTest-return-test

Conversation

@roberth
Copy link
Member

@roberth roberth commented Sep 29, 2022

Description of changes

Fixes the problem introduced by 12b3066 which caused nixos/release.nix to return the wrong attributes, while intending to only affect nixos/lib's runTest.
This also removes callTest from the test options, because callTest is only ever invoked by all-tests.nix.

Things done

Manually tested in nix repl, covering multiple "styles" of test invocation.

release.nix, nixosTests:

  • acme
  • agda
  • avahi
  • simple
  • proxy

pkgs.tests.testers.nixosTest-example
runTest { nodes = {}; name = "foo"; testScript = ""; hostPkgs = import ./. {}; }

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Fixes the problem introduced by 12b3066
which caused nixos/release.nix to return the wrong attributes, while
intending to only affect nixos/lib's runTest.
This also removes callTest from the test options, because callTest is
only ever invoked by all-tests.nix.
@roberth roberth requested a review from vcunat September 29, 2022 08:53
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Sep 29, 2022
@roberth
Copy link
Member Author

roberth commented Sep 29, 2022

@ofborg test acme agda avahi proxy simple

@ofborg build nixosTests.acme nixosTests.agda nixosTests.avahi nixosTests.proxy nixosTests.simple tests.testers.nixosTest-example

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 29, 2022
@vcunat vcunat added the 1.severity: channel blocker Blocks a channel label Sep 29, 2022
@K900 K900 merged commit f54f12a into NixOS:master Sep 29, 2022
@K900
Copy link
Contributor

K900 commented Sep 29, 2022

@roberth
Copy link
Member Author

roberth commented Sep 29, 2022

Different problem #193175 (comment)

@roberth roberth removed the 1.severity: channel blocker Blocks a channel label Sep 29, 2022
@Ma27
Copy link
Member

Ma27 commented Sep 29, 2022

@roberth This breaks numerous tests such as the kenrel tests:

❯ ~/Projects/nixpkgs-prs remotes/origin/linux-kernel-updates → nix-build nixos/tests/kernel-generic.nix -A linux_4_9 -A linux_4_19 -A linux_4_14 -A linux_4_14_hardened -j0
error: The option `machine' is used but not defined.
(use '--show-trace' to show detailed location information)

@Ma27 Ma27 mentioned this pull request Sep 29, 2022
13 tasks
@roberth
Copy link
Member Author

roberth commented Oct 1, 2022

@Ma27 here's a fix: #193917

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Oct 1, 2022
Within NixOS#193485 (and the previous changes) the internal structure of the
testing driver was changed. Since then, `makeTest` returns the
attributes for the VM test(s) (including `driverInteractive`) inside a
sub-attribute called `test`, so without this change running
`nixos-build-vms` would fail like this:

    error: attribute 'driverInteractive' missing
@Ma27 Ma27 mentioned this pull request Oct 1, 2022
13 tasks
@dhess
Copy link
Contributor

dhess commented Oct 12, 2022

I believe that this change, or perhaps one of the several recent related changes to testing, have broken the ability to use the NixOS Python testing framework from third-party sources when combined with recurseForDerivations.

For example, I rigged up this convenience function when we switched our nixpkgs overlay project to a flake. It allows you to point to a directory full of NixOS Python tests to create an attrset from them:

https://github.com/hackworthltd/hacknix/blob/479e37c977a16c8991cdaa060aa90e77d732b6a8/nix/overlays/114-lib-testing.nix#L5

Then, in our flake's hydraJobs attribute, we add the attrset to hydraJobs.tests (nested):

https://github.com/hackworthltd/hacknix/blob/ed730b35040f293d06dbb402f552c3b1faacdb0a/flake.nix#L348

Finally, we use another little hack I wrote so that you can run nix-instantitate -A ciJobs and get Hydra-like recursive evaluation of the entire nested hydraJobs attrset:

https://github.com/hackworthltd/hacknix/blob/ed730b35040f293d06dbb402f552c3b1faacdb0a/flake.nix#L406
https://github.com/hackworthltd/hacknix/blob/74c9287d2ad90c9b86e6e71e4d1985b2b2e0f048/nix/overlays/113-lib-flakes.nix#L100

(@roberth, I think I cribbed the recurseIntoHydraJobs from something similar you wrote for Hercules.)

Anyway, this used to work fine, but now, when it evaluates the Python test attrset, it blows up:

 nix-instantiate -A ciJobs --dry-run --show-trace                                                                           ~/git/hacknix
trace: warning: system.stateVersion is not set, defaulting to 22.11. Read why this matters on https://nixos.org/manual/nixos/stable/options.html#opt-system.stateVersion.
trace: warning: system.stateVersion is not set, defaulting to 22.11. Read why this matters on https://nixos.org/manual/nixos/stable/options.html#opt-system.stateVersion.
error: Please be informed that this pseudo-package is not the only part of
       Nixpkgs that fails to evaluate. You should not evaluate entire Nixpkgs
       without some special measures to handle failing packages, like those taken
       by Hydra.


       … while evaluating the attribute 'AAAAAASomeThingsFailToEvaluate'

       at /nix/store/p56cgddkmqqms9srh36w3kna9yrp5dvc-source/pkgs/top-level/all-packages.nix:108:3:

          107|   ### Evaluating the entire Nixpkgs naively will fail, make failure fast
          108|   AAAAAASomeThingsFailToEvaluate = throw ''
             |   ^
          109|     Please be informed that this pseudo-package is not the only part of

Granted, it's not the most elegant Nix code I've ever written, but it did do the job. I'm not exactly sure why this is happening all of a sudden, but it seems to have something to do with the nested test derivations returning more attributes than they used to, or something like that?

For reference this is showing up with e0ed589. The last known-good nixpkgs pin is 8ba1204.

@roberth
Copy link
Member Author

roberth commented Oct 12, 2022

Hi @dhess,

I'm sorry to have broken your code. I've tried hard to keep all the entrypoints to the test suite and test framework working, but I didn't find a satisfactory solution for testing-python.nix.
The good news is that we now have an official, documented, stable entrypoint for vm tests.
I'll see if I can come up with an "unsatisfactory" solution to bridge the gap and have a proper deprecation after 22.05 is EOL.

@dhess
Copy link
Contributor

dhess commented Oct 12, 2022

Hi @robert, absolutely no need for an apology.

I'm thrilled to see there's an official way to do this now. Let me see if I can get that working with recurseForDerivations.

@roberth
Copy link
Member Author

roberth commented Oct 12, 2022

Oh, forgot to mention that nixos-lib is nixpkgsFlake.lib.nixos.
Apparently we're not documenting flake stuff until it's stable, which I disagree with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants