Mic92 / nixpkgs-review

Review pull-requests on https://github.com/NixOS/nixpkgs
MIT License
373 stars 60 forks source link

(new) nixos test not run automatically #178

Open davidak opened 3 years ago

davidak commented 3 years ago

The features list includes "allow to build nixos tests", so i expect that a nixos test, that is added in a PR, get executed.

That seem not to be the case:

[davidak@gaming:~/code/nixpkgs]$ nix run -f channel:nixos-unstable nixpkgs-review -c nixpkgs-review pr --post-result --token ***** 103705
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/103705/head:refs/nixpkgs-review/1
remote: Enumerating objects: 11, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 14 (delta 7), reused 7 (delta 7), pack-reused 3
Unpacking objects: 100% (14/14), 191.38 KiB | 1.08 MiB/s, done.
From https://github.com/NixOS/nixpkgs
   2a1bda47f7a..1b115789685  master     -> refs/nixpkgs-review/0
$ git worktree add /home/davidak/.cache/nixpkgs-review/pr-103705-1/nixpkgs 1b1157896857ae1467f5b0a421dee86388a55638
Preparing worktree (detached HEAD 1b115789685)
Updating files: 100% (24735/24735), done.
HEAD is now at 1b115789685 Merge pull request #115062 from manveru/cbonsai
$ git merge --no-commit 10fa80fd300ce9cd081b578621861332ddf3e411
Auto-merging pkgs/top-level/all-packages.nix
Auto-merging nixos/tests/all-tests.nix
Auto-merging nixos/modules/module-list.nix
Automatic merge went well; stopped before committing as requested
$ nix --experimental-features nix-command build --no-link --keep-going --option build-use-sandbox relaxed -f /home/davidak/.cache/nixpkgs-review/pr-103705-1/build.nix

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/103705

1 package blacklisted:
tests.nixos-functions.nixos-test

2 packages built:
plik plikd

error: --- Error ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ nix
build log of '/nix/store/immmi2c41zdcxzrw1bii0qv4q9m20f5x-nixos-system-nixos-test.drv' is not available
Posting result comment on https://github.com/NixOS/nixpkgs/pull/103705
$ nix-shell /home/davidak/.cache/nixpkgs-review/pr-103705-1/shell.nix

Or is it expected behavior that nixos tests get blacklisted and have to get enabled manually?

In that case, please add an explanation to the output and instructions how to run them. Maybe even interactive from within the nix-shell.

Running tests like described in the Readme works.

[davidak@gaming:~/code/nixpkgs]$ nix run -f channel:nixos-unstable nixpkgs-review -c nixpkgs-review pr --post-result --token ***** -p nixosTests.plikd 103705
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0 pull/103705/head:refs/nixpkgs-review/1
remote: Enumerating objects: 51, done.
remote: Counting objects: 100% (43/43), done.
remote: Compressing objects: 100% (13/13), done.
remote: Total 25 (delta 20), reused 16 (delta 12), pack-reused 0
Unpacking objects: 100% (25/25), 8.39 KiB | 859.00 KiB/s, done.
From https://github.com/NixOS/nixpkgs
   1b115789685..7f76da8f7a4  master     -> refs/nixpkgs-review/0
$ git worktree add /home/davidak/.cache/nixpkgs-review/pr-103705-2/nixpkgs 7f76da8f7a497f216ca02a673fb4e051f0ed8754
Preparing worktree (detached HEAD 7f76da8f7a4)
Updating files: 100% (24735/24735), done.
HEAD is now at 7f76da8f7a4 Merge pull request #115118 from obsidiansystems/prebuilt-android-fix-eval
$ git merge --no-commit 10fa80fd300ce9cd081b578621861332ddf3e411
Auto-merging pkgs/top-level/all-packages.nix
Auto-merging nixos/tests/all-tests.nix
Auto-merging nixos/modules/module-list.nix
Automatic merge went well; stopped before committing as requested
$ nix --experimental-features nix-command build --no-link --keep-going --option build-use-sandbox relaxed -f /home/davidak/.cache/nixpkgs-review/pr-103705-2/build.nix

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/103705

1 tests built:
nixosTests.plikd

Posting result comment on https://github.com/NixOS/nixpkgs/pull/103705
$ nix-shell /home/davidak/.cache/nixpkgs-review/pr-103705-2/shell.nix

Thanks for providing this great tool!

Mic92 commented 3 years ago

tests.nixos-functions.nixos-test is not what you expect. It is not a particular nixos test but a function that we should not try to build. Did you expect with this package a particular nixos service to be run?

davidak commented 3 years ago

I have seen that the PR adds a nixos test and

1 package blacklisted:
tests.nixos-functions.nixos-test

in the log, so my conclusion was that that is related to the added nixos test.

The "allow to build nixos tests" in the readme can be understood as it is an option to run nixos tests and it is described how to do so.

So my question is, would it make sense to run them automatically? I think that would make sense when you review and test a PR.

What do you think?

Mic92 commented 3 years ago

Yes. Feel free to implement that. However note that tests should be not evaluated with the other packages and build separately as they can consume a lot of memory. My recommendation is to retrieve the tests name from passthru.tests in nixpkgs_review/nix/evalAttrs.nix and than build those tests after the normal build and before starting the nix-shell.