Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: NixOS/nixpkgs
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 13892da3e763
Choose a base ref
...
head repository: NixOS/nixpkgs
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 6141939d6e0a
Choose a head ref
  • 2 commits
  • 7 files changed
  • 2 contributors

Commits on Oct 30, 2018

  1. dovecot, opensmtpd: add link to test in meta.tests 

    Rationale
---------

Currently, tests are hard to discover. For instance, someone updating
`dovecot` might not notice that the interaction of `dovecot` with
`opensmtpd` is handled in the `opensmtpd.nix` test.

And even for someone updating `opensmtpd`, it requires manual work to go
check in `nixos/tests` whether there is actually a test, especially
given not so many packages in `nixpkgs` have tests and this is thus most
of the time useless.

Finally, for the reviewer, it is much easier to check that the “Tested
via one or more NixOS test(s)” has been checked if the file modified
already includes the list of relevant tests.

Implementation
--------------

Currently, this commit only adds the metadata in the package. Each
element of the `meta.tests` attribute is a derivation that, when it
builds successfully, means the test has passed (ie. following the same
convention as NixOS tests).

Future Work
-----------

In the future, the tools could be made aware of this `meta.tests`
attribute, and for instance a `--with-tests` could be added to
`nix-build` so that it also builds all the tests. Or a `--without-tests`
to build without all the tests. @Profpatsch described in his NixCon talk
such systems.

Another thing that would help in the future would be the possibility to
reasonably easily have cross-derivation nix tests without the whole
NixOS VM stack. @7c6f434c already proposed such a system.

This RFC currently handles none of these concerns. Only the addition of
`meta.tests` as metadata to be used by maintainers to remember to run
relevant tests.
    @Ekleog
    Ekleog committed Oct 30, 2018
    1
    Copy the full SHA
    02e1f00 View commit details

Commits on Nov 6, 2018

  1. Merge pull request #44439 from Ekleog/meta-tests 

    [RFC] Use `meta.tests` to link from packages to the tests that test them
    @timokau
    timokau authored Nov 6, 2018
    Copy the full SHA
    6141939 View commit details
55 changes: 55 additions & 0 deletions doc/meta.xml
View file
Original file line number Diff line number Diff line change
@@ -250,6 +250,61 @@ meta.platforms = stdenv.lib.platforms.linux;
</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<varname>tests</varname>
</term>
<listitem>
<para>
An attribute set with as values tests. A test is a derivation, which
builds successfully when the test passes, and fails to build otherwise. A
derivation that is a test requires some <literal>meta</literal> elements
to be defined: <literal>needsVMSupport</literal> (automatically filled-in
for NixOS tests) and <literal>timeout</literal>.
</para>
<para>
The NixOS tests are available as <literal>nixosTests</literal> in
parameters of derivations. For instance, the OpenSMTPD derivation
includes lines similar to:
<programlisting>
{ /* ... */, nixosTests }:
{
# ...
meta.tests = {
basic-functionality-and-dovecot-integration = nixosTests.opensmtpd;
};
}
</programlisting>
</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<varname>timeout</varname>
</term>
<listitem>
<para>
A timeout (in seconds) for building the derivation. If the derivation
takes longer than this time to build, it can fail due to breaking the
timeout. However, all computers do not have the same computing power,
hence some builders may decide to apply a multiplicative factor to this
value. When filling this value in, try to keep it approximately
consistent with other values already present in
<literal>nixpkgs</literal>.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<varname>needsVMSupport</varname>
</term>
<listitem>
<para>
A boolan that states whether the derivation requires build-time support
for Virtual Machine to build successfully.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<varname>hydraPlatforms</varname>
4 changes: 3 additions & 1 deletion nixos/lib/testing.nix
View file
Original file line number Diff line number Diff line change
@@ -69,7 +69,9 @@ in rec {
mkdir -p $out/coverage-data
mv $i $out/coverage-data/$(dirname $(dirname $i))
done
''; # */
'';

meta.needsVMSupport = true;
};


2 changes: 2 additions & 0 deletions nixos/tests/opensmtpd.nix
View file
Original file line number Diff line number Diff line change
@@ -120,4 +120,6 @@ import ./make-test.nix {
$smtp2->waitUntilFails('smtpctl show queue | egrep .');
$client->succeed('check-mail-landed >&2');
'';

meta.timeout = 30;
}
4 changes: 4 additions & 0 deletions pkgs/servers/mail/dovecot/default.nix
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{ stdenv, lib, fetchurl, perl, pkgconfig, systemd, openssl
, bzip2, zlib, lz4, inotify-tools, pam, libcap
, clucene_core_2, icu, openldap, libsodium, libstemmer, cyrus_sasl
, nixosTests
# Auth modules
, withMySQL ? false, mysql
, withPgSQL ? false, postgresql
@@ -74,5 +75,8 @@ stdenv.mkDerivation rec {
description = "Open source IMAP and POP3 email server written with security primarily in mind";
maintainers = with stdenv.lib.maintainers; [ peti rickynils fpletz ];
platforms = stdenv.lib.platforms.unix;
tests = {
opensmtpd-interaction = nixosTests.opensmtpd;
};
};
}
5 changes: 4 additions & 1 deletion pkgs/servers/mail/opensmtpd/default.nix
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{ stdenv, lib, fetchurl, fetchpatch, autoconf, automake, libtool, bison
, libasr, libevent, zlib, libressl, db, pam
, libasr, libevent, zlib, libressl, db, pam, nixosTests
}:

stdenv.mkDerivation rec {
@@ -61,5 +61,8 @@ stdenv.mkDerivation rec {
license = licenses.isc;
platforms = platforms.linux;
maintainers = with maintainers; [ rickynils obadz ekleog ];
tests = {
basic-functionality-and-dovecot-interaction = nixosTests.opensmtpd;
};
};
}
12 changes: 10 additions & 2 deletions pkgs/stdenv/generic/check-meta.nix
View file
Original file line number Diff line number Diff line change
@@ -165,6 +165,16 @@ let
platforms = listOf (either str lib.systems.parsedPlatform.types.system);
hydraPlatforms = listOf str;
broken = bool;
# TODO: refactor once something like Profpatsch's types-simple will land
tests = attrsOf (mkOptionType {
name = "test";
check = x: isDerivation x &&
x ? meta.timeout &&
x ? meta.needsVMSupport;
merge = lib.options.mergeOneOption;
});
needsVMSupport = bool;
timeout = int;

# Weirder stuff that doesn't appear in the documentation?
knownVulnerabilities = listOf str;
@@ -184,8 +194,6 @@ let
isIbusEngine = bool;
isGutenprint = bool;
badPlatforms = platforms;
# Hydra build timeout
timeout = int;
};

checkMetaAttr = k: v:
20 changes: 20 additions & 0 deletions pkgs/top-level/all-packages.nix
View file
Original file line number Diff line number Diff line change
@@ -71,6 +71,26 @@ with pkgs;

common-updater-scripts = callPackage ../common-updater/scripts.nix { };

### Push NixOS tests inside the fixed point

nixosTests =
let
# TODO(Ericson2314,ekleog): Check this will work correctly with cross-
system = builtins.currentSystem;
rawTests = (import ../../nixos/release.nix {
nixpkgs = pkgs;
}).tests;
testNames = builtins.attrNames rawTests;
filteredList = builtins.filter
(test: rawTests.${test} ? ${system})
testNames;
finalList = map
(test: { name = test; value = rawTests.${test}.${system}; })
filteredList;
finalTests = builtins.listToAttrs finalList;
in
finalTests;

### BUILD SUPPORT

autoreconfHook = makeSetupHook