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

nixos/testing-python: Avoid infinite recursion in node definitions #144110

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

talyz
Copy link
Contributor

@talyz talyz commented Nov 1, 2021

Motivation for this change

This is a follow-up to #140792 and #144014.

#144014 solves the infinite recursion issue when useNixStoreImage is false. This follow-up solves it in the general case by passing a version of the nodes without additionalPaths to testScript.

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 via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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
  • Fits CONTRIBUTING.md.

Pass a version of the nodes without `additionalPaths` to `testScript`
in order to avoid infinite recursion when the test script refers to
any of the nodes' toplevel closures.
@talyz talyz requested a review from roberth November 1, 2021 12:55
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Nov 1, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 1, 2021
mkTestScript = qemu_pkg:
# Call the test script with the computed nodes.
if lib.isFunction testScript
then testScript { nodes = nodes qemu_pkg false; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
then testScript { nodes = nodes qemu_pkg false; }
# To avoid an infinite recursion (nodes -> testScript -> nodes), we force disable
# the effect of config.virtualisation.useNixStoreImage, so `testScript -> nodes`
# becomes `testScript -> nodes2` where `nodes2` does not reference the `testScript`.
then testScript { nodes = nodes qemu_pkg false; }

Comment on lines +245 to +249
inherit enableOCR skipLint passthru;
testScript = mkTestScript pkgs.qemu_test;
testName = name;
qemu_pkg = pkgs.qemu_test;
nodes = nodes pkgs.qemu_test;
nodes = nodes pkgs.qemu_test true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best if the testScript could reuse this nodes value instead; only invoking the configs an extra time, when lib.any (n: n.config.virtualisation.useNixStoreImage) nodes.

@roberth
Copy link
Member

roberth commented Jun 6, 2022

I've translated this PR's logic into part of a module in #176557, testScript.nix. I think we'll resolve it that way.

roberth added a commit to hercules-ci/nixpkgs that referenced this pull request Jun 15, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of NixOS#144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
roberth added a commit to hercules-ci/nixpkgs that referenced this pull request Jun 21, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of NixOS#144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
roberth added a commit to hercules-ci/nixpkgs that referenced this pull request Jun 27, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of NixOS#144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
roberth added a commit to hercules-ci/nixpkgs that referenced this pull request Jun 28, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of NixOS#144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
roberth added a commit that referenced this pull request Sep 17, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of #144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
roberth added a commit to hercules-ci/nixpkgs that referenced this pull request Sep 21, 2022
This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.

By refactoring the glue, we accomplish the following:

 - NixOS tests can now use `imports` and other module system features.
    - Network-wide test setup can now be reusable; example:
       - A setup with all VMs configured to use a DNS server
       - Split long, slow tests into multiple tests that import a
         common module that has most of the setup.
    - Type checking for the test arguments
    - (TBD) "generated" options reference docs
 - Aspects that had to be wired through all the glue are now in their
   own files.
    - Chief example: interactive.nix.
    - Also: network.nix

In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.

I've made two exceptions to this rule

 - Introduction of `extraDriverArgs` instead of hardcoded
   interactivity logic.

 - Incorporation of NixOS#144110
   in testScript.nix.

I might revert the latter and split it into a new commit.
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 8, 2023
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 28, 2024 11:00
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 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: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants