From 3e72e7c01483666a4465abfeefbcd4c668c79865 Mon Sep 17 00:00:00 2001 From: benaryorg Date: Thu, 25 Jan 2024 17:19:37 +0000 Subject: 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 | 156 ++++++++++++++++++++++++++++++---------- 1 file changed, 118 insertions(+), 38 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; - - runBuildTest = name: { drv, expected }: pkgs.runCommand name { + # 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}"; + } + ]; + + # 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; -- cgit 1.4.1 From 8b2d86b982c5d5c6f4c7402546d32b17eb6b07f8 Mon Sep 17 00:00:00 2001 From: benaryorg Date: Thu, 25 Jan 2024 22:09:23 +0000 Subject: pkgs.formats: toINIWithGlobalSection wrapper The new format is based on the existing wrapper and generates an INI file with an unnamed global section at the top as is used by *stunnel* for instance. Technically the INI format is a subset of this however testing, type checking, and API guarantees profit from two separate generators. Co-authored-by: tim-tx Signed-off-by: benaryorg --- .../manual/development/settings-options.section.md | 28 +++++ pkgs/pkgs-lib/formats.nix | 121 +++++++++++++-------- pkgs/pkgs-lib/tests/formats.nix | 95 ++++++++++++++++ 3 files changed, 199 insertions(+), 45 deletions(-) diff --git a/nixos/doc/manual/development/settings-options.section.md b/nixos/doc/manual/development/settings-options.section.md index 3a4800742b04..71ec9bbc8892 100644 --- a/nixos/doc/manual/development/settings-options.section.md +++ b/nixos/doc/manual/development/settings-options.section.md @@ -73,6 +73,34 @@ have a predefined type and string generator already declared under It returns a set with INI-specific attributes `type` and `generate` as specified [below](#pkgs-formats-result). + The type of the input is an *attrset* of sections; key-value pairs where + the key is the section name and the value is the corresponding content + which is also an *attrset* of key-value pairs for the actual key-value + mappings of the INI format. + The values of the INI atoms are subject to the above parameters (e.g. lists + may be transformed into multiple key-value pairs depending on + `listToValue`). + +`pkgs.formats.iniWithGlobalSection` { *`listsAsDuplicateKeys`* ? false, *`listToValue`* ? null, \.\.\. } + +: A function taking an attribute set with values + + `listsAsDuplicateKeys` + + : A boolean for controlling whether list values can be used to + represent duplicate INI keys + + `listToValue` + + : A function for turning a list of values into a single value. + + It returns a set with INI-specific attributes `type` and `generate` + as specified [below](#pkgs-formats-result). + The type of the input is an *attrset* of the structure + `{ sections = {}; globalSection = {}; }` where *sections* are several + sections as with *pkgs.formats.ini* and *globalSection* being just a single + attrset of key-value pairs for a single section, the global section which + preceedes the section definitions. `pkgs.formats.toml` { } diff --git a/pkgs/pkgs-lib/formats.nix b/pkgs/pkgs-lib/formats.nix index c78bd82e01ef..1b72270b43ca 100644 --- a/pkgs/pkgs-lib/formats.nix +++ b/pkgs/pkgs-lib/formats.nix @@ -95,29 +95,13 @@ rec { }; - ini = { - # Represents lists as duplicate keys - listsAsDuplicateKeys ? false, - # Alternative to listsAsDuplicateKeys, converts list to non-list - # listToValue :: [IniAtom] -> IniAtom - listToValue ? null, - ... - }@args: - assert !listsAsDuplicateKeys || listToValue == null; - { - - type = with lib.types; let - - singleIniAtom = nullOr (oneOf [ - bool - int - float - str - ]) // { + # the ini formats share a lot of code + inherit ( + let + singleIniAtom = with lib.types; nullOr (oneOf [ bool int float str ]) // { description = "INI atom (null, bool, int, float or string)"; }; - - iniAtom = + iniAtom = with lib.types; { listsAsDuplicateKeys, listToValue }: if listsAsDuplicateKeys then coercedTo singleIniAtom lib.singleton (listOf singleIniAtom) // { description = singleIniAtom.description + " or a list of them for duplicate keys"; @@ -128,21 +112,79 @@ rec { } else singleIniAtom; + iniSection = with lib.types; { listsAsDuplicateKeys, listToValue }@args: + attrsOf (iniAtom args) // { + description = "section of an INI file (attrs of " + (iniAtom args).description + ")"; + }; - in attrsOf (attrsOf iniAtom); + maybeToList = listToValue: if listToValue != null then lib.mapAttrs (key: val: if lib.isList val then listToValue val else val) else lib.id; + in { + ini = { + # Represents lists as duplicate keys + listsAsDuplicateKeys ? false, + # Alternative to listsAsDuplicateKeys, converts list to non-list + # listToValue :: [IniAtom] -> IniAtom + listToValue ? null, + ... + }@args: + assert listsAsDuplicateKeys -> listToValue == null; + { - generate = name: value: - let - transformedValue = - if listToValue != null - then - lib.mapAttrs (section: lib.mapAttrs (key: val: - if lib.isList val then listToValue val else val - )) value - else value; - in pkgs.writeText name (lib.generators.toINI (removeAttrs args ["listToValue"]) transformedValue); + type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }); - }; + generate = name: value: + lib.pipe value + [ + (lib.mapAttrs (_: maybeToList listToValue)) + (lib.generators.toINI (removeAttrs args ["listToValue"])) + (pkgs.writeText name) + ]; + }; + + iniWithGlobalSection = { + # Represents lists as duplicate keys + listsAsDuplicateKeys ? false, + # Alternative to listsAsDuplicateKeys, converts list to non-list + # listToValue :: [IniAtom] -> IniAtom + listToValue ? null, + ... + }@args: + assert listsAsDuplicateKeys -> listToValue == null; + { + type = lib.types.submodule { + options = { + sections = lib.mkOption rec { + type = lib.types.attrsOf (iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }); + default = {}; + description = type.description; + }; + globalSection = lib.mkOption rec { + type = iniSection { listsAsDuplicateKeys = listsAsDuplicateKeys; listToValue = listToValue; }; + default = {}; + description = "global " + type.description; + }; + }; + }; + generate = name: { sections ? {}, globalSection ? {}, ... }: + pkgs.writeText name (lib.generators.toINIWithGlobalSection (removeAttrs args ["listToValue"]) + { + globalSection = maybeToList listToValue globalSection; + sections = lib.mapAttrs (_: maybeToList listToValue) sections; + }); + }; + + gitIni = { listsAsDuplicateKeys ? false, ... }@args: { + type = let + atom = iniAtom { + listsAsDuplicateKeys = listsAsDuplicateKeys; + listToValue = null; + }; + in with lib.types; attrsOf (attrsOf (either atom (attrsOf atom))); + + generate = name: value: pkgs.writeText name (lib.generators.toGitINI value); + }; + + }) ini iniWithGlobalSection gitIni; # As defined by systemd.syntax(7) # @@ -166,7 +208,7 @@ rec { listToValue ? null, ... }@args: - assert !listsAsDuplicateKeys || listToValue == null; + assert listsAsDuplicateKeys -> listToValue == null; { type = with lib.types; let @@ -207,17 +249,6 @@ rec { }; - gitIni = { listsAsDuplicateKeys ? false, ... }@args: { - - type = with lib.types; let - - iniAtom = (ini args).type/*attrsOf*/.functor.wrapped/*attrsOf*/.functor.wrapped; - - in attrsOf (attrsOf (either iniAtom (attrsOf iniAtom))); - - generate = name: value: pkgs.writeText name (lib.generators.toGitINI value); - }; - toml = {}: json {} // { type = with lib.types; let valueType = oneOf [ diff --git a/pkgs/pkgs-lib/tests/formats.nix b/pkgs/pkgs-lib/tests/formats.nix index 72994803d584..3243f4058e68 100644 --- a/pkgs/pkgs-lib/tests/formats.nix +++ b/pkgs/pkgs-lib/tests/formats.nix @@ -222,6 +222,101 @@ in runBuildTests { ''; }; + iniWithGlobalNoSections = shouldPass { + format = formats.iniWithGlobalSection {}; + input = {}; + expected = ""; + }; + + iniWithGlobalOnlySections = shouldPass { + format = formats.iniWithGlobalSection {}; + input = { + sections = { + foo = { + bar = "baz"; + }; + }; + }; + expected = '' + [foo] + bar=baz + ''; + }; + + iniWithGlobalOnlyGlobal = shouldPass { + format = formats.iniWithGlobalSection {}; + input = { + globalSection = { + bar = "baz"; + }; + }; + expected = '' + bar=baz + + ''; + }; + + iniWithGlobalWrongSections = shouldFail { + format = formats.iniWithGlobalSection {}; + input = { + foo = {}; + }; + }; + + iniWithGlobalEverything = shouldPass { + format = formats.iniWithGlobalSection {}; + input = { + globalSection = { + bar = true; + }; + sections = { + foo = { + bool = true; + int = 10; + float = 3.141; + str = "string"; + }; + }; + }; + expected = '' + bar=true + + [foo] + bool=true + float=3.141000 + int=10 + str=string + ''; + }; + + iniWithGlobalListToValue = shouldPass { + format = formats.iniWithGlobalSection { listToValue = lib.concatMapStringsSep ", " (lib.generators.mkValueStringDefault {}); }; + input = { + globalSection = { + bar = [ null true "test" 1.2 10 ]; + baz = false; + qux = "qux"; + }; + sections = { + foo = { + bar = [ null true "test" 1.2 10 ]; + baz = false; + qux = "qux"; + }; + }; + }; + expected = '' + bar=null, true, test, 1.200000, 10 + baz=false + qux=qux + + [foo] + bar=null, true, test, 1.200000, 10 + baz=false + qux=qux + ''; + }; + keyValueAtoms = shouldPass { format = formats.keyValue {}; input = { -- cgit 1.4.1