From 0faf5326bd333eeef126730683dc02009a06402f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 11 Jan 2023 17:31:32 -0500 Subject: [PATCH] Improve tests for `OutputsSpec` --- src/libstore/outputs-spec.cc | 21 +++-- src/libstore/outputs-spec.hh | 9 +-- src/libstore/tests/outputs-spec.cc | 123 ++++++++++++++++++----------- 3 files changed, 97 insertions(+), 56 deletions(-) diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc index 1eeab1aa8..8e6e40c2b 100644 --- a/src/libstore/outputs-spec.cc +++ b/src/libstore/outputs-spec.cc @@ -40,22 +40,33 @@ std::optional OutputsSpec::parseOpt(std::string_view s) OutputsSpec OutputsSpec::parse(std::string_view s) { - std::optional spec = OutputsSpec::parseOpt(s); + std::optional spec = parseOpt(s); if (!spec) throw Error("Invalid outputs specifier: '%s'", s); return *spec; } -std::pair ExtendedOutputsSpec::parse(std::string_view s) +std::optional> ExtendedOutputsSpec::parseOpt(std::string_view s) { auto found = s.rfind('^'); if (found == std::string::npos) - return { s, ExtendedOutputsSpec::Default {} }; + return std::pair { s, ExtendedOutputsSpec::Default {} }; - auto spec = OutputsSpec::parse(s.substr(found + 1)); - return { s.substr(0, found), ExtendedOutputsSpec::Explicit { spec } }; + auto specOpt = OutputsSpec::parseOpt(s.substr(found + 1)); + if (!specOpt) + return std::nullopt; + return std::pair { s.substr(0, found), ExtendedOutputsSpec::Explicit { *std::move(specOpt) } }; +} + + +std::pair ExtendedOutputsSpec::parse(std::string_view s) +{ + std::optional spec = parseOpt(s); + if (!spec) + throw Error("Invalid extended outputs specifier: '%s'", s); + return *spec; } diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh index babf29d16..9211a4fc6 100644 --- a/src/libstore/outputs-spec.hh +++ b/src/libstore/outputs-spec.hh @@ -25,9 +25,7 @@ struct OutputNames : std::set { OutputNames() = delete; }; -struct AllOutputs { - bool operator < (const AllOutputs & _) const { return false; } -}; +struct AllOutputs : std::monostate { }; typedef std::variant _OutputsSpecRaw; @@ -64,9 +62,7 @@ struct OutputsSpec : _OutputsSpecRaw { std::string to_string() const; }; -struct DefaultOutputs { - bool operator < (const DefaultOutputs & _) const { return false; } -}; +struct DefaultOutputs : std::monostate { }; typedef std::variant _ExtendedOutputsSpecRaw; @@ -84,6 +80,7 @@ struct ExtendedOutputsSpec : _ExtendedOutputsSpecRaw { /* Parse a string of the form 'prefix^output1,...outputN' or 'prefix^*', returning the prefix and the extended outputs spec. */ static std::pair parse(std::string_view s); + static std::optional> parseOpt(std::string_view s); std::string to_string() const; }; diff --git a/src/libstore/tests/outputs-spec.cc b/src/libstore/tests/outputs-spec.cc index 03259e7c7..6d07795aa 100644 --- a/src/libstore/tests/outputs-spec.cc +++ b/src/libstore/tests/outputs-spec.cc @@ -4,63 +4,96 @@ namespace nix { -TEST(OutputsSpec_parse, basic) -{ - { - auto outputsSpec = OutputsSpec::parse("*"); - ASSERT_TRUE(std::get_if(&outputsSpec)); +#define TEST_DONT_PARSE(NAME, STR) \ + TEST(OutputsSpec, bad_ ## NAME) { \ + std::optional OutputsSpecOpt = \ + OutputsSpec::parseOpt(STR); \ + ASSERT_FALSE(OutputsSpecOpt); \ } - { - auto outputsSpec = OutputsSpec::parse("out"); - ASSERT_TRUE(std::get(outputsSpec) == OutputsSpec::Names({"out"})); - } +TEST_DONT_PARSE(empty, "") +TEST_DONT_PARSE(garbage, "&*()") +TEST_DONT_PARSE(double_star, "**") +TEST_DONT_PARSE(star_first, "*,foo") +TEST_DONT_PARSE(star_second, "foo,*") - { - auto outputsSpec = OutputsSpec::parse("out,bin"); - ASSERT_TRUE(std::get(outputsSpec) == OutputsSpec::Names({"out", "bin"})); - } +#undef TEST_DONT_PARSE - { - std::optional outputsSpecOpt = OutputsSpec::parseOpt("&*()"); - ASSERT_FALSE(outputsSpecOpt); - } +TEST(OutputsSpec, all) { + std::string_view str = "*"; + OutputsSpec expected = OutputsSpec::All { }; + ASSERT_EQ(OutputsSpec::parse(str), expected); + ASSERT_EQ(expected.to_string(), str); +} + +TEST(OutputsSpec, names_out) { + std::string_view str = "out"; + OutputsSpec expected = OutputsSpec::Names { "out" }; + ASSERT_EQ(OutputsSpec::parse(str), expected); + ASSERT_EQ(expected.to_string(), str); +} + +TEST(OutputsSpec, names_out_bin) { + OutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(OutputsSpec::parse("out,bin"), expected); + // N.B. This normalization is OK. + ASSERT_EQ(expected.to_string(), "bin,out"); } -TEST(ExtendedOutputsSpec_parse, basic) -{ - { - auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get_if(&extendedOutputsSpec)); +#define TEST_DONT_PARSE(NAME, STR) \ + TEST(ExtendedOutputsSpec, bad_ ## NAME) { \ + std::optional extendedOutputsSpecOpt = \ + ExtendedOutputsSpec::parseOpt(STR); \ + ASSERT_FALSE(extendedOutputsSpecOpt); \ } - { - auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^*"); - ASSERT_EQ(prefix, "foo"); - auto * explicit_p = std::get_if(&extendedOutputsSpec); - ASSERT_TRUE(explicit_p); - ASSERT_TRUE(std::get_if(explicit_p)); - } +TEST_DONT_PARSE(carot_empty, "^") +TEST_DONT_PARSE(prefix_carot_empty, "foo^") - { - auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get(std::get(extendedOutputsSpec)) == OutputsSpec::Names({"out"})); - } +#undef TEST_DONT_PARSE - { - auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out,bin"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get(std::get(extendedOutputsSpec)) == OutputsSpec::Names({"out", "bin"})); - } +TEST(ExtendedOutputsSpec, defeault) { + std::string_view str = "foo"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = ExtendedOutputsSpec::Default { }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); +} - { - auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^bar^out,bin"); - ASSERT_EQ(prefix, "foo^bar"); - ASSERT_TRUE(std::get(std::get(extendedOutputsSpec)) == OutputsSpec::Names({"out", "bin"})); - } +TEST(ExtendedOutputsSpec, all) { + std::string_view str = "foo^*"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::All { }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); +} + +TEST(ExtendedOutputsSpec, out) { + std::string_view str = "foo^out"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); +} + +TEST(ExtendedOutputsSpec, out_bin) { + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out,bin"); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), "foo^bin,out"); +} + +TEST(ExtendedOutputsSpec, many_carrot) { + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^bar^out,bin"); + ASSERT_EQ(prefix, "foo^bar"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), "foo^bar^bin,out"); } }