From f17e7b185597715049fa7a12e6a0512ef66801b6 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 03:46:48 +0100 Subject: [PATCH] Merge pull request #8923 from obsidiansystems/test-proto Unit test some worker protocol serializers (cherry picked from commit c6faef61a6f31c71146aee5d88168e861df9a22a) Change-Id: I99e36f5f17eb7642211a4e42a16b143424f164b4 --- Makefile | 1 + doc/manual/src/contributing/testing.md | 64 +++++++- flake.nix | 1 + mk/programs.mk | 2 +- src/libstore/worker-protocol-impl.hh | 16 ++ src/libstore/worker-protocol.hh | 4 + tests/unit/libstore/worker-protocol.cc | 139 ++++++++++++++++++ .../worker-protocol/content-address.bin | Bin 0 -> 208 bytes .../libstore/worker-protocol/derived-path.bin | Bin 0 -> 120 bytes .../libstore/worker-protocol/store-path.bin | Bin 0 -> 120 bytes .../libstore/worker-protocol/string.bin | Bin 0 -> 88 bytes 11 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 tests/unit/libstore/worker-protocol.cc create mode 100644 unit-test-data/libstore/worker-protocol/content-address.bin create mode 100644 unit-test-data/libstore/worker-protocol/derived-path.bin create mode 100644 unit-test-data/libstore/worker-protocol/store-path.bin create mode 100644 unit-test-data/libstore/worker-protocol/string.bin diff --git a/Makefile b/Makefile index 41c0ab68a..e0be7cdcd 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ makefiles = \ -include Makefile.config ifeq ($(tests), yes) +UNIT_TEST_ENV = _NIX_TEST_UNIT_DATA=unit-test-data makefiles += \ tests/unit/libutil/local.mk \ tests/unit/libutil-support/local.mk \ diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md index 1b15d12b1..6f954ac68 100644 --- a/doc/manual/src/contributing/testing.md +++ b/doc/manual/src/contributing/testing.md @@ -2,10 +2,48 @@ ## Unit-tests -The unit-tests for each Nix library (`libexpr`, `libstore`, etc..) are defined -under `tests/unit/{library_name}/tests` using the -[googletest](https://google.github.io/googletest/) and -[rapidcheck](https://github.com/emil-e/rapidcheck) frameworks. +The unit tests are defined using the [googletest] and [rapidcheck] frameworks. + +[googletest]: https://google.github.io/googletest/ +[rapidcheck]: https://github.com/emil-e/rapidcheck + +### Source and header layout + +> An example of some files, demonstrating much of what is described below +> +> ``` +> src +> ├── libexpr +> │ ├── value/context.hh +> │ ├── value/context.cc +> │ │ +> │ … +> └── tests +> │ ├── value/context.hh +> │ ├── value/context.cc +> │ │ +> │ … +> │ +> ├── unit-test-data +> │ ├── libstore +> │ │ ├── worker-protocol/content-address.bin +> │ │ … +> │ … +> … +> ``` + +The unit tests for each Nix library (`libnixexpr`, `libnixstore`, etc..) live inside a directory `src/${library_shortname}/tests` within the directory for the library (`src/${library_shortname}`). + +The data is in `unit-test-data`, with one subdir per library, with the same name as where the code goes. +For example, `libnixstore` code is in `src/libstore`, and its test data is in `unit-test-data/libstore`. +The path to the `unit-test-data` directory is passed to the unit test executable with the environment variable `_NIX_TEST_UNIT_DATA`. + +> **Note** +> Due to the way googletest works, downstream unit test executables will actually include and re-run upstream library tests. +> Therefore it is important that the same value for `_NIX_TEST_UNIT_DATA` be used with the tests for each library. +> That is why we have the test data nested within a single `unit-test-data` directory. + +### Running tests You can run the whole testsuite with `make check`, or the tests for a specific component with `make libfoo-tests_RUN`. Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option, or the `GTEST_FILTER` environment variable. @@ -22,6 +60,24 @@ It is important that these testing libraries don't contain any actual tests them On some platforms they would be run as part of every test executable that uses them, which is redundant. On other platforms they wouldn't be run at all. +### Characterization testing + +See [below](#characterization-testing-1) for a broader discussion of characterization testing. + +Like with the functional characterization, `_NIX_TEST_ACCEPT=1` is also used. +For example: +```shell-session +$ _NIX_TEST_ACCEPT=1 make libstore-tests-exe_RUN +... +[ SKIPPED ] WorkerProtoTest.string_read +[ SKIPPED ] WorkerProtoTest.string_write +[ SKIPPED ] WorkerProtoTest.storePath_read +[ SKIPPED ] WorkerProtoTest.storePath_write +... +``` +will regenerate the "golden master" expected result for the `libnixstore` characterization tests. +The characterization tests will mark themselves "skipped" since they regenerated the expected result instead of actually testing anything. + ## Functional tests The functional tests reside under the `tests/functional` directory and are listed in `tests/functional/local.mk`. diff --git a/flake.nix b/flake.nix index 0722d4a5b..ac1d0606a 100644 --- a/flake.nix +++ b/flake.nix @@ -77,6 +77,7 @@ ./src ./tests/functional ./tests/unit + ./unit-test-data ./COPYING ./scripts/local.mk (fileset.fileFilter (f: lib.strings.hasPrefix "nix-profile" f.name) ./scripts) diff --git a/mk/programs.mk b/mk/programs.mk index 1ee1d3fa5..a88d9d949 100644 --- a/mk/programs.mk +++ b/mk/programs.mk @@ -87,6 +87,6 @@ define build-program # Phony target to run this program (typically as a dependency of 'check'). .PHONY: $(1)_RUN $(1)_RUN: $$($(1)_PATH) - $(trace-test) $$($(1)_PATH) + $(trace-test) $$(UNIT_TEST_ENV) $$($(1)_PATH) endef diff --git a/src/libstore/worker-protocol-impl.hh b/src/libstore/worker-protocol-impl.hh index d3d2792ff..4f797f95a 100644 --- a/src/libstore/worker-protocol-impl.hh +++ b/src/libstore/worker-protocol-impl.hh @@ -75,4 +75,20 @@ void WorkerProto::Serialise>::write(const Store & store, WorkerPr } } +template +std::tuple WorkerProto::Serialise>::read(const Store & store, WorkerProto::ReadConn conn) +{ + return std::tuple { + WorkerProto::Serialise::read(store, conn)..., + }; +} + +template +void WorkerProto::Serialise>::write(const Store & store, WorkerProto::WriteConn conn, const std::tuple & res) +{ + std::apply([&](const Us &... args) { + (WorkerProto::Serialise::write(store, conn, args), ...); + }, res); +} + } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index ff762c924..70a5bddb9 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -28,6 +28,8 @@ class Store; struct Source; // items being serialised +class StorePath; +struct ContentAddress; struct DerivedPath; struct DrvOutput; struct Realisation; @@ -220,6 +222,8 @@ template MAKE_WORKER_PROTO(std::vector); template MAKE_WORKER_PROTO(std::set); +template +MAKE_WORKER_PROTO(std::tuple); template #define X_ std::map diff --git a/tests/unit/libstore/worker-protocol.cc b/tests/unit/libstore/worker-protocol.cc new file mode 100644 index 000000000..4a6ccf7c0 --- /dev/null +++ b/tests/unit/libstore/worker-protocol.cc @@ -0,0 +1,139 @@ +#include + +#include +#include + +#include "worker-protocol.hh" +#include "worker-protocol-impl.hh" +#include "derived-path.hh" +#include "tests/libstore.hh" + +namespace nix { + +class WorkerProtoTest : public LibStoreTest +{ +public: + Path unitTestData = getEnv("_NIX_TEST_UNIT_DATA").value() + "/libstore/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); \ + } + +CHARACTERIZATION_TEST( + string, + "string", + (std::tuple { + "", + "hi", + "white rabbit", + "大白兔", + "oh no \0\0\0 what was that!", + })) + +CHARACTERIZATION_TEST( + storePath, + "store-path", + (std::tuple { + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" }, + })) + +CHARACTERIZATION_TEST( + 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( + derivedPath, + "derived-path", + (std::tuple { + DerivedPath::Opaque { + .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }), + .outputs = OutputsSpec::Names { "x", "y" }, + }, + })) + +} diff --git a/unit-test-data/libstore/worker-protocol/content-address.bin b/unit-test-data/libstore/worker-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/worker-protocol/derived-path.bin b/unit-test-data/libstore/worker-protocol/derived-path.bin new file mode 100644 index 0000000000000000000000000000000000000000..bb1a81ac6d096cddf510a457e7506e6a81ee66a3 GIT binary patch literal 120 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7Eu*IT`wr5{vXwipmr#bSfDDBbg*z literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/worker-protocol/store-path.bin b/unit-test-data/libstore/worker-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/worker-protocol/string.bin b/unit-test-data/libstore/worker-protocol/string.bin new file mode 100644 index 0000000000000000000000000000000000000000..aa7b5a604745b735473ec34283177e69320776e4 GIT binary patch literal 88 zcmZQzfB+^aoskJ)@Id+H8JQ)i3Pp)YNtq=eAx^0H(