From 9a04f1e73214df9cc477a36d219fcfede7bc763c Mon Sep 17 00:00:00 2001 From: Brian McKenna Date: Fri, 7 Mar 2025 23:20:11 +1100 Subject: [PATCH] rapidcheck: change to working arbitrary instances Here we're switching to combinators instead of dereference operator. It turns out the dereference operator was being executed upon test setup, meaning that we were only using a only single value for each of the executions of the property tests! Really not good. And on Windows, we instead get: operator* is not allowed in this context https://github.com/emil-e/rapidcheck/blob/ff6af6fc683159deb51c543b065eba14dfcf329b/src/gen/detail/GenerationHandler.cpp#L16C31-L16C71 Now a few of the property tests fail, because we're generating cases which haven't been exercised before. --- .../tests/value/context.cc | 33 ++++++---- .../tests/derived-path.cc | 62 ++++++++++++------- .../tests/outputs-spec.cc | 24 +++---- 3 files changed, 72 insertions(+), 47 deletions(-) diff --git a/src/libexpr-test-support/tests/value/context.cc b/src/libexpr-test-support/tests/value/context.cc index 8658bdaef..36837cd6a 100644 --- a/src/libexpr-test-support/tests/value/context.cc +++ b/src/libexpr-test-support/tests/value/context.cc @@ -8,23 +8,32 @@ using namespace nix; Gen Arbitrary::arbitrary() { - return gen::just(NixStringContextElem::DrvDeep { - .drvPath = *gen::arbitrary(), + return gen::map(gen::arbitrary(), [](StorePath drvPath) { + return NixStringContextElem::DrvDeep{ + .drvPath = drvPath, + }; }); } Gen Arbitrary::arbitrary() { - switch (*gen::inRange(0, std::variant_size_v)) { - case 0: - return gen::just(*gen::arbitrary()); - case 1: - return gen::just(*gen::arbitrary()); - case 2: - return gen::just(*gen::arbitrary()); - default: - assert(false); - } + return gen::mapcat( + gen::inRange(0, std::variant_size_v), + [](uint8_t n) -> Gen { + switch (n) { + case 0: + return gen::map( + gen::arbitrary(), [](NixStringContextElem a) { return a; }); + case 1: + return gen::map( + gen::arbitrary(), [](NixStringContextElem a) { return a; }); + case 2: + return gen::map( + gen::arbitrary(), [](NixStringContextElem a) { return a; }); + default: + assert(false); + } + }); } } diff --git a/src/libstore-test-support/tests/derived-path.cc b/src/libstore-test-support/tests/derived-path.cc index 078615bbd..b9f6a3171 100644 --- a/src/libstore-test-support/tests/derived-path.cc +++ b/src/libstore-test-support/tests/derived-path.cc @@ -9,49 +9,63 @@ using namespace nix; Gen Arbitrary::arbitrary() { - return gen::just(DerivedPath::Opaque { - .path = *gen::arbitrary(), + return gen::map(gen::arbitrary(), [](StorePath path) { + return DerivedPath::Opaque{ + .path = path, + }; }); } Gen Arbitrary::arbitrary() { - return gen::just(SingleDerivedPath::Built { - .drvPath = make_ref(*gen::arbitrary()), - .output = (*gen::arbitrary()).name, + return gen::mapcat(gen::arbitrary(), [](SingleDerivedPath drvPath) { + return gen::map(gen::arbitrary(), [drvPath](StorePathName outputPath) { + return SingleDerivedPath::Built{ + .drvPath = make_ref(drvPath), + .output = outputPath.name, + }; + }); }); } Gen Arbitrary::arbitrary() { - return gen::just(DerivedPath::Built { - .drvPath = make_ref(*gen::arbitrary()), - .outputs = *gen::arbitrary(), + return gen::mapcat(gen::arbitrary(), [](SingleDerivedPath drvPath) { + return gen::map(gen::arbitrary(), [drvPath](OutputsSpec outputs) { + return DerivedPath::Built{ + .drvPath = make_ref(drvPath), + .outputs = outputs, + }; + }); }); } Gen Arbitrary::arbitrary() { - switch (*gen::inRange(0, std::variant_size_v)) { - case 0: - return gen::just(*gen::arbitrary()); - case 1: - return gen::just(*gen::arbitrary()); - default: - assert(false); - } + return gen::mapcat(gen::inRange(0, std::variant_size_v), [](uint8_t n) { + switch (n) { + case 0: + return gen::map(gen::arbitrary(), [](SingleDerivedPath a) { return a; }); + case 1: + return gen::map(gen::arbitrary(), [](SingleDerivedPath a) { return a; }); + default: + assert(false); + } + }); } Gen Arbitrary::arbitrary() { - switch (*gen::inRange(0, std::variant_size_v)) { - case 0: - return gen::just(*gen::arbitrary()); - case 1: - return gen::just(*gen::arbitrary()); - default: - assert(false); - } + return gen::mapcat(gen::inRange(0, std::variant_size_v), [](uint8_t n) { + switch (n) { + case 0: + return gen::map(gen::arbitrary(), [](DerivedPath a) { return a; }); + case 1: + return gen::map(gen::arbitrary(), [](DerivedPath a) { return a; }); + default: + assert(false); + } + }); } } diff --git a/src/libstore-test-support/tests/outputs-spec.cc b/src/libstore-test-support/tests/outputs-spec.cc index e9d602203..1a3020f17 100644 --- a/src/libstore-test-support/tests/outputs-spec.cc +++ b/src/libstore-test-support/tests/outputs-spec.cc @@ -7,18 +7,20 @@ using namespace nix; Gen Arbitrary::arbitrary() { - switch (*gen::inRange(0, std::variant_size_v)) { - case 0: - return gen::just((OutputsSpec) OutputsSpec::All { }); - case 1: - return gen::just((OutputsSpec) OutputsSpec::Names { - *gen::nonEmpty(gen::container(gen::map( - gen::arbitrary(), - [](StorePathName n) { return n.name; }))), + return gen::mapcat( + gen::inRange(0, std::variant_size_v), [](uint8_t n) -> Gen { + switch (n) { + case 0: + return gen::just((OutputsSpec) OutputsSpec::All{}); + case 1: + return gen::map( + gen::nonEmpty(gen::container( + gen::map(gen::arbitrary(), [](StorePathName n) { return n.name; }))), + [](StringSet names) { return (OutputsSpec) OutputsSpec::Names{names}; }); + default: + assert(false); + } }); - default: - assert(false); - } } }