nixos/test-driver: fix #153766

[Synthetica9]

Motivation for this change

Fixes #153766, alternative to #153854

Things done

• 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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.05 Release Notes (or backporting 21.11 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.

[marijanp]

I also thought about making the out-dir an optional parameter, defaulting to the PWD. Now the problem I thought of was, if a user runs the driver inside a directory without write-permission. The test-run will fail pretty late i.e. when screenshot or copy_from_vm is called. I found it nicer to fail as early as possible. The case can occur e.g. when someone does this:

nix-build . -A nixosTests.<test>.driver
cd result/bin
./nixos-test-driver

Here the result dir is a symlink to the nix store obviously...

[Synthetica9]

agree, but I think that check should be happening in the initialization of Driver, not in parsing the arguments. I rebased and pushed a new commit to fix this, please verify

[marijanp]

Can we get rid of all accesses to environment variables, I already did that in https://github.com/NixOS/nixpkgs/pull/153854/files
And you will also need @tfc s approval as he's the code owner

[tfc]

Hi, i tend to prefer the other PR, because it is already cleaned out from env var usage, and doesn't mix in the image stuff (which should be individually tested and merged for the record imho).

[Synthetica9]

I have some thoughts on the other one, but I'll leave them there.