From 32a1be3690bcbbac4ce6e06bb2ef32394378b890 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Fri, 15 Sep 2023 14:28:15 -0700 Subject: [PATCH] Detect invalid yaml. (#3239) Previous code could fail silently, only caught with yaml output mismatches. This uses ErrorOr to be explicit about the error detection; recovery isn't supported because we should only print valid yaml (in tests, at least). Also moves the test helpers from //toolchain/base to //toolchain/testing. Switches tree_test to use the matchers so that, on mismatch, gtest prints a path to the mismatch (the straight value means it was just printing "not equal"). Addresses https://github.com/carbon-language/carbon-lang/pull/3217#discussion_r1323586836 --- toolchain/base/BUILD | 12 --- toolchain/driver/BUILD | 2 +- toolchain/driver/driver_test.cpp | 8 +- toolchain/lex/BUILD | 2 +- toolchain/lex/tokenized_buffer_test.cpp | 56 ++++++------ toolchain/parse/BUILD | 2 +- toolchain/parse/tree_test.cpp | 90 +++++++++---------- toolchain/sem_ir/BUILD | 2 +- toolchain/sem_ir/file_test.cpp | 5 +- toolchain/testing/BUILD | 25 ++++++ .../{base => testing}/yaml_test_helpers.cpp | 21 +++-- .../{base => testing}/yaml_test_helpers.h | 90 +++++++++++-------- toolchain/testing/yaml_test_helpers_test.cpp | 32 +++++++ 13 files changed, 204 insertions(+), 143 deletions(-) rename toolchain/{base => testing}/yaml_test_helpers.cpp (82%) rename toolchain/{base => testing}/yaml_test_helpers.h (57%) create mode 100644 toolchain/testing/yaml_test_helpers_test.cpp diff --git a/toolchain/base/BUILD b/toolchain/base/BUILD index 7d0df7c41095..89c2a4e59a0f 100644 --- a/toolchain/base/BUILD +++ b/toolchain/base/BUILD @@ -22,15 +22,3 @@ cc_library( "@llvm-project//llvm:Support", ], ) - -cc_library( - name = "yaml_test_helpers", - testonly = 1, - srcs = ["yaml_test_helpers.cpp"], - hdrs = ["yaml_test_helpers.h"], - deps = [ - "//common:ostream", - "@com_google_googletest//:gtest", - "@llvm-project//llvm:Support", - ], -) diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index a5e87e5880ae..8fda2ff2a03e 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -45,9 +45,9 @@ cc_test( ":driver", "//testing/base:gtest_main", "//testing/base:test_raw_ostream", - "//toolchain/base:yaml_test_helpers", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/lex:tokenized_buffer_test_helpers", + "//toolchain/testing:yaml_test_helpers", "@com_google_googletest//:gtest", "@llvm-project//llvm:Object", "@llvm-project//llvm:Support", diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index e681b694766d..7998322cebea 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -15,10 +15,12 @@ #include "llvm/Object/Binary.h" #include "llvm/Support/FormatVariadic.h" #include "testing/base/test_raw_ostream.h" +#include "toolchain/testing/yaml_test_helpers.h" namespace Carbon::Testing { namespace { +using ::testing::_; using ::testing::ContainsRegex; using ::testing::HasSubstr; using ::testing::StrEq; @@ -125,7 +127,8 @@ TEST_F(DriverTest, DumpTokens) { driver_.RunCommand({"compile", "--phase=lex", "--dump-tokens", file})); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); // Verify there is output without examining it. - EXPECT_FALSE(test_output_stream_.TakeStr().empty()); + EXPECT_THAT(Yaml::Value::FromText(test_output_stream_.TakeStr()), + Yaml::IsYaml(_)); } TEST_F(DriverTest, DumpParseTree) { @@ -134,7 +137,8 @@ TEST_F(DriverTest, DumpParseTree) { {"compile", "--phase=parse", "--dump-parse-tree", file})); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); // Verify there is output without examining it. - EXPECT_FALSE(test_output_stream_.TakeStr().empty()); + EXPECT_THAT(Yaml::Value::FromText(test_output_stream_.TakeStr()), + Yaml::IsYaml(_)); } TEST_F(DriverTest, StdoutOutput) { diff --git a/toolchain/lex/BUILD b/toolchain/lex/BUILD index 5125802e30f3..ca8e437a4c21 100644 --- a/toolchain/lex/BUILD +++ b/toolchain/lex/BUILD @@ -215,9 +215,9 @@ cc_test( ":tokenized_buffer_test_helpers", "//testing/base:gtest_main", "//testing/base:test_raw_ostream", - "//toolchain/base:yaml_test_helpers", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:mocks", + "//toolchain/testing:yaml_test_helpers", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", ], diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index e32e467b5ffc..3cac0caefa67 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -12,10 +12,10 @@ #include "llvm/ADT/ArrayRef.h" #include "testing/base/test_raw_ostream.h" -#include "toolchain/base/yaml_test_helpers.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/mocks.h" #include "toolchain/lex/tokenized_buffer_test_helpers.h" +#include "toolchain/testing/yaml_test_helpers.h" namespace Carbon::Testing { namespace { @@ -27,6 +27,7 @@ using ::testing::_; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::HasSubstr; +using ::testing::Pair; class LexerTest : public ::testing::Test { protected: @@ -1003,36 +1004,29 @@ TEST_F(LexerTest, PrintingAsYaml) { EXPECT_THAT( Yaml::Value::FromText(print_stream.TakeStr()), - ElementsAre(Yaml::SequenceValue{Yaml::MappingValue{ - {"filename", Yaml::ScalarValue{source_storage_.front().filename()}}, - {"tokens", Yaml::SequenceValue{ - Yaml::MappingValue{{"index", "0"}, - {"kind", "Semi"}, - {"line", "2"}, - {"column", "2"}, - {"indent", "2"}, - {"spelling", ";"}, - {"has_trailing_space", "true"}}, - Yaml::MappingValue{{"index", "1"}, - {"kind", "Semi"}, - {"line", "5"}, - {"column", "1"}, - {"indent", "1"}, - {"spelling", ";"}, - {"has_trailing_space", "true"}}, - Yaml::MappingValue{{"index", "2"}, - {"kind", "Semi"}, - {"line", "5"}, - {"column", "3"}, - {"indent", "1"}, - {"spelling", ";"}, - {"has_trailing_space", "true"}}, - Yaml::MappingValue{{"index", "3"}, - {"kind", "EndOfFile"}, - {"line", "15"}, - {"column", "1"}, - {"indent", "1"}, - {"spelling", ""}}}}}})); + IsYaml(ElementsAre(Yaml::Sequence(ElementsAre(Yaml::Mapping(ElementsAre( + Pair("filename", source_storage_.front().filename().str()), + Pair("tokens", + Yaml::Sequence(ElementsAre( + Yaml::Mapping(ElementsAre( + Pair("index", "0"), Pair("kind", "Semi"), + Pair("line", "2"), Pair("column", "2"), + Pair("indent", "2"), Pair("spelling", ";"), + Pair("has_trailing_space", "true"))), + Yaml::Mapping( + ElementsAre(Pair("index", "1"), Pair("kind", "Semi"), + Pair("line", "5"), Pair("column", "1"), + Pair("indent", "1"), Pair("spelling", ";"), + Pair("has_trailing_space", "true"))), + Yaml::Mapping( + ElementsAre(Pair("index", "2"), Pair("kind", "Semi"), + Pair("line", "5"), Pair("column", "3"), + Pair("indent", "1"), Pair("spelling", ";"), + Pair("has_trailing_space", "true"))), + Yaml::Mapping(ElementsAre( + Pair("index", "3"), Pair("kind", "EndOfFile"), + Pair("line", "15"), Pair("column", "1"), + Pair("indent", "1"), Pair("spelling", ""))))))))))))); } } // namespace diff --git a/toolchain/parse/BUILD b/toolchain/parse/BUILD index 056b2443decf..b5878eb48f3c 100644 --- a/toolchain/parse/BUILD +++ b/toolchain/parse/BUILD @@ -70,10 +70,10 @@ cc_test( "//common:ostream", "//testing/base:gtest_main", "//testing/base:test_raw_ostream", - "//toolchain/base:yaml_test_helpers", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:mocks", "//toolchain/lex:tokenized_buffer", + "//toolchain/testing:yaml_test_helpers", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", ], diff --git a/toolchain/parse/tree_test.cpp b/toolchain/parse/tree_test.cpp index 2dd5dcb03b2f..e3321e31a915 100644 --- a/toolchain/parse/tree_test.cpp +++ b/toolchain/parse/tree_test.cpp @@ -10,16 +10,17 @@ #include #include "testing/base/test_raw_ostream.h" -#include "toolchain/base/yaml_test_helpers.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/mocks.h" #include "toolchain/lex/tokenized_buffer.h" +#include "toolchain/testing/yaml_test_helpers.h" namespace Carbon::Testing { namespace { using Parse::Tree; using ::testing::ElementsAre; +using ::testing::Pair; class TreeTest : public ::testing::Test { protected: @@ -56,26 +57,23 @@ TEST_F(TreeTest, PrintPostorderAsYAML) { TestRawOstream print_stream; tree.Print(print_stream); - auto file = Yaml::SequenceValue{ - Yaml::MappingValue{{"kind", "FunctionIntroducer"}, {"text", "fn"}}, - Yaml::MappingValue{{"kind", "Name"}, {"text", "F"}}, - Yaml::MappingValue{{"kind", "ParameterListStart"}, {"text", "("}}, - Yaml::MappingValue{ - {"kind", "ParameterList"}, {"text", ")"}, {"subtree_size", "2"}}, - Yaml::MappingValue{{"kind", "FunctionDeclaration"}, - {"text", ";"}, - {"subtree_size", "5"}}, - Yaml::MappingValue{{"kind", "FileEnd"}, {"text", ""}}, - }; + auto file = Yaml::Sequence(ElementsAre( + Yaml::Mapping( + ElementsAre(Pair("kind", "FunctionIntroducer"), Pair("text", "fn"))), + Yaml::Mapping(ElementsAre(Pair("kind", "Name"), Pair("text", "F"))), + Yaml::Mapping( + ElementsAre(Pair("kind", "ParameterListStart"), Pair("text", "("))), + Yaml::Mapping(ElementsAre(Pair("kind", "ParameterList"), + Pair("text", ")"), Pair("subtree_size", "2"))), + Yaml::Mapping(ElementsAre(Pair("kind", "FunctionDeclaration"), + Pair("text", ";"), Pair("subtree_size", "5"))), + Yaml::Mapping(ElementsAre(Pair("kind", "FileEnd"), Pair("text", ""))))); - auto root = Yaml::SequenceValue{ - Yaml::MappingValue{ - {"filename", "test.carbon"}, - {"parse_tree", file}, - }, - }; + auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping( + ElementsAre(Pair("filename", "test.carbon"), Pair("parse_tree", file))))); - EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root)); + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), + IsYaml(ElementsAre(root))); } TEST_F(TreeTest, PrintPreorderAsYAML) { @@ -85,40 +83,34 @@ TEST_F(TreeTest, PrintPreorderAsYAML) { TestRawOstream print_stream; tree.Print(print_stream, /*preorder=*/true); - auto parameter_list = Yaml::SequenceValue{ - Yaml::MappingValue{ - {"node_index", "2"}, {"kind", "ParameterListStart"}, {"text", "("}}, - }; + auto parameter_list = Yaml::Sequence(ElementsAre(Yaml::Mapping( + ElementsAre(Pair("node_index", "2"), Pair("kind", "ParameterListStart"), + Pair("text", "("))))); - auto function_decl = Yaml::SequenceValue{ - Yaml::MappingValue{ - {"node_index", "0"}, {"kind", "FunctionIntroducer"}, {"text", "fn"}}, - Yaml::MappingValue{{"node_index", "1"}, {"kind", "Name"}, {"text", "F"}}, - Yaml::MappingValue{{"node_index", "3"}, - {"kind", "ParameterList"}, - {"text", ")"}, - {"subtree_size", "2"}, - {"children", parameter_list}}, - }; + auto function_decl = Yaml::Sequence(ElementsAre( + Yaml::Mapping(ElementsAre(Pair("node_index", "0"), + Pair("kind", "FunctionIntroducer"), + Pair("text", "fn"))), + Yaml::Mapping(ElementsAre(Pair("node_index", "1"), Pair("kind", "Name"), + Pair("text", "F"))), + Yaml::Mapping(ElementsAre(Pair("node_index", "3"), + Pair("kind", "ParameterList"), + Pair("text", ")"), Pair("subtree_size", "2"), + Pair("children", parameter_list))))); - auto file = Yaml::SequenceValue{ - Yaml::MappingValue{{"node_index", "4"}, - {"kind", "FunctionDeclaration"}, - {"text", ";"}, - {"subtree_size", "5"}, - {"children", function_decl}}, - Yaml::MappingValue{ - {"node_index", "5"}, {"kind", "FileEnd"}, {"text", ""}}, - }; + auto file = Yaml::Sequence(ElementsAre( + Yaml::Mapping(ElementsAre(Pair("node_index", "4"), + Pair("kind", "FunctionDeclaration"), + Pair("text", ";"), Pair("subtree_size", "5"), + Pair("children", function_decl))), + Yaml::Mapping(ElementsAre(Pair("node_index", "5"), + Pair("kind", "FileEnd"), Pair("text", ""))))); - auto root = Yaml::SequenceValue{ - Yaml::MappingValue{ - {"filename", "test.carbon"}, - {"parse_tree", file}, - }, - }; + auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping( + ElementsAre(Pair("filename", "test.carbon"), Pair("parse_tree", file))))); - EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root)); + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), + IsYaml(ElementsAre(root))); } TEST_F(TreeTest, HighRecursion) { diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 70915e66e742..efb517b8ca6c 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -74,8 +74,8 @@ cc_test( "//common:ostream", "//testing/base:gtest_main", "//testing/base:test_raw_ostream", - "//toolchain/base:yaml_test_helpers", "//toolchain/driver", + "//toolchain/testing:yaml_test_helpers", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", ], diff --git a/toolchain/sem_ir/file_test.cpp b/toolchain/sem_ir/file_test.cpp index a929d9e81291..793f0f4a337e 100644 --- a/toolchain/sem_ir/file_test.cpp +++ b/toolchain/sem_ir/file_test.cpp @@ -9,8 +9,8 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" #include "testing/base/test_raw_ostream.h" -#include "toolchain/base/yaml_test_helpers.h" #include "toolchain/driver/driver.h" +#include "toolchain/testing/yaml_test_helpers.h" namespace Carbon::Testing { namespace { @@ -72,7 +72,8 @@ TEST(SemIRTest, YAML) { auto root = Yaml::Sequence(ElementsAre(Yaml::Mapping( ElementsAre(Pair("filename", "test.carbon"), Pair("sem_ir", file))))); - EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(root)); + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), + IsYaml(ElementsAre(root))); } } // namespace diff --git a/toolchain/testing/BUILD b/toolchain/testing/BUILD index 091357d52463..ff1c0f6944b4 100644 --- a/toolchain/testing/BUILD +++ b/toolchain/testing/BUILD @@ -2,6 +2,7 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") load("//testing/file_test:rules.bzl", "file_test") package(default_visibility = ["//visibility:public"]) @@ -23,3 +24,27 @@ file_test( "@llvm-project//llvm:Support", ], ) + +cc_library( + name = "yaml_test_helpers", + testonly = 1, + srcs = ["yaml_test_helpers.cpp"], + hdrs = ["yaml_test_helpers.h"], + deps = [ + "//common:error", + "//common:ostream", + "@com_google_googletest//:gtest", + "@llvm-project//llvm:Support", + ], +) + +cc_test( + name = "yaml_test_helpers_test", + size = "small", + srcs = ["yaml_test_helpers_test.cpp"], + deps = [ + ":yaml_test_helpers", + "//testing/base:gtest_main", + "@com_google_googletest//:gtest", + ], +) diff --git a/toolchain/base/yaml_test_helpers.cpp b/toolchain/testing/yaml_test_helpers.cpp similarity index 82% rename from toolchain/base/yaml_test_helpers.cpp rename to toolchain/testing/yaml_test_helpers.cpp index 89c0ddfede21..fa7943a9f302 100644 --- a/toolchain/base/yaml_test_helpers.cpp +++ b/toolchain/testing/yaml_test_helpers.cpp @@ -2,7 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#include "toolchain/base/yaml_test_helpers.h" +#include "toolchain/testing/yaml_test_helpers.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/YAMLParser.h" @@ -10,9 +10,7 @@ namespace Carbon::Testing::Yaml { static auto Parse(llvm::yaml::Node* node) -> Value { - if (!node) { - return Value{ErrorValue()}; - } + CARBON_CHECK(node != nullptr); // getType returns an unsigned int which should map to the enum. switch (static_cast(node->getType())) { @@ -57,13 +55,25 @@ static auto Parse(llvm::yaml::Node* node) -> Value { llvm_unreachable("unknown yaml node kind"); } -auto Value::FromText(llvm::StringRef text) -> SequenceValue { +auto Value::FromText(llvm::StringRef text) -> ErrorOr { llvm::SourceMgr sm; + std::optional error_message; + sm.setDiagHandler( + [](const llvm::SMDiagnostic& diag, void* context) -> void { + auto* error_message = static_cast*>(context); + *error_message = std::string(); + llvm::raw_string_ostream stream(**error_message); + diag.print(/*ProgName=*/nullptr, stream, /*ShowColors=*/false); + }, + &error_message); llvm::yaml::Stream yaml_stream(text, sm); SequenceValue result; for (llvm::yaml::Document& document : yaml_stream) { result.push_back(Parse(document.getRoot())); + if (error_message) { + return Error(*error_message); + } } return result; } @@ -74,7 +84,6 @@ auto operator<<(std::ostream& os, const Value& v) -> std::ostream& { struct Printer { auto operator()(NullValue /*v*/) -> void { out << "Yaml::NullValue()"; } auto operator()(AliasValue /*v*/) -> void { out << "Yaml::AliasValue()"; } - auto operator()(ErrorValue /*v*/) -> void { out << "Yaml::ErrorValue()"; } auto operator()(const ScalarValue& v) -> void { out << std::quoted(v); } auto operator()(const MappingValue& v) -> void { out << "Yaml::MappingValue{"; diff --git a/toolchain/base/yaml_test_helpers.h b/toolchain/testing/yaml_test_helpers.h similarity index 57% rename from toolchain/base/yaml_test_helpers.h rename to toolchain/testing/yaml_test_helpers.h index 262b93241a4f..250b8715c199 100644 --- a/toolchain/base/yaml_test_helpers.h +++ b/toolchain/testing/yaml_test_helpers.h @@ -27,33 +27,33 @@ // // Exact values can be matched by constructing the desired value. // EXPECT_THAT( // yaml, -// ElementsAre( +// Yaml::IsYaml(ElementsAre( // Yaml::MappingValue{ // {"fruits", Yaml::SequenceValue{"apple", "orange", "pear"}}}, // Yaml::SequenceValue{Yaml::MappingValue{ // {Yaml::SequenceValue{Yaml::MappingValue{{"foo", "bar"}}}, -// "baz"}}})); +// "baz"}}}))); // // // Properties can be checked using Yaml::Mapping or Yaml::Sequence to // // adapt regular gmock container matchers. // EXPECT_THAT( // yaml, -// Contains(Yaml::Mapping( -// Contains(Pair("fruits", Yaml::Sequence(Contains("orange"))))))); +// Yaml::IsYaml(Contains(Yaml::Mapping( +// Contains(Pair("fruits", Yaml::Sequence(Contains("orange")))))))); // // On match failure, Yaml::Values are printed as C++ code that can be used to // recreate the value, for easy copy-pasting into test expectations. -#ifndef CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_ -#define CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_ +#ifndef CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_ +#define CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_ #include #include #include -#include #include +#include "common/error.h" #include "llvm/ADT/StringRef.h" namespace Carbon::Testing::Yaml { @@ -75,38 +75,50 @@ using ScalarValue = std::string; using MappingValue = std::vector>; using SequenceValue = std::vector; struct AliasValue : EmptyComparable {}; -struct ErrorValue : EmptyComparable {}; // A thin wrapper around a variant of possible YAML value types. This type // intentionally provides no additional encapsulation or invariants beyond // those of the variant. struct Value : std::variant { + AliasValue> { using variant::variant; // Prints the Value in the form of code to recreate the value. friend auto operator<<(std::ostream& os, const Value& v) -> std::ostream&; // Parses a sequence of YAML documents from the given YAML text. - static auto FromText(llvm::StringRef text) -> SequenceValue; + static auto FromText(llvm::StringRef text) -> ErrorOr; }; -template -auto DescribeMatcher(::testing::Matcher matcher) -> std::string { - std::ostringstream out; - matcher.DescribeTo(&out); - return out.str(); +// Used to examine the results of Value::FromText. +// NOLINTNEXTLINE: Expands from GoogleTest. +MATCHER_P(IsYaml, matcher, + "is yaml root sequence that " + + ::testing::DescribeMatcher(matcher)) { + const ErrorOr& yaml = arg; + const ::testing::Matcher& typed_matcher = matcher; + if (yaml.ok()) { + // It's hard to intercept printing of the ErrorOr value, so just print it + // here. + *result_listener << "\n which is: " << *yaml << "\n "; + return typed_matcher.MatchAndExplain(*yaml, result_listener); + } + + *result_listener << "\n with the error: " << yaml.error() << "\n "; + return false; } // Match a Value that is a MappingValue. -// Same as testing::VariantWith(contents). +// Similar to testing::VariantWith(matcher), but with better +// descriptions. // NOLINTNEXTLINE: Expands from GoogleTest. -MATCHER_P(Mapping, contents, - "is mapping that " + DescribeMatcher(contents)) { - ::testing::Matcher contents_matcher = contents; - - if (auto* map = std::get_if(&arg)) { - return contents_matcher.MatchAndExplain(*map, result_listener); +MATCHER_P(Mapping, matcher, + "is mapping that " + + ::testing::DescribeMatcher(matcher)) { + const Value& val = arg; + const ::testing::Matcher& typed_matcher = matcher; + if (const auto* map = std::get_if(&val)) { + return typed_matcher.MatchAndExplain(*map, result_listener); } *result_listener << "which is not a mapping"; @@ -114,14 +126,16 @@ MATCHER_P(Mapping, contents, } // Match a Value that is a SequenceValue. -// Same as testing::VariantWith(contents). +// Similar to testing::VariantWith(matcher), but with better +// descriptions. // NOLINTNEXTLINE: Expands from GoogleTest. -MATCHER_P(Sequence, contents, - "is mapping that " + DescribeMatcher(contents)) { - ::testing::Matcher contents_matcher = contents; - - if (auto* map = std::get_if(&arg)) { - return contents_matcher.MatchAndExplain(*map, result_listener); +MATCHER_P(Sequence, matcher, + "is sequence that " + + ::testing::DescribeMatcher(matcher)) { + const Value& val = arg; + const ::testing::Matcher& typed_matcher = matcher; + if (const auto* map = std::get_if(&val)) { + return typed_matcher.MatchAndExplain(*map, result_listener); } *result_listener << "which is not a sequence"; @@ -129,14 +143,16 @@ MATCHER_P(Sequence, contents, } // Match a Value that is a ScalarValue. -// Same as testing::VariantWith(contents). +// Similar to testing::VariantWith(matcher), but with better +// descriptions. // NOLINTNEXTLINE: Expands from GoogleTest. -MATCHER_P(Scalar, value, - "has scalar value " + DescribeMatcher(value)) { - ::testing::Matcher value_matcher = value; - - if (auto* map = std::get_if(&arg)) { - return value_matcher.MatchAndExplain(*map, result_listener); +MATCHER_P(Scalar, matcher, + "has scalar value " + + ::testing::DescribeMatcher(matcher)) { + const Value& val = arg; + const ::testing::Matcher& typed_matcher = matcher; + if (const auto* map = std::get_if(&val)) { + return typed_matcher.MatchAndExplain(*map, result_listener); } *result_listener << "which is not a scalar"; @@ -145,4 +161,4 @@ MATCHER_P(Scalar, value, } // namespace Carbon::Testing::Yaml -#endif // CARBON_TOOLCHAIN_BASE_YAML_TEST_HELPERS_H_ +#endif // CARBON_TOOLCHAIN_TESTING_YAML_TEST_HELPERS_H_ diff --git a/toolchain/testing/yaml_test_helpers_test.cpp b/toolchain/testing/yaml_test_helpers_test.cpp new file mode 100644 index 000000000000..4e704be651f9 --- /dev/null +++ b/toolchain/testing/yaml_test_helpers_test.cpp @@ -0,0 +1,32 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/testing/yaml_test_helpers.h" + +#include +#include + +namespace Carbon::Testing { +namespace { + +using ::testing::_; +using ::testing::ElementsAre; +using ::testing::Not; + +TEST(YamlTestHelpersTest, ValidYaml) { + EXPECT_THAT( + Yaml::Value::FromText("[foo, bar]"), + Yaml::IsYaml(ElementsAre(Yaml::Sequence(ElementsAre("foo", "bar"))))); +} + +TEST(YamlTestHelpersTest, InvalidYaml) { + auto result = Yaml::Value::FromText("- foo\nbar"); + // Make sure we've constructed invalid YAML. + EXPECT_FALSE(result.ok()); + // Make sure the matcher detects the invalid YAML. + EXPECT_THAT(result, Not(Yaml::IsYaml(_))); +} + +} // namespace +} // namespace Carbon::Testing