From 3e72e7c01483666a4465abfeefbcd4c668c79865 Mon Sep 17 00:00:00 2001 From: benaryorg Date: Thu, 25 Jan 2024 17:19:37 +0000 Subject: [PATCH] pkgs.formats: negative type checking tests Tests using `shouldFail` (new) enable testing for whether the type system catches any unintended uses (missing parameters or unavailable data types in the format). It should not be necessary to test for all possible outliers for each format, however format-specific tests (for instance those using a rigid submodule structure) can ensure that common mistakes err out instead of being silently discarded, while also allowing test-driven development of sorts. Signed-off-by: benaryorg --- pkgs/pkgs-lib/tests/formats.nix | 154 ++++++++++++++++++++++++-------- 1 file changed, 117 insertions(+), 37 deletions(-) diff --git a/pkgs/pkgs-lib/tests/formats.nix b/pkgs/pkgs-lib/tests/formats.nix index b7e100dd73bc..72994803d584 100644 --- a/pkgs/pkgs-lib/tests/formats.nix +++ b/pkgs/pkgs-lib/tests/formats.nix @@ -1,24 +1,21 @@ { pkgs }: let inherit (pkgs) lib formats; -in -with lib; -let - evalFormat = format: args: def: - let - formatSet = format args; - config = formatSet.type.merge [] (imap1 (n: def: { - # We check the input values, so that - # - we don't write nonsensical tests that will impede progress - # - the test author has a slightly more realistic view of the - # final format during development. - value = lib.throwIfNot (formatSet.type.check def) (builtins.trace def "definition does not pass the type's check function") def; - file = "def${toString n}"; - }) [ def ]); - in formatSet.generate "test-format-file" config; + # merging allows us to add metadata to the input + # this makes error messages more readable during development + mergeInput = name: format: input: + format.type.merge [] [ + { + # explicitly throw here to trigger the code path that prints the error message for users + value = lib.throwIfNot (format.type.check input) (builtins.trace input "definition does not pass the type's check function") input; + # inject the name + file = "format-test-${name}"; + } + ]; - runBuildTest = name: { drv, expected }: pkgs.runCommand name { + # run a diff between expected and real output + runDiff = name: drv: expected: pkgs.runCommand name { passAsFile = ["expected"]; inherit expected drv; } '' @@ -31,12 +28,66 @@ let fi ''; - runBuildTests = tests: pkgs.linkFarm "nixpkgs-pkgs-lib-format-tests" (mapAttrsToList (name: value: { inherit name; path = runBuildTest name value; }) (filterAttrs (name: value: value != null) tests)); + # use this to check for proper serialization + # in practice you do not have to supply the name parameter as this one will be added by runBuildTests + shouldPass = { format, input, expected }: name: { + name = "pass-${name}"; + path = runDiff "test-format-${name}" (format.generate "test-format-${name}" (mergeInput name format input)) expected; + }; + + # use this function to assert that a type check must fail + # in practice you do not have to supply the name parameter as this one will be added by runBuildTests + # note that as per 352e7d330a26 and 352e7d330a26 the type checking of attrsets and lists are not strict + # this means that the code below needs to properly merge the module type definition and also evaluate the (lazy) return value + shouldFail = { format, input }: name: + let + # trigger a deep type check using the module system + typeCheck = lib.modules.mergeDefinitions + [ "tests" name ] + format.type + [ + { + file = "format-test-${name}"; + value = input; + } + ]; + # actually use the return value to trigger the evaluation + eval = builtins.tryEval (typeCheck.mergedValue == input); + # the check failing is what we want, so don't do anything here + typeFails = pkgs.runCommand "test-format-${name}" {} "touch $out"; + # bail with some verbose information in case the type check passes + typeSucceeds = pkgs.runCommand "test-format-${name}" { + passAsFile = [ "inputText" ]; + testName = name; + # this will fail if the input contains functions as values + # however that should get caught by the type check already + inputText = builtins.toJSON input; + } + '' + echo "Type check $testName passed when it shouldn't." + echo "The following data was used as input:" + echo + cat "$inputTextPath" + exit 1 + ''; + in { + name = "fail-${name}"; + path = if eval.success then typeSucceeds else typeFails; + }; + + # this function creates a linkFarm for all the tests below such that the results are easily visible in the filesystem after a build + # the parameters are an attrset of name: test pairs where the name is automatically passed to the test + # the test therefore is an invocation of ShouldPass or shouldFail with the attrset parameters but *not* the name (which this adds for convenience) + runBuildTests = (lib.flip lib.pipe) [ + (lib.mapAttrsToList (name: value: value name)) + (pkgs.linkFarm "nixpkgs-pkgs-lib-format-tests") + ]; in runBuildTests { - testJsonAtoms = { - drv = evalFormat formats.json {} { + jsonAtoms = shouldPass { + format = formats.json {}; + input = { null = null; false = false; true = true; @@ -67,8 +118,9 @@ in runBuildTests { ''; }; - testYamlAtoms = { - drv = evalFormat formats.yaml {} { + yamlAtoms = shouldPass { + format = formats.yaml {}; + input = { null = null; false = false; true = true; @@ -93,8 +145,9 @@ in runBuildTests { ''; }; - testIniAtoms = { - drv = evalFormat formats.ini {} { + iniAtoms = shouldPass { + format = formats.ini {}; + input = { foo = { bool = true; int = 10; @@ -111,8 +164,29 @@ in runBuildTests { ''; }; - testIniDuplicateKeys = { - drv = evalFormat formats.ini { listsAsDuplicateKeys = true; } { + iniInvalidAtom = shouldFail { + format = formats.ini {}; + input = { + foo = { + function = _: 1; + }; + }; + }; + + iniDuplicateKeysWithoutList = shouldFail { + format = formats.ini {}; + input = { + foo = { + bar = [ null true "test" 1.2 10 ]; + baz = false; + qux = "qux"; + }; + }; + }; + + iniDuplicateKeys = shouldPass { + format = formats.ini { listsAsDuplicateKeys = true; }; + input = { foo = { bar = [ null true "test" 1.2 10 ]; baz = false; @@ -131,8 +205,9 @@ in runBuildTests { ''; }; - testIniListToValue = { - drv = evalFormat formats.ini { listToValue = concatMapStringsSep ", " (generators.mkValueStringDefault {}); } { + iniListToValue = shouldPass { + format = formats.ini { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); }; + input = { foo = { bar = [ null true "test" 1.2 10 ]; baz = false; @@ -147,8 +222,9 @@ in runBuildTests { ''; }; - testKeyValueAtoms = { - drv = evalFormat formats.keyValue {} { + keyValueAtoms = shouldPass { + format = formats.keyValue {}; + input = { bool = true; int = 10; float = 3.141; @@ -162,8 +238,9 @@ in runBuildTests { ''; }; - testKeyValueDuplicateKeys = { - drv = evalFormat formats.keyValue { listsAsDuplicateKeys = true; } { + keyValueDuplicateKeys = shouldPass { + format = formats.keyValue { listsAsDuplicateKeys = true; }; + input = { bar = [ null true "test" 1.2 10 ]; baz = false; qux = "qux"; @@ -179,8 +256,9 @@ in runBuildTests { ''; }; - testKeyValueListToValue = { - drv = evalFormat formats.keyValue { listToValue = concatMapStringsSep ", " (generators.mkValueStringDefault {}); } { + keyValueListToValue = shouldPass { + format = formats.keyValue { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); }; + input = { bar = [ null true "test" 1.2 10 ]; baz = false; qux = "qux"; @@ -192,8 +270,9 @@ in runBuildTests { ''; }; - testTomlAtoms = { - drv = evalFormat formats.toml {} { + tomlAtoms = shouldPass { + format = formats.toml {}; + input = { false = false; true = true; int = 10; @@ -222,8 +301,9 @@ in runBuildTests { # 1. testing type coercions # 2. providing a more readable example test # Whereas java-properties/default.nix tests the low level escaping, etc. - testJavaProperties = { - drv = evalFormat formats.javaProperties {} { + javaProperties = shouldPass { + format = formats.javaProperties {}; + input = { floaty = 3.1415; tautologies = true; contradictions = false;