From 01b2037bc077d3a9567a8e6911ac53985cb280ad Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 24 Sep 2025 11:55:21 -0400 Subject: [PATCH] Minimize the use of C Macros for characterization tests Fewer macros is better! Introduce a new `JsonChacterizationTest` mixin class to help with this. Also, avoid some needless copies with `GetParam`. Part of my effort shoring up the JSON formats with #13570. --- src/libexpr-tests/primops.cc | 6 +- src/libfetchers-tests/public-key.cc | 55 ++-- ...ivationDeps.drv => dyn-dep-derivation.drv} | 0 ...ationDeps.json => dyn-dep-derivation.json} | 0 .../{simple.drv => simple-derivation.drv} | 0 .../{simple.json => simple-derivation.json} | 0 src/libstore-tests/derivation.cc | 299 ++++++++++-------- src/libstore-tests/outputs-spec.cc | 78 +++-- src/libstore-tests/path.cc | 49 +-- src/libstore-tests/realisation.cc | 20 +- src/libstore/derivations.cc | 32 +- src/libstore/include/nix/store/derivations.hh | 10 +- .../nix/util/tests/json-characterization.hh | 54 ++++ .../include/nix/util/tests/meson.build | 1 + src/libutil-tests/sort.cc | 12 +- 15 files changed, 364 insertions(+), 252 deletions(-) rename src/libstore-tests/data/derivation/{dynDerivationDeps.drv => dyn-dep-derivation.drv} (100%) rename src/libstore-tests/data/derivation/{dynDerivationDeps.json => dyn-dep-derivation.json} (100%) rename src/libstore-tests/data/derivation/{simple.drv => simple-derivation.drv} (100%) rename src/libstore-tests/data/derivation/{simple.json => simple-derivation.json} (100%) create mode 100644 src/libutil-test-support/include/nix/util/tests/json-characterization.hh diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index aa4ef5e21..74d676844 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -642,7 +642,7 @@ class ToStringPrimOpTest : public PrimOpTest, TEST_P(ToStringPrimOpTest, toString) { - const auto [input, output] = GetParam(); + const auto & [input, output] = GetParam(); auto v = eval(input); ASSERT_THAT(v, IsStringEq(output)); } @@ -798,7 +798,7 @@ class CompareVersionsPrimOpTest : public PrimOpTest, TEST_P(CompareVersionsPrimOpTest, compareVersions) { - auto [expression, expectation] = GetParam(); + const auto & [expression, expectation] = GetParam(); auto v = eval(expression); ASSERT_THAT(v, IsIntEq(expectation)); } @@ -834,7 +834,7 @@ class ParseDrvNamePrimOpTest TEST_P(ParseDrvNamePrimOpTest, parseDrvName) { - auto [input, expectedName, expectedVersion] = GetParam(); + const auto & [input, expectedName, expectedVersion] = GetParam(); const auto expr = fmt("builtins.parseDrvName \"%1%\"", input); auto v = eval(expr); ASSERT_THAT(v, IsAttrsOfSize(2)); diff --git a/src/libfetchers-tests/public-key.cc b/src/libfetchers-tests/public-key.cc index 97a232447..2991223f6 100644 --- a/src/libfetchers-tests/public-key.cc +++ b/src/libfetchers-tests/public-key.cc @@ -1,14 +1,14 @@ #include #include "nix/fetchers/fetchers.hh" #include "nix/util/json-utils.hh" -#include -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" namespace nix { using nlohmann::json; -class PublicKeyTest : public CharacterizationTest +class PublicKeyTest : public JsonCharacterizationTest, + public ::testing::WithParamInterface> { std::filesystem::path unitTestData = getUnitTestData() / "public-key"; @@ -19,30 +19,35 @@ public: } }; -#define TEST_JSON(FIXTURE, NAME, VAL) \ - TEST_F(FIXTURE, PublicKey_##NAME##_from_json) \ - { \ - readTest(#NAME ".json", [&](const auto & encoded_) { \ - fetchers::PublicKey expected{VAL}; \ - fetchers::PublicKey got = nlohmann::json::parse(encoded_); \ - ASSERT_EQ(got, expected); \ - }); \ - } \ - \ - TEST_F(FIXTURE, PublicKey_##NAME##_to_json) \ - { \ - writeTest( \ - #NAME ".json", \ - [&]() -> json { return nlohmann::json(fetchers::PublicKey{VAL}); }, \ - [](const auto & file) { return json::parse(readFile(file)); }, \ - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ - } +TEST_P(PublicKeyTest, from_json) +{ + const auto & [name, expected] = GetParam(); + readJsonTest(name, expected); +} -TEST_JSON(PublicKeyTest, simple, (fetchers::PublicKey{.type = "ssh-rsa", .key = "ABCDE"})) +TEST_P(PublicKeyTest, to_json) +{ + const auto & [name, value] = GetParam(); + writeJsonTest(name, value); +} -TEST_JSON(PublicKeyTest, defaultType, fetchers::PublicKey{.key = "ABCDE"}) - -#undef TEST_JSON +INSTANTIATE_TEST_SUITE_P( + PublicKeyJSON, + PublicKeyTest, + ::testing::Values( + std::pair{ + "simple", + fetchers::PublicKey{ + .type = "ssh-rsa", + .key = "ABCDE", + }, + }, + std::pair{ + "defaultType", + fetchers::PublicKey{ + .key = "ABCDE", + }, + })); TEST_F(PublicKeyTest, PublicKey_noRoundTrip_from_json) { diff --git a/src/libstore-tests/data/derivation/dynDerivationDeps.drv b/src/libstore-tests/data/derivation/dyn-dep-derivation.drv similarity index 100% rename from src/libstore-tests/data/derivation/dynDerivationDeps.drv rename to src/libstore-tests/data/derivation/dyn-dep-derivation.drv diff --git a/src/libstore-tests/data/derivation/dynDerivationDeps.json b/src/libstore-tests/data/derivation/dyn-dep-derivation.json similarity index 100% rename from src/libstore-tests/data/derivation/dynDerivationDeps.json rename to src/libstore-tests/data/derivation/dyn-dep-derivation.json diff --git a/src/libstore-tests/data/derivation/simple.drv b/src/libstore-tests/data/derivation/simple-derivation.drv similarity index 100% rename from src/libstore-tests/data/derivation/simple.drv rename to src/libstore-tests/data/derivation/simple-derivation.drv diff --git a/src/libstore-tests/data/derivation/simple.json b/src/libstore-tests/data/derivation/simple-derivation.json similarity index 100% rename from src/libstore-tests/data/derivation/simple.json rename to src/libstore-tests/data/derivation/simple-derivation.json diff --git a/src/libstore-tests/derivation.cc b/src/libstore-tests/derivation.cc index 35992c5ec..65a5d011d 100644 --- a/src/libstore-tests/derivation.cc +++ b/src/libstore-tests/derivation.cc @@ -5,13 +5,13 @@ #include "nix/store/derivations.hh" #include "nix/store/tests/libstore.hh" -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" namespace nix { using nlohmann::json; -class DerivationTest : public CharacterizationTest, public LibStoreTest +class DerivationTest : public virtual CharacterizationTest, public LibStoreTest { std::filesystem::path unitTestData = getUnitTestData() / "derivation"; @@ -66,146 +66,183 @@ TEST_F(DynDerivationTest, BadATerm_oldVersionDynDeps) FormatError); } -#define TEST_JSON(FIXTURE, NAME, VAL, DRV_NAME, OUTPUT_NAME) \ - TEST_F(FIXTURE, DerivationOutput_##NAME##_from_json) \ - { \ - readTest("output-" #NAME ".json", [&](const auto & encoded_) { \ - auto encoded = json::parse(encoded_); \ - DerivationOutput got = DerivationOutput::fromJSON(DRV_NAME, OUTPUT_NAME, encoded, mockXpSettings); \ - DerivationOutput expected{VAL}; \ - ASSERT_EQ(got, expected); \ - }); \ - } \ - \ - TEST_F(FIXTURE, DerivationOutput_##NAME##_to_json) \ - { \ - writeTest( \ - "output-" #NAME ".json", \ - [&]() -> json { return DerivationOutput{(VAL)}.toJSON((DRV_NAME), (OUTPUT_NAME)); }, \ - [](const auto & file) { return json::parse(readFile(file)); }, \ - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ +#define MAKE_OUTPUT_JSON_TEST_P(FIXTURE) \ + TEST_P(FIXTURE, from_json) \ + { \ + const auto & [name, expected] = GetParam(); \ + /* Don't use readJsonTest because we want to check experimental \ + features. */ \ + readTest(Path{"output-"} + name + ".json", [&](const auto & encoded_) { \ + json j = json::parse(encoded_); \ + DerivationOutput got = DerivationOutput::fromJSON(j, mockXpSettings); \ + ASSERT_EQ(got, expected); \ + }); \ + } \ + \ + TEST_P(FIXTURE, to_json) \ + { \ + const auto & [name, value] = GetParam(); \ + writeJsonTest("output-" + name, value); \ } -TEST_JSON( - DerivationTest, - inputAddressed, - (DerivationOutput::InputAddressed{ - .path = store->parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-drv-name-output-name"), - }), - "drv-name", - "output-name") +struct DerivationOutputJsonTest : DerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; -TEST_JSON( - DerivationTest, - caFixedFlat, - (DerivationOutput::CAFixed{ - .ca = - { - .method = ContentAddressMethod::Raw::Flat, - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - }), - "drv-name", - "output-name") +MAKE_OUTPUT_JSON_TEST_P(DerivationOutputJsonTest) -TEST_JSON( - DerivationTest, - caFixedNAR, - (DerivationOutput::CAFixed{ - .ca = - { +INSTANTIATE_TEST_SUITE_P( + DerivationOutputJSON, + DerivationOutputJsonTest, + ::testing::Values( + std::pair{ + "inputAddressed", + DerivationOutput{DerivationOutput::InputAddressed{ + .path = StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-drv-name-output-name"}, + }}, + }, + std::pair{ + "caFixedFlat", + DerivationOutput{DerivationOutput::CAFixed{ + .ca = + { + .method = ContentAddressMethod::Raw::Flat, + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), + }, + }}, + }, + std::pair{ + "caFixedNAR", + DerivationOutput{DerivationOutput::CAFixed{ + .ca = + { + .method = ContentAddressMethod::Raw::NixArchive, + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), + }, + }}, + }, + std::pair{ + "deferred", + DerivationOutput{DerivationOutput::Deferred{}}, + })); + +struct DynDerivationOutputJsonTest : DynDerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; + +MAKE_OUTPUT_JSON_TEST_P(DynDerivationOutputJsonTest); + +INSTANTIATE_TEST_SUITE_P( + DynDerivationOutputJSON, + DynDerivationOutputJsonTest, + ::testing::Values( + std::pair{ + "caFixedText", + DerivationOutput{DerivationOutput::CAFixed{ + .ca = + { + .method = ContentAddressMethod::Raw::Text, + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), + }, + }}, + })); + +struct CaDerivationOutputJsonTest : CaDerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; + +MAKE_OUTPUT_JSON_TEST_P(CaDerivationOutputJsonTest); + +INSTANTIATE_TEST_SUITE_P( + CaDerivationOutputJSON, + CaDerivationOutputJsonTest, + ::testing::Values( + std::pair{ + "caFloating", + DerivationOutput{DerivationOutput::CAFloating{ .method = ContentAddressMethod::Raw::NixArchive, - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - }), - "drv-name", - "output-name") + .hashAlgo = HashAlgorithm::SHA256, + }}, + })); -TEST_JSON( - DynDerivationTest, - caFixedText, - (DerivationOutput::CAFixed{ - .ca = - { - .method = ContentAddressMethod::Raw::Text, - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - }), - "drv-name", - "output-name") +struct ImpureDerivationOutputJsonTest : ImpureDerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; -TEST_JSON( - CaDerivationTest, - caFloating, - (DerivationOutput::CAFloating{ - .method = ContentAddressMethod::Raw::NixArchive, - .hashAlgo = HashAlgorithm::SHA256, - }), - "drv-name", - "output-name") +MAKE_OUTPUT_JSON_TEST_P(ImpureDerivationOutputJsonTest); -TEST_JSON(DerivationTest, deferred, DerivationOutput::Deferred{}, "drv-name", "output-name") +INSTANTIATE_TEST_SUITE_P( + ImpureDerivationOutputJSON, + ImpureDerivationOutputJsonTest, + ::testing::Values( + std::pair{ + "impure", + DerivationOutput{DerivationOutput::Impure{ + .method = ContentAddressMethod::Raw::NixArchive, + .hashAlgo = HashAlgorithm::SHA256, + }}, + })); -TEST_JSON( - ImpureDerivationTest, - impure, - (DerivationOutput::Impure{ - .method = ContentAddressMethod::Raw::NixArchive, - .hashAlgo = HashAlgorithm::SHA256, - }), - "drv-name", - "output-name") +#undef MAKE_OUTPUT_JSON_TEST_P -#undef TEST_JSON - -#define TEST_JSON(FIXTURE, NAME, VAL) \ - TEST_F(FIXTURE, Derivation_##NAME##_from_json) \ - { \ - readTest(#NAME ".json", [&](const auto & encoded_) { \ - auto encoded = json::parse(encoded_); \ - Derivation expected{VAL}; \ - Derivation got = Derivation::fromJSON(encoded, mockXpSettings); \ - ASSERT_EQ(got, expected); \ - }); \ - } \ - \ - TEST_F(FIXTURE, Derivation_##NAME##_to_json) \ - { \ - writeTest( \ - #NAME ".json", \ - [&]() -> json { return Derivation{VAL}.toJSON(); }, \ - [](const auto & file) { return json::parse(readFile(file)); }, \ - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ +#define MAKE_TEST_P(FIXTURE) \ + TEST_P(FIXTURE, from_json) \ + { \ + const auto & drv = GetParam(); \ + /* Don't use readJsonTest because we want to check experimental \ + features. */ \ + readTest(drv.name + ".json", [&](const auto & encoded_) { \ + auto encoded = json::parse(encoded_); \ + Derivation got = Derivation::fromJSON(encoded, mockXpSettings); \ + ASSERT_EQ(got, drv); \ + }); \ + } \ + \ + TEST_P(FIXTURE, to_json) \ + { \ + const auto & drv = GetParam(); \ + writeJsonTest(drv.name, drv); \ + } \ + \ + TEST_P(FIXTURE, from_aterm) \ + { \ + const auto & drv = GetParam(); \ + readTest(drv.name + ".drv", [&](auto encoded) { \ + auto got = parseDerivation(*store, std::move(encoded), drv.name, mockXpSettings); \ + ASSERT_EQ(got.toJSON(), drv.toJSON()); \ + ASSERT_EQ(got, drv); \ + }); \ + } \ + \ + TEST_P(FIXTURE, to_aterm) \ + { \ + const auto & drv = GetParam(); \ + writeTest(drv.name + ".drv", [&]() -> std::string { return drv.unparse(*store, false); }); \ } -#define TEST_ATERM(FIXTURE, NAME, VAL, DRV_NAME) \ - TEST_F(FIXTURE, Derivation_##NAME##_from_aterm) \ - { \ - readTest(#NAME ".drv", [&](auto encoded) { \ - Derivation expected{VAL}; \ - auto got = parseDerivation(*store, std::move(encoded), DRV_NAME, mockXpSettings); \ - ASSERT_EQ(got.toJSON(), expected.toJSON()); \ - ASSERT_EQ(got, expected); \ - }); \ - } \ - \ - TEST_F(FIXTURE, Derivation_##NAME##_to_aterm) \ - { \ - writeTest(#NAME ".drv", [&]() -> std::string { return (VAL).unparse(*store, false); }); \ - } +struct DerivationJsonAtermTest : DerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface +{}; -Derivation makeSimpleDrv(const Store & store) +MAKE_TEST_P(DerivationJsonAtermTest); + +Derivation makeSimpleDrv() { Derivation drv; drv.name = "simple-derivation"; drv.inputSrcs = { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"), + StorePath("c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"), }; drv.inputDrvs = { .map = { { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv"), + StorePath("c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv"), { .value = { @@ -231,22 +268,27 @@ Derivation makeSimpleDrv(const Store & store) return drv; } -TEST_JSON(DerivationTest, simple, makeSimpleDrv(*store)) +INSTANTIATE_TEST_SUITE_P(DerivationJSONATerm, DerivationJsonAtermTest, ::testing::Values(makeSimpleDrv())); -TEST_ATERM(DerivationTest, simple, makeSimpleDrv(*store), "simple-derivation") +struct DynDerivationJsonAtermTest : DynDerivationTest, + JsonCharacterizationTest, + ::testing::WithParamInterface +{}; -Derivation makeDynDepDerivation(const Store & store) +MAKE_TEST_P(DynDerivationJsonAtermTest); + +Derivation makeDynDepDerivation() { Derivation drv; drv.name = "dyn-dep-derivation"; drv.inputSrcs = { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"), + StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep1"}, }; drv.inputDrvs = { .map = { { - store.parseStorePath("/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv"), + StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-dep2.drv"}, DerivedPathMap::ChildNode{ .value = { @@ -293,11 +335,8 @@ Derivation makeDynDepDerivation(const Store & store) return drv; } -TEST_JSON(DynDerivationTest, dynDerivationDeps, makeDynDepDerivation(*store)) +INSTANTIATE_TEST_SUITE_P(DynDerivationJSONATerm, DynDerivationJsonAtermTest, ::testing::Values(makeDynDepDerivation())); -TEST_ATERM(DynDerivationTest, dynDerivationDeps, makeDynDepDerivation(*store), "dyn-dep-derivation") - -#undef TEST_JSON -#undef TEST_ATERM +#undef MAKE_TEST_P } // namespace nix diff --git a/src/libstore-tests/outputs-spec.cc b/src/libstore-tests/outputs-spec.cc index 7b3fc8f45..1fac222fc 100644 --- a/src/libstore-tests/outputs-spec.cc +++ b/src/libstore-tests/outputs-spec.cc @@ -3,12 +3,11 @@ #include #include "nix/store/tests/outputs-spec.hh" - -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" namespace nix { -class OutputsSpecTest : public CharacterizationTest +class OutputsSpecTest : public virtual CharacterizationTest { std::filesystem::path unitTestData = getUnitTestData() / "outputs-spec"; @@ -20,7 +19,7 @@ public: } }; -class ExtendedOutputsSpecTest : public CharacterizationTest +class ExtendedOutputsSpecTest : public virtual CharacterizationTest { std::filesystem::path unitTestData = getUnitTestData() / "outputs-spec" / "extended"; @@ -214,40 +213,49 @@ TEST_F(ExtendedOutputsSpecTest, many_carrot) ASSERT_EQ(std::string{prefix} + expected.to_string(), "foo^bar^bin,out"); } -#define TEST_JSON(FIXTURE, TYPE, NAME, VAL) \ - static const TYPE FIXTURE##_##NAME = VAL; \ - \ - TEST_F(FIXTURE, NAME##_from_json) \ - { \ - using namespace nlohmann; \ - \ - readTest(#NAME ".json", [&](const auto & encoded_) { \ - auto encoded = json::parse(encoded_); \ - TYPE got = adl_serializer::from_json(encoded); \ - ASSERT_EQ(got, FIXTURE##_##NAME); \ - }); \ - } \ - \ - TEST_F(FIXTURE, NAME##_to_json) \ - { \ - using namespace nlohmann; \ - \ - writeTest( \ - #NAME ".json", \ - [&]() -> json { return static_cast(FIXTURE##_##NAME); }, \ - [](const auto & file) { return json::parse(readFile(file)); }, \ - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ +#define MAKE_TEST_P(FIXTURE, TYPE) \ + TEST_P(FIXTURE, from_json) \ + { \ + const auto & [name, value] = GetParam(); \ + readJsonTest(name, value); \ + } \ + \ + TEST_P(FIXTURE, to_json) \ + { \ + const auto & [name, value] = GetParam(); \ + writeJsonTest(name, value); \ } -TEST_JSON(OutputsSpecTest, OutputsSpec, all, OutputsSpec::All{}) -TEST_JSON(OutputsSpecTest, OutputsSpec, name, OutputsSpec::Names{"a"}) -TEST_JSON(OutputsSpecTest, OutputsSpec, names, (OutputsSpec::Names{"a", "b"})) +struct OutputsSpecJsonTest : OutputsSpecTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; -TEST_JSON(ExtendedOutputsSpecTest, ExtendedOutputsSpec, def, ExtendedOutputsSpec::Default{}) -TEST_JSON(ExtendedOutputsSpecTest, ExtendedOutputsSpec, all, ExtendedOutputsSpec::Explicit{OutputsSpec::All{}}) -TEST_JSON(ExtendedOutputsSpecTest, ExtendedOutputsSpec, name, ExtendedOutputsSpec::Explicit{OutputsSpec::Names{"a"}}) -TEST_JSON( - ExtendedOutputsSpecTest, ExtendedOutputsSpec, names, (ExtendedOutputsSpec::Explicit{OutputsSpec::Names{"a", "b"}})) +MAKE_TEST_P(OutputsSpecJsonTest, OutputsSpec); + +INSTANTIATE_TEST_SUITE_P( + OutputsSpecJSON, + OutputsSpecJsonTest, + ::testing::Values( + std::pair{"all", OutputsSpec{OutputsSpec::All{}}}, + std::pair{"name", OutputsSpec{OutputsSpec::Names{"a"}}}, + std::pair{"names", OutputsSpec{OutputsSpec::Names{"a", "b"}}})); + +struct ExtendedOutputsSpecJsonTest : ExtendedOutputsSpecTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; + +MAKE_TEST_P(ExtendedOutputsSpecJsonTest, ExtendedOutputsSpec); + +INSTANTIATE_TEST_SUITE_P( + ExtendedOutputsSpecJSON, + ExtendedOutputsSpecJsonTest, + ::testing::Values( + std::pair{"def", ExtendedOutputsSpec{ExtendedOutputsSpec::Default{}}}, + std::pair{"all", ExtendedOutputsSpec{ExtendedOutputsSpec::Explicit{OutputsSpec::All{}}}}, + std::pair{"name", ExtendedOutputsSpec{ExtendedOutputsSpec::Explicit{OutputsSpec::Names{"a"}}}}, + std::pair{"names", ExtendedOutputsSpec{ExtendedOutputsSpec::Explicit{OutputsSpec::Names{"a", "b"}}}})); #undef TEST_JSON diff --git a/src/libstore-tests/path.cc b/src/libstore-tests/path.cc index b6a1a541f..eb860a34d 100644 --- a/src/libstore-tests/path.cc +++ b/src/libstore-tests/path.cc @@ -7,7 +7,7 @@ #include "nix/store/path-regex.hh" #include "nix/store/store-api.hh" -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" #include "nix/store/tests/libstore.hh" #include "nix/store/tests/path.hh" @@ -16,7 +16,7 @@ namespace nix { #define STORE_DIR "/nix/store/" #define HASH_PART "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q" -class StorePathTest : public CharacterizationTest, public LibStoreTest +class StorePathTest : public virtual CharacterizationTest, public LibStoreTest { std::filesystem::path unitTestData = getUnitTestData() / "store-path"; @@ -149,27 +149,30 @@ RC_GTEST_FIXTURE_PROP(StorePathTest, prop_check_regex_eq_parse, ()) using nlohmann::json; -#define TEST_JSON(FIXTURE, NAME, VAL) \ - static const StorePath NAME = VAL; \ - \ - TEST_F(FIXTURE, NAME##_from_json) \ - { \ - readTest(#NAME ".json", [&](const auto & encoded_) { \ - auto encoded = json::parse(encoded_); \ - StorePath got = static_cast(encoded); \ - ASSERT_EQ(got, NAME); \ - }); \ - } \ - \ - TEST_F(FIXTURE, NAME##_to_json) \ - { \ - writeTest( \ - #NAME ".json", \ - [&]() -> json { return static_cast(NAME); }, \ - [](const auto & file) { return json::parse(readFile(file)); }, \ - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ - } +struct StorePathJsonTest : StorePathTest, + JsonCharacterizationTest, + ::testing::WithParamInterface> +{}; -TEST_JSON(StorePathTest, simple, StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}); +TEST_P(StorePathJsonTest, from_json) +{ + auto & [name, expected] = GetParam(); + readJsonTest(name, expected); +} + +TEST_P(StorePathJsonTest, to_json) +{ + auto & [name, value] = GetParam(); + writeJsonTest(name, value); +} + +INSTANTIATE_TEST_SUITE_P( + StorePathJSON, + StorePathJsonTest, + ::testing::Values( + std::pair{ + "simple", + StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, + })); } // namespace nix diff --git a/src/libstore-tests/realisation.cc b/src/libstore-tests/realisation.cc index 2e4d592dc..a5a5bee50 100644 --- a/src/libstore-tests/realisation.cc +++ b/src/libstore-tests/realisation.cc @@ -6,12 +6,12 @@ #include "nix/store/store-api.hh" -#include "nix/util/tests/characterization.hh" +#include "nix/util/tests/json-characterization.hh" #include "nix/store/tests/libstore.hh" namespace nix { -class RealisationTest : public CharacterizationTest, public LibStoreTest +class RealisationTest : public JsonCharacterizationTest, public LibStoreTest { std::filesystem::path unitTestData = getUnitTestData() / "realisation"; @@ -34,22 +34,14 @@ struct RealisationJsonTest : RealisationTest, ::testing::WithParamInterface(encoded); - ASSERT_EQ(got, expected); - }); + const auto & [name, expected] = GetParam(); + readJsonTest(name, expected); } TEST_P(RealisationJsonTest, to_json) { - auto [name, value] = GetParam(); - writeTest( - name + ".json", - [&]() -> json { return static_cast(value); }, - [](const auto & file) { return json::parse(readFile(file)); }, - [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); + const auto & [name, value] = GetParam(); + writeJsonTest(name, value); } INSTANTIATE_TEST_SUITE_P( diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index a0c709791..6d7dbc99c 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1257,14 +1257,18 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const const Hash impureOutputHash = hashString(HashAlgorithm::SHA256, "impure"); -nlohmann::json DerivationOutput::toJSON(std::string_view drvName, OutputNameView outputName) const +nlohmann::json DerivationOutput::toJSON() const { nlohmann::json res = nlohmann::json::object(); std::visit( overloaded{ [&](const DerivationOutput::InputAddressed & doi) { res["path"] = doi.path; }, [&](const DerivationOutput::CAFixed & dof) { - // res["path"] = dof.path(store, drvName, outputName); + /* it would be nice to output the path for user convenience, but + this would require us to know the store dir. */ +#if 0 + res["path"] = dof.path(store, drvName, outputName); +#endif res["method"] = std::string{dof.ca.method.render()}; res["hashAlgo"] = printHashAlgo(dof.ca.hash.algo); res["hash"] = dof.ca.hash.to_string(HashFormat::Base16, false); @@ -1285,11 +1289,8 @@ nlohmann::json DerivationOutput::toJSON(std::string_view drvName, OutputNameView return res; } -DerivationOutput DerivationOutput::fromJSON( - std::string_view drvName, - OutputNameView outputName, - const nlohmann::json & _json, - const ExperimentalFeatureSettings & xpSettings) +DerivationOutput +DerivationOutput::fromJSON(const nlohmann::json & _json, const ExperimentalFeatureSettings & xpSettings) { std::set keys; auto & json = getObject(_json); @@ -1321,6 +1322,8 @@ DerivationOutput DerivationOutput::fromJSON( .hash = Hash::parseNonSRIUnprefixed(getString(valueAt(json, "hash")), hashAlgo), }, }; + /* We no longer produce this (denormalized) field (for the + reasons described above), so we don't need to check it. */ #if 0 if (dof.path(store, drvName, outputName) != static_cast(valueAt(json, "path"))) throw Error("Path doesn't match derivation output"); @@ -1367,7 +1370,7 @@ nlohmann::json Derivation::toJSON() const nlohmann::json & outputsObj = res["outputs"]; outputsObj = nlohmann::json::object(); for (auto & [outputName, output] : outputs) { - outputsObj[outputName] = output.toJSON(name, outputName); + outputsObj[outputName] = output; } } @@ -1427,8 +1430,7 @@ Derivation Derivation::fromJSON(const nlohmann::json & _json, const Experimental try { auto outputs = getObject(valueAt(json, "outputs")); for (auto & [outputName, output] : outputs) { - res.outputs.insert_or_assign( - outputName, DerivationOutput::fromJSON(res.name, outputName, output, xpSettings)); + res.outputs.insert_or_assign(outputName, DerivationOutput::fromJSON(output, xpSettings)); } } catch (Error & e) { e.addTrace({}, "while reading key 'outputs'"); @@ -1489,6 +1491,16 @@ namespace nlohmann { using namespace nix; +DerivationOutput adl_serializer::from_json(const json & json) +{ + return DerivationOutput::fromJSON(json); +} + +void adl_serializer::to_json(json & json, const DerivationOutput & c) +{ + json = c.toJSON(); +} + Derivation adl_serializer::from_json(const json & json) { return Derivation::fromJSON(json); diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index d66bcef2e..0dfb80347 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -135,15 +135,12 @@ struct DerivationOutput std::optional path(const StoreDirConfig & store, std::string_view drvName, OutputNameView outputName) const; - nlohmann::json toJSON(std::string_view drvName, OutputNameView outputName) const; + nlohmann::json toJSON() const; /** * @param xpSettings Stop-gap to avoid globals during unit tests. */ - static DerivationOutput fromJSON( - std::string_view drvName, - OutputNameView outputName, - const nlohmann::json & json, - const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); + static DerivationOutput + fromJSON(const nlohmann::json & json, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); }; typedef std::map DerivationOutputs; @@ -540,4 +537,5 @@ std::string hashPlaceholder(const OutputNameView outputName); } // namespace nix +JSON_IMPL(nix::DerivationOutput) JSON_IMPL(nix::Derivation) diff --git a/src/libutil-test-support/include/nix/util/tests/json-characterization.hh b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh new file mode 100644 index 000000000..5a38b8e2c --- /dev/null +++ b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh @@ -0,0 +1,54 @@ +#pragma once +///@file + +#include +#include + +#include "nix/util/types.hh" +#include "nix/util/file-system.hh" + +#include "nix/util/tests/characterization.hh" + +namespace nix { + +/** + * Mixin class for writing characterization tests for `nlohmann::json` + * conversions for a given type. + */ +template +struct JsonCharacterizationTest : virtual CharacterizationTest +{ + /** + * Golden test for reading + * + * @param test hook that takes the contents of the file and does the + * actual work + */ + void readJsonTest(PathView testStem, const T & expected) + { + using namespace nlohmann; + readTest(Path{testStem} + ".json", [&](const auto & encodedRaw) { + auto encoded = json::parse(encodedRaw); + T decoded = adl_serializer::from_json(encoded); + ASSERT_EQ(decoded, expected); + }); + } + + /** + * Golden test for writing + * + * @param test hook that produces contents of the file and does the + * actual work + */ + void writeJsonTest(PathView testStem, const T & value) + { + using namespace nlohmann; + writeTest( + Path{testStem} + ".json", + [&]() -> json { return static_cast(value); }, + [](const auto & file) { return json::parse(readFile(file)); }, + [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); + } +}; + +} // namespace nix diff --git a/src/libutil-test-support/include/nix/util/tests/meson.build b/src/libutil-test-support/include/nix/util/tests/meson.build index ab143757c..3be085892 100644 --- a/src/libutil-test-support/include/nix/util/tests/meson.build +++ b/src/libutil-test-support/include/nix/util/tests/meson.build @@ -7,6 +7,7 @@ headers = files( 'gmock-matchers.hh', 'gtest-with-params.hh', 'hash.hh', + 'json-characterization.hh', 'nix_api_util.hh', 'string_callback.hh', ) diff --git a/src/libutil-tests/sort.cc b/src/libutil-tests/sort.cc index 8eee961c8..11d8e5938 100644 --- a/src/libutil-tests/sort.cc +++ b/src/libutil-tests/sort.cc @@ -102,14 +102,14 @@ struct RandomPeekSort : public ::testing::TestWithParam< void SetUp() override { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); urng_ = std::mt19937(GTEST_FLAG_GET(random_seed)); distribution_ = std::uniform_int_distribution(min, max); } auto regenerate() { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); std::size_t dataSize = std::uniform_int_distribution(0, maxSize)(urng_); data_.resize(dataSize); std::generate(data_.begin(), data_.end(), [&]() { return distribution_(urng_); }); @@ -118,7 +118,7 @@ struct RandomPeekSort : public ::testing::TestWithParam< TEST_P(RandomPeekSort, defaultComparator) { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); for (std::size_t i = 0; i < iterations; ++i) { regenerate(); @@ -132,7 +132,7 @@ TEST_P(RandomPeekSort, defaultComparator) TEST_P(RandomPeekSort, greater) { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); for (std::size_t i = 0; i < iterations; ++i) { regenerate(); @@ -146,7 +146,7 @@ TEST_P(RandomPeekSort, greater) TEST_P(RandomPeekSort, brokenComparator) { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); /* This is a pretty nice way of modeling a worst-case scenario for a broken comparator. If the sorting algorithm doesn't break in such case, then surely all deterministic @@ -170,7 +170,7 @@ TEST_P(RandomPeekSort, brokenComparator) TEST_P(RandomPeekSort, stability) { - auto [maxSize, min, max, iterations] = GetParam(); + const auto & [maxSize, min, max, iterations] = GetParam(); for (std::size_t i = 0; i < iterations; ++i) { regenerate();