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

Reworking test infrastructure #18142

Closed
wants to merge 59 commits into from
Closed

Conversation

ericsagnes
Copy link
Contributor

@ericsagnes ericsagnes commented Aug 31, 2016

Updated the PR description and title on 2016/09/03 according to current state.

Motivation for this change

This PR has the following goals:

  • make Nix tests easier to work with (callTest and nix-run-test)
  • Allow module maintainers to have better control of module tests by tying the tests to the modules, and putting the test file in a folder of their liking.

Main improvements

  • Simpler test infrastructure, tests are just a function
  • Tests are not tied to make-test.nix anymore, and can be run from anywhere
  • Relationship between tests and modules, maintainership is improved as tests and modules can be in the same/close folders
  • nix-run-test command
  • moved callTest to lib, so it can easily be called from custom projects

Changes

  • Adds a new meta.tests attribute to modules
  • Add a new callTest function that has a wider use range than the previous one
  • Removes previous callTest, callSubTests as their functionality is superseded by the new callTest
  • Move callTest to lib.testing so it can easily be called from other places than release.nix
  • Move all module related tests to the module meta.tests attributes
  • Modules related tests are in tests.module subset of release.nix
  • Simplify test files by removing the need of calling makeTest in test files, test files should be a test set, a set of test sets, or a test derivation
  • Some test related files that were in nixos/lib, namely test-driver/* and testing.nix, are relocated in lib/testing
  • A new nix-run-test command had been added to run test in an easy way
  • Manual has been updated accordingly to the changes
  • Removes unneeded nixos/tests/make-test.nix

Possibly controversial changes

  • Removes previous callTest, callSubTests as their functionality is superseded by the new callTest
  • Some test related files that were in nixos/lib, namely test-driver/* and testing.nix, are relocated in lib/testing
  • Removes unneeded nixos/tests/make-test.nix

How it works

modules

meta.tests is a new attribute of modules that takes a set of tests.
Key is the set name, value is the test file, eg:

  meta.tests = {
    containers-ipv4       = ./tests/containers-ipv4.nix;
    containers-ipv6       = ./tests/containers-ipv6.nix;
    containers-bridge     = ./tests/containers-bridge.nix;
    containers-imperative = ./tests/containers-imperative.nix;
    containers-extra_veth = ./tests/containers-extra_veth.nix;
  };

release.nix

In release.nix, module tests are gathered with the moduleTests function, and added in the tests.module set.
Problematic tests can be blacklisted by adding their name to the blacklistedModuleTests list.

blacklistedModuleTests = [ "postgis" ];

Module tests can be run like any other tests, eg:

$ nix-build -A tests.module.panamax ./nixos/release.nix

The moduleTests accept a set argument to set arguments on dedicated tests if needed.

  tests.module = moduleTests {
    cadvisor = { systems = [ "x86_64-linux" ]; };
    kubernetes = { systems = [ "x86_64-linux" ]; };
    docker = { systems = [ "x86_64-linux" ]; };
    dockerRegistry = { systems = [ "x86_64-linux" ]; };
    etcd = { systems = [ "x86_64-linux" ]; };
    fleet = { systems = [ "x86_64-linux" ]; };
    panamax = { systems = [ "x86_64-linux" ]; };
  };

callTest

callTest has been reworked to accept test sets, set of tests sets or test derivations.
Also, there is no need to use makeTest in the test files anymore.

A new lib subset, lib.testing was added. lib.testing expose only the callTest function.

This change make callTest a part of lib, so it can be called from places other than release.nix , examples:

a. Call a test:

lib.testing.callTest ./nixos/tests/simple.nix {};

b. Call a test for only x86_64-linux:

lib.testing.callTest ./nixos/tests/simple.nix { systems = [ "x86_64-linux" ]; };

c. Call a test with custom parameters:

lib.testing.callTest ./nixos/tests/nfs.nix { version = 3; };

nix-run-test

nix-run-test, a new utility to run nix tests has been added. It is just a wrapper around nix-build and callTest to run tests in an easy manner.

a. Call a test:

$ nix-run-test ./nixos/tests/simple.nix

b. Prepare an interactive test:

$ nix-run-test --interactive ./nixos/tests/simple.nix

TODO

  • Test in a Hydra
Things done
  • Tested using sandboxing
    (nix.useChroot on NixOS,
    or option build-use-chroot in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@ericsagnes, thanks for your PR! By analyzing the annotation information on this pull request, we identified @edolstra, @aszlig and @kampfschlaefer to be potential reviewers

@lsix lsix added the 2.status: work-in-progress This PR isn't done label Aug 31, 2016
@ericsagnes ericsagnes force-pushed the feat/meta-tests branch 3 times, most recently from 064ea6f to ef748d4 Compare September 1, 2016 07:39
@aszlig
Copy link
Member

aszlig commented Sep 1, 2016

Cool, this was something on my TODO list for quite a while.

I already have an implementation of this in vuizvui for machine-dependant tests:

https://github.com/openlab-aux/vuizvui/blob/master/modules/core/tests.nix

Having tests in within the meta attributes however won't help for machine-related tests (I'm building channels based on which settings are enabled in the machine configuration) so maybe it's a better idea to add them as a module option.

Which benefit does it have if the tests are defined in the meta attributes?

@ericsagnes ericsagnes force-pushed the feat/meta-tests branch 2 times, most recently from 5226884 to 181bf34 Compare September 1, 2016 12:11
@ericsagnes
Copy link
Contributor Author

@aszlig

Thanks for looking at this!
I added a few commits to clean up and give a better idea of what it should look like when finished.

Which benefit does it have if the tests are defined in the meta attributes?

The benefits I can see of having the tests in meta are:

  • obvious one, tests are tied to modules
  • it is easy to fetch all the modules tests in release.nix (see moduleTests function in release.nix)
  • there is no need to declare a new option in every module
  • not really because of meta, but as a side effect tests and modules are closer on the filesystem

Having tests in within the meta attributes however won't help for machine-related tests

To be honest I haven't thought at all about machine-related tests for this.
It is true meta just tie the tests with a module file and knows nothing about the options defined in the module, so it cannot be a great help for machine-related tests.

One approach I can think of (no idea if it is realistic or not), would be to define a function relevant :: Config -> Bool in the test, so by passing config to it, we can decide if the test is relevant for a particular config.
Then it would just be a matter of collecting all the "relevant" tests for a particular config.

Test pseudo code:

{ pkgs, ... } :

{
  name = "foo";

  relevant = config:
    config.services.httpd.enable && elem "proxy_balancer" config.services.httpd.extraModules;

  nodes = {
    # ...
  };

  testScript = ''
    ...
  '';
}

release.nix pseudo code:

  dummyAttrs = { ... };
  moduleTests = (import lib/eval-config.nix { modules = []; }).config.meta.tests;
  relevantTests = config: filterAttrs (k: v: (v dummyAttrs).relevant config) moduleTests;
  testsForMachineA = relevantTests machineAConfig;

@ericsagnes ericsagnes force-pushed the feat/meta-tests branch 2 times, most recently from 006b21f to ade56f4 Compare September 2, 2016 11:37
@ericsagnes
Copy link
Contributor Author

Update: Added a few commits and updated the PR description.

@ericsagnes ericsagnes changed the title [WIP] tie tests to modules [WIP] Reworking test infrastructure Sep 3, 2016
@ericsagnes ericsagnes force-pushed the feat/meta-tests branch 2 times, most recently from 3315471 to 1fa1e12 Compare September 3, 2016 08:15
@ericsagnes
Copy link
Contributor Author

Update

The PR is almost complete functionality wise, from now I am just planning to test and fix any problems that could rise without changing much of its logic.

It changed a lot since its creation so please take the time to read the updated description.

As this contains some changes that could be considered controversial (see description for a list) and touch a wide range of files, I am very interested in getting some feedback. Especially from @edolstra and @aszlig as they are the main contributors to the current test infrastructure and @domenkozar and @joachifm as they were interested by the concept.

Thank you!

@ericsagnes ericsagnes force-pushed the feat/meta-tests branch 5 times, most recently from 4ab6f6e to 6f0ee57 Compare September 4, 2016 22:56
@ericsagnes ericsagnes changed the title [WIP] Reworking test infrastructure Reworking test infrastructure Sep 5, 2016
@aszlig
Copy link
Member

aszlig commented Sep 5, 2016

One approach I can think of (no idea if it is realistic or not), would be to define a function relevant :: Config -> Bool in the test, so by passing config to it, we can decide if the test is relevant for a particular config.

The downside of this is that once modules are refactored it's very likely that people forget to adapt the test definitions.

In vuizvui the tests are defined by a module option so defining a test in a module's config is tying it to the module, like this (of course this currently uses lists of lists instead of a list of test attributes, because right in <nixpkgs> it's not possible to reference tests directly within the module system).

Of course this has the downside that it's no longer that easy to enumerate all tests, except by using a dummy config that enables all modules with tests.

Another thing I'm interested is having NixOS tests for normal packages, like for example for Chromium. Possibly also tied to whether such a package is defined in systemPackages. The implementation for this could be done by mapping through systemPackages for packages that define a meta.test attribute.

  • there is no need to declare a new option in every module

Why would one need to declare an option for a test?

@aszlig
Copy link
Member

aszlig commented Sep 5, 2016

Also, I'd rename nix-run-tests to nixos-run-tests, because those tests are NixOS-specific.

@ericsagnes
Copy link
Contributor Author

The downside of this is that once modules are refactored it's very likely that people forget to adapt the test definitions.

If that breaks tests for some, they will fix the definitions.
But yes, it is not a perfect solution.

Another thing I'm interested is having NixOS tests for normal packages, like for example for Chromium. Possibly also tied to whether such a package is defined in systemPackages. The implementation for this could be done by mapping through systemPackages for packages that define a meta.test attribute.

Actually, now that tests files can be moved around easily, I was thinking to move packages related tests in packages meta and closer to packages files in a second phase.

Why would one need to declare an option for a test?

Sorry, I was a little fast on reading your config. No, there is no need for that.

Anyway, the original goal of this PR was to tie modules and tests together so modules and tests maintainership can be improved.
I think machine-dependent testing would be best addressed in a future PR.
Maybe we can create an issue to discuss about the right way to implement it?

But let me know if you think that some changes this PR brings would make a future implementation machine-dependent testing difficult, I would be very happy to fix these.

Also, I'd rename nix-run-tests to nixos-run-tests, because those tests are NixOS-specific.

I was hesitating on this point, nix-run-test is just a wrapper to nix-build so I hoped it could work for packages tests on platforms were nix only is installed, such as other linuxes, maybe that was too much of optimistic thinking?

@0xABAB
Copy link
Contributor

0xABAB commented Jul 9, 2017

@ericsagnes Does it just smell funny or is it dead?

@Ekleog
Copy link
Member

Ekleog commented Dec 20, 2018

@ericsagnes Are you still willing to push this forward? If not, given how badly this appears to have bitrotten, maybe it would make sense to open an issue with the changes you tried to do (from a quick read, replace in-file make-test.nix with out-of-file callTest and addition of meta.tests to modules), and close this PR?

@Lassulus Lassulus added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 24, 2019
@c0bw3b c0bw3b added the 6.topic: testing Tooling for automated testing of packages and modules label Apr 28, 2019
@mmahut
Copy link
Member

mmahut commented Aug 13, 2019

Are there any updates on this pull request, please?

@davidak
Copy link
Member

davidak commented Nov 9, 2019

Instead of meta.tests we have passthru.tests now.

#44439
NixOS/nix#2532 (comment)

The testing situation has also changed lastly. Module tests have been rewritten in Python. Package tests are possible now.

#73076

@ericsagnes can you create individual issues or PRs for the changes that are still missing?

@infinisil infinisil closed this Mar 12, 2020
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: work-in-progress This PR isn't done 6.topic: testing Tooling for automated testing of packages and modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.