From 4de54b21907e3fc41b9e6250396eb01d2b10c4db Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 4 Oct 2023 23:27:50 -0400 Subject: [PATCH] Unit test the "common protocol" too Copy the relevant tests to ensure the new interfaces added in the last commit are tested. Perhaps I should try to deduplicat these tests some more. However its not clear how to do that outside of a big ugly C++ macro. https://github.com/google/googletest/blob/main/docs/advanced.md has some stuff but it is cumbersome and I didn't figure it out yet. This is done in a separate commit in order to be sure that the first commit really didn't change any behavior; if we changed the implementation and the tests at once, it would be harder to tell whether or not some behavioral changes slipped in what is supposed to be a "pure refactor". Co-Authored-By: Valentin Gagarin --- src/libstore/tests/characterization.hh | 23 +++ src/libstore/tests/common-protocol.cc | 152 ++++++++++++++++++ src/libstore/tests/protocol.hh | 88 ++++++++++ src/libstore/tests/worker-protocol.cc | 90 ++--------- .../common-protocol/content-address.bin | Bin 0 -> 208 bytes .../libstore/common-protocol/drv-output.bin | Bin 0 -> 176 bytes .../optional-content-address.bin | Bin 0 -> 64 bytes .../common-protocol/optional-store-path.bin | Bin 0 -> 72 bytes .../libstore/common-protocol/realisation.bin | Bin 0 -> 520 bytes .../libstore/common-protocol/set.bin | Bin 0 -> 152 bytes .../libstore/common-protocol/store-path.bin | Bin 0 -> 120 bytes .../libstore/common-protocol/string.bin | Bin 0 -> 88 bytes .../libstore/common-protocol/vector.bin | Bin 0 -> 152 bytes 13 files changed, 280 insertions(+), 73 deletions(-) create mode 100644 src/libstore/tests/characterization.hh create mode 100644 src/libstore/tests/common-protocol.cc create mode 100644 src/libstore/tests/protocol.hh create mode 100644 unit-test-data/libstore/common-protocol/content-address.bin create mode 100644 unit-test-data/libstore/common-protocol/drv-output.bin create mode 100644 unit-test-data/libstore/common-protocol/optional-content-address.bin create mode 100644 unit-test-data/libstore/common-protocol/optional-store-path.bin create mode 100644 unit-test-data/libstore/common-protocol/realisation.bin create mode 100644 unit-test-data/libstore/common-protocol/set.bin create mode 100644 unit-test-data/libstore/common-protocol/store-path.bin create mode 100644 unit-test-data/libstore/common-protocol/string.bin create mode 100644 unit-test-data/libstore/common-protocol/vector.bin diff --git a/src/libstore/tests/characterization.hh b/src/libstore/tests/characterization.hh new file mode 100644 index 000000000..5f366cb42 --- /dev/null +++ b/src/libstore/tests/characterization.hh @@ -0,0 +1,23 @@ +#pragma once +///@file + +namespace nix { + +/** + * The path to the `unit-test-data` directory. See the contributing + * guide in the manual for further details. + */ +static Path getUnitTestData() { + return getEnv("_NIX_TEST_UNIT_DATA").value(); +} + +/** + * Whether we should update "golden masters" instead of running tests + * against them. See the contributing guide in the manual for further + * details. + */ +static bool testAccept() { + return getEnv("_NIX_TEST_ACCEPT") == "1"; +} + +} diff --git a/src/libstore/tests/common-protocol.cc b/src/libstore/tests/common-protocol.cc new file mode 100644 index 000000000..de57211f0 --- /dev/null +++ b/src/libstore/tests/common-protocol.cc @@ -0,0 +1,152 @@ +#include + +#include +#include + +#include "common-protocol.hh" +#include "common-protocol-impl.hh" +#include "build-result.hh" +#include "tests/protocol.hh" +#include "tests/characterization.hh" + +namespace nix { + +const char commonProtoDir[] = "common-protocol"; + +using CommonProtoTest = ProtoTest; + +CHARACTERIZATION_TEST( + CommonProtoTest, + string, + "string", + (std::tuple { + "", + "hi", + "white rabbit", + "大白兔", + "oh no \0\0\0 what was that!", + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + storePath, + "store-path", + (std::tuple { + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + contentAddress, + "content-address", + (std::tuple { + ContentAddress { + .method = TextIngestionMethod {}, + .hash = hashString(HashType::htSHA256, "Derive(...)"), + }, + ContentAddress { + .method = FileIngestionMethod::Flat, + .hash = hashString(HashType::htSHA1, "blob blob..."), + }, + ContentAddress { + .method = FileIngestionMethod::Recursive, + .hash = hashString(HashType::htSHA256, "(...)"), + }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + drvOutput, + "drv-output", + (std::tuple { + { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", + }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + realisation, + "realisation", + (std::tuple { + Realisation { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + }, + Realisation { + .id = { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + .dependentRealisations = { + { + DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", + }, + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + vector, + "vector", + (std::tuple, std::vector, std::vector, std::vector>> { + { }, + { "" }, + { "", "foo", "bar" }, + { {}, { "" }, { "", "1", "2" } }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + set, + "set", + (std::tuple, std::set, std::set, std::set>> { + { }, + { "" }, + { "", "foo", "bar" }, + { {}, { "" }, { "", "1", "2" } }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + optionalStorePath, + "optional-store-path", + (std::tuple, std::optional> { + std::nullopt, + std::optional { + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" }, + }, + })) + +CHARACTERIZATION_TEST( + CommonProtoTest, + optionalContentAddress, + "optional-content-address", + (std::tuple, std::optional> { + std::nullopt, + std::optional { + ContentAddress { + .method = FileIngestionMethod::Flat, + .hash = hashString(HashType::htSHA1, "blob blob..."), + }, + }, + })) + +} diff --git a/src/libstore/tests/protocol.hh b/src/libstore/tests/protocol.hh new file mode 100644 index 000000000..86a900757 --- /dev/null +++ b/src/libstore/tests/protocol.hh @@ -0,0 +1,88 @@ +#include +#include + +#include "tests/libstore.hh" +#include "tests/characterization.hh" + +namespace nix { + +template +class ProtoTest : public LibStoreTest +{ + /** + * Read this as simply `using S = Inner::Serialise;`. + * + * See `LengthPrefixedProtoHelper::S` for the same trick, and its + * rationale. + */ + template using S = typename Proto::template Serialise; + +public: + Path unitTestData = getUnitTestData() + "/libstore/" + protocolDir; + + Path goldenMaster(std::string_view testStem) { + return unitTestData + "/" + testStem + ".bin"; + } + + /** + * Golden test for `T` reading + */ + template + void readTest(PathView testStem, T value) + { + if (testAccept()) + { + GTEST_SKIP() << "Cannot read golden master because another test is also updating it"; + } + else + { + auto expected = readFile(goldenMaster(testStem)); + + T got = ({ + StringSource from { expected }; + S::read( + *store, + typename Proto::ReadConn { .from = from }); + }); + + ASSERT_EQ(got, value); + } + } + + /** + * Golden test for `T` write + */ + template + void writeTest(PathView testStem, const T & value) + { + auto file = goldenMaster(testStem); + + StringSink to; + Proto::write( + *store, + typename Proto::WriteConn { .to = to }, + value); + + if (testAccept()) + { + createDirs(dirOf(file)); + writeFile(file, to.s); + GTEST_SKIP() << "Updating golden master"; + } + else + { + auto expected = readFile(file); + ASSERT_EQ(to.s, expected); + } + } +}; + +#define CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VALUE) \ + TEST_F(FIXTURE, NAME ## _read) { \ + readTest(STEM, VALUE); \ + } \ + TEST_F(FIXTURE, NAME ## _write) { \ + writeTest(STEM, VALUE); \ + } + +} diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index fa7cbe121..59d7ff96d 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -7,85 +7,17 @@ #include "worker-protocol-impl.hh" #include "derived-path.hh" #include "build-result.hh" -#include "tests/libstore.hh" +#include "tests/protocol.hh" +#include "tests/characterization.hh" namespace nix { -class WorkerProtoTest : public LibStoreTest -{ -public: - Path unitTestData = getEnv("_NIX_TEST_UNIT_DATA").value() + "/libstore/worker-protocol"; +const char workerProtoDir[] = "worker-protocol"; - bool testAccept() { - return getEnv("_NIX_TEST_ACCEPT") == "1"; - } - - Path goldenMaster(std::string_view testStem) { - return unitTestData + "/" + testStem + ".bin"; - } - - /** - * Golden test for `T` reading - */ - template - void readTest(PathView testStem, T value) - { - if (testAccept()) - { - GTEST_SKIP() << "Cannot read golden master because another test is also updating it"; - } - else - { - auto expected = readFile(goldenMaster(testStem)); - - T got = ({ - StringSource from { expected }; - WorkerProto::Serialise::read( - *store, - WorkerProto::ReadConn { .from = from }); - }); - - ASSERT_EQ(got, value); - } - } - - /** - * Golden test for `T` write - */ - template - void writeTest(PathView testStem, const T & value) - { - auto file = goldenMaster(testStem); - - StringSink to; - WorkerProto::write( - *store, - WorkerProto::WriteConn { .to = to }, - value); - - if (testAccept()) - { - createDirs(dirOf(file)); - writeFile(file, to.s); - GTEST_SKIP() << "Updating golden master"; - } - else - { - auto expected = readFile(file); - ASSERT_EQ(to.s, expected); - } - } -}; - -#define CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ - TEST_F(WorkerProtoTest, NAME ## _read) { \ - readTest(STEM, VALUE); \ - } \ - TEST_F(WorkerProtoTest, NAME ## _write) { \ - writeTest(STEM, VALUE); \ - } +using WorkerProtoTest = ProtoTest; CHARACTERIZATION_TEST( + WorkerProtoTest, string, "string", (std::tuple { @@ -97,6 +29,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, storePath, "store-path", (std::tuple { @@ -105,6 +38,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, contentAddress, "content-address", (std::tuple { @@ -123,6 +57,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, derivedPath, "derived-path", (std::tuple { @@ -138,6 +73,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, drvOutput, "drv-output", (std::tuple { @@ -152,6 +88,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, realisation, "realisation", (std::tuple { @@ -183,6 +120,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, buildResult, "build-result", ({ @@ -240,6 +178,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, keyedBuildResult, "keyed-build-result", ({ @@ -275,6 +214,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, optionalTrustedFlag, "optional-trusted-flag", (std::tuple, std::optional, std::optional> { @@ -284,6 +224,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, vector, "vector", (std::tuple, std::vector, std::vector, std::vector>> { @@ -294,6 +235,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, set, "set", (std::tuple, std::set, std::set, std::set>> { @@ -304,6 +246,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, optionalStorePath, "optional-store-path", (std::tuple, std::optional> { @@ -314,6 +257,7 @@ CHARACTERIZATION_TEST( })) CHARACTERIZATION_TEST( + WorkerProtoTest, optionalContentAddress, "optional-content-address", (std::tuple, std::optional> { diff --git a/unit-test-data/libstore/common-protocol/content-address.bin b/unit-test-data/libstore/common-protocol/content-address.bin new file mode 100644 index 0000000000000000000000000000000000000000..8f14bcdb3e50cc72d87106ce159a6fd7b9f75713 GIT binary patch literal 208 zcmY+;K@P$o5QSmy;|7WYrYjRqQfMm$+LWPzDW_MfG4bucKks(>Y#V56lkFOiEzi24 zUMdC5VUCa86OJ&gL0BF^B{HBQEY%f|I<6v7#q+l_PBirI5N~-md%37WE|g33QTE2k zc~-WF&R}7Oxc@o)T?ojpD!+*5=xpO;%IoCKSV5FMh={>xePK|8<${>E)*huNHZ(pX literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/drv-output.bin b/unit-test-data/libstore/common-protocol/drv-output.bin new file mode 100644 index 0000000000000000000000000000000000000000..800a45fd8757a15e8810066aed48be56a600b7d3 GIT binary patch literal 176 zcmY++xeWp_5I|8{*$(Wn=b{B@!gGlfp_LHTb8N(qe)KM%U7Sq@}q)U|6h9n2j1!MT?w+C~^gO WO9=?G&ojrYjy`x4ZufnEe#tjMpDPdm literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/optional-content-address.bin b/unit-test-data/libstore/common-protocol/optional-content-address.bin new file mode 100644 index 0000000000000000000000000000000000000000..f8cfe65ba27fd78ec7787eed7ebec8dfdead2fba GIT binary patch literal 64 zcmZQzfB#ECTBU06lLKJOBUy literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/realisation.bin b/unit-test-data/libstore/common-protocol/realisation.bin new file mode 100644 index 0000000000000000000000000000000000000000..2176c6c4afd96b32fe372de6084ac7f4c7a11d49 GIT binary patch literal 520 zcmdUrO-{o=429t+opq7s&z_l_03#3x z?rW}!>Uc!$VBY>Sr2mUC`<2<)qY;)1KN Po44)luetw6d9AunL$;dR literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/set.bin b/unit-test-data/libstore/common-protocol/set.bin new file mode 100644 index 0000000000000000000000000000000000000000..ce11ede7fe7aa061a30b211a18cd7a336f879de8 GIT binary patch literal 152 ucmZQzfB;4)4WpQ03@8obCnXkvMPU52{CpHXOdBEdVDg4g4KThDln(&+h63;a literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/store-path.bin b/unit-test-data/libstore/common-protocol/store-path.bin new file mode 100644 index 0000000000000000000000000000000000000000..3fc05f2981d3398ab30ec34125c903cdcefa8729 GIT binary patch literal 120 tcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7Eu*F?srQlM;)-IsqP{BM|@q literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/common-protocol/string.bin b/unit-test-data/libstore/common-protocol/string.bin new file mode 100644 index 0000000000000000000000000000000000000000..aa7b5a604745b735473ec34283177e69320776e4 GIT binary patch literal 88 zcmZQzfB+^aoskJ)@Id+H8JQ)i3Pp)YNtq=eAx^0H(