From d18c1347d7711d04c53b578ba440acf33e09bf85 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 14 Jun 2023 09:31:56 -0700 Subject: [PATCH] Migrate compatible uses to TestRawOstream. (#2891) Replacing direct raw_string_ostream uses. I figure the wrapper should be used more consistently. There are still remaining raw_string_ostream uses that weren't compatible -- I'm continuing to look at those, but felt it was cleaner to have this on its own. --- common/BUILD | 3 +++ common/enum_base_test.cpp | 10 +++++++--- common/error_test.cpp | 19 +++++++++++-------- common/vlog_test.cpp | 13 +++++++------ explorer/BUILD | 1 + explorer/common/BUILD | 1 + explorer/common/error_builders_test.cpp | 13 +++++-------- explorer/file_test.cpp | 6 +++--- explorer/fuzzing/BUILD | 1 + explorer/fuzzing/ast_to_proto_test.cpp | 6 +++--- testing/file_test/BUILD | 1 + testing/file_test/file_test_base.cpp | 13 ++++++------- toolchain/driver/BUILD | 1 + toolchain/driver/driver_fuzzer.cpp | 7 +++---- toolchain/lexer/BUILD | 1 + toolchain/lexer/tokenized_buffer_test.cpp | 7 +++---- toolchain/parser/BUILD | 1 + toolchain/parser/parse_tree_test.cpp | 13 +++++-------- toolchain/semantics/BUILD | 1 + toolchain/semantics/semantics_ir_test.cpp | 6 +++--- 20 files changed, 67 insertions(+), 57 deletions(-) diff --git a/common/BUILD b/common/BUILD index 05ddac481212..bcda208fa0e4 100644 --- a/common/BUILD +++ b/common/BUILD @@ -55,6 +55,7 @@ cc_test( ":enum_base", ":enum_base_test_def", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", ], ) @@ -75,6 +76,7 @@ cc_test( deps = [ ":error", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", ], ) @@ -156,6 +158,7 @@ cc_test( deps = [ ":vlog", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", ], ) diff --git a/common/enum_base_test.cpp b/common/enum_base_test.cpp index cf168f212f1e..5e1911035ca4 100644 --- a/common/enum_base_test.cpp +++ b/common/enum_base_test.cpp @@ -6,8 +6,11 @@ #include +#include "testing/util/test_raw_ostream.h" + namespace Carbon { +// These are directly in the Carbon namespace because the defines require it. CARBON_DEFINE_RAW_ENUM_CLASS(TestKind, uint8_t) { #define CARBON_ENUM_BASE_TEST_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name) #include "common/enum_base_test.def" @@ -31,6 +34,7 @@ CARBON_DEFINE_ENUM_CLASS_NAMES(TestKind) = { #include "common/enum_base_test.def" }; +namespace Testing { namespace { TEST(EnumBaseTest, NamesAndConstants) { @@ -40,8 +44,7 @@ TEST(EnumBaseTest, NamesAndConstants) { } TEST(EnumBaseTest, Printing) { - std::string s; - llvm::raw_string_ostream stream(s); + TestRawOstream stream; TestKind kind = TestKind::Beep; stream << kind << " " << TestKind::Beep; @@ -50,7 +53,7 @@ TEST(EnumBaseTest, Printing) { // Check the streamed results and also make sure we can stream into GoogleTest // assertions. - EXPECT_EQ("Beep Beep Boop", s) << "Final kind: " << kind; + EXPECT_EQ("Beep Beep Boop", stream.TakeStr()) << "Final kind: " << kind; } TEST(EnumBaseTest, Switch) { @@ -105,4 +108,5 @@ TEST(EnumBaseTest, IntConversion) { } } // namespace +} // namespace Testing } // namespace Carbon diff --git a/common/error_test.cpp b/common/error_test.cpp index 2a08ed213735..1898a4b5c638 100644 --- a/common/error_test.cpp +++ b/common/error_test.cpp @@ -6,6 +6,8 @@ #include +#include "testing/util/test_raw_ostream.h" + namespace Carbon::Testing { namespace { @@ -98,15 +100,16 @@ TEST(ErrorTest, AssignOrReturnHasErrorInExpected) { } TEST(ErrorTest, ErrorBuilderOperatorImplicitCast) { - ErrorOr result1 = ErrorBuilder() << "msg"; - ASSERT_FALSE(result1.ok()); - EXPECT_EQ(result1.error().message(), "msg"); + ErrorOr result = ErrorBuilder() << "msg"; + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.error().message(), "msg"); +} - auto result2 = static_cast(ErrorBuilder("TestFunc") << "msg"); - std::string result2_output; - llvm::raw_string_ostream oss(result2_output); - result2.Print(oss); - EXPECT_EQ(oss.str(), "TestFunc: msg"); +TEST(ErrorTest, StreamError) { + Error result = ErrorBuilder("TestFunc") << "msg"; + TestRawOstream result_stream; + result_stream << result; + EXPECT_EQ(result_stream.TakeStr(), "TestFunc: msg"); } } // namespace diff --git a/common/vlog_test.cpp b/common/vlog_test.cpp index 77f4cd2a711e..d207ec41fc86 100644 --- a/common/vlog_test.cpp +++ b/common/vlog_test.cpp @@ -7,6 +7,8 @@ #include #include +#include "testing/util/test_raw_ostream.h" + namespace Carbon::Testing { namespace { @@ -16,7 +18,7 @@ using ::testing::StrEq; // Helper class with a vlog_stream_ member for CARBON_VLOG. class VLogger { public: - explicit VLogger(bool enable) : buffer_(buffer_str_) { + explicit VLogger(bool enable) { if (enable) { vlog_stream_ = &buffer_; } @@ -24,11 +26,10 @@ class VLogger { void VLog() { CARBON_VLOG() << "Test\n"; } - auto buffer() -> llvm::StringRef { return buffer_str_; } + auto TakeStr() -> std::string { return buffer_.TakeStr(); } private: - std::string buffer_str_; - llvm::raw_string_ostream buffer_; + TestRawOstream buffer_; llvm::raw_ostream* vlog_stream_ = nullptr; }; @@ -36,13 +37,13 @@ class VLogger { TEST(VLogTest, Enabled) { VLogger vlog(/*enable=*/true); vlog.VLog(); - EXPECT_THAT(vlog.buffer(), StrEq("Test\n")); + EXPECT_THAT(vlog.TakeStr(), StrEq("Test\n")); } TEST(VLogTest, Disabled) { VLogger vlog(/*enable=*/false); vlog.VLog(); - EXPECT_THAT(vlog.buffer(), IsEmpty()); + EXPECT_THAT(vlog.TakeStr(), IsEmpty()); } } // namespace diff --git a/explorer/BUILD b/explorer/BUILD index 4576f018a764..918de0eee9a6 100644 --- a/explorer/BUILD +++ b/explorer/BUILD @@ -50,6 +50,7 @@ file_test( deps = [ "//explorer/parse_and_execute", "//testing/file_test:file_test_base", + "//testing/util:test_raw_ostream", ], ) diff --git a/explorer/common/BUILD b/explorer/common/BUILD index 1662030f8a42..a897b3bd9ff0 100644 --- a/explorer/common/BUILD +++ b/explorer/common/BUILD @@ -28,6 +28,7 @@ cc_test( ":error_builders", ":source_location", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", ], ) diff --git a/explorer/common/error_builders_test.cpp b/explorer/common/error_builders_test.cpp index 4cf285543fad..e52574d4531b 100644 --- a/explorer/common/error_builders_test.cpp +++ b/explorer/common/error_builders_test.cpp @@ -7,22 +7,19 @@ #include #include "explorer/common/source_location.h" +#include "testing/util/test_raw_ostream.h" namespace Carbon::Testing { namespace { -auto ToString(const Error& err) -> std::string { - std::string result; - llvm::raw_string_ostream out(result); - err.Print(out); - return result; -} - TEST(ErrorBuildersTest, ProgramError) { Error err = ProgramError(SourceLocation("x", 1)) << "test"; EXPECT_EQ(err.location(), "x:1"); EXPECT_EQ(err.message(), "test"); - EXPECT_EQ(ToString(err), "x:1: test"); + + TestRawOstream out; + out << err; + EXPECT_EQ(out.TakeStr(), "x:1: test"); } } // namespace diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index ddd7cc17833f..825b3c7433fd 100644 --- a/explorer/file_test.cpp +++ b/explorer/file_test.cpp @@ -4,6 +4,7 @@ #include "explorer/parse_and_execute/parse_and_execute.h" #include "testing/file_test/file_test_base.h" +#include "testing/util/test_raw_ostream.h" namespace Carbon::Testing { namespace { @@ -43,8 +44,7 @@ class ParseAndExecuteTestFile : public FileTestBase { // Capture trace streaming, but only when in debug mode. TraceStream trace_stream; - std::string trace_stream_str; - llvm::raw_string_ostream trace_stream_ostream(trace_stream_str); + TestRawOstream trace_stream_ostream; if (trace_) { trace_stream.set_stream(&trace_stream_ostream); trace_stream.set_allowed_phases({ProgramPhase::All}); @@ -69,7 +69,7 @@ class ParseAndExecuteTestFile : public FileTestBase { } if (trace_) { - EXPECT_FALSE(trace_stream_str.empty()) + EXPECT_FALSE(trace_stream_ostream.TakeStr().empty()) << "Tracing should always do something"; } diff --git a/explorer/fuzzing/BUILD b/explorer/fuzzing/BUILD index fb3dbf7f9f61..e08f1a6f3a83 100644 --- a/explorer/fuzzing/BUILD +++ b/explorer/fuzzing/BUILD @@ -48,6 +48,7 @@ cc_test( "//common/fuzzing:carbon_cc_proto", "//common/fuzzing:proto_to_carbon_lib", "//explorer/syntax", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", "@com_google_protobuf//:protobuf_headers", ], diff --git a/explorer/fuzzing/ast_to_proto_test.cpp b/explorer/fuzzing/ast_to_proto_test.cpp index 80fa17f097df..d06dd9f6b6b0 100644 --- a/explorer/fuzzing/ast_to_proto_test.cpp +++ b/explorer/fuzzing/ast_to_proto_test.cpp @@ -16,6 +16,7 @@ #include "common/fuzzing/proto_to_carbon.h" #include "explorer/syntax/parse.h" +#include "testing/util/test_raw_ostream.h" namespace Carbon::Testing { namespace { @@ -29,14 +30,13 @@ static std::vector* carbon_files = nullptr; // Returns a string representation of `ast`. auto AstToString(const AST& ast) -> std::string { - std::string s; - llvm::raw_string_ostream out(s); + TestRawOstream out; out << "package " << ast.package.package << (ast.is_api ? "api" : "impl") << ";\n"; for (auto* declaration : ast.declarations) { out << *declaration << "\n"; } - return s; + return out.TakeStr(); } // Concatenates message and field names. diff --git a/testing/file_test/BUILD b/testing/file_test/BUILD index 1dfa3eb89127..2ac22ff1601e 100644 --- a/testing/file_test/BUILD +++ b/testing/file_test/BUILD @@ -13,6 +13,7 @@ cc_library( hdrs = ["file_test_base.h"], deps = [ "//common:check", + "//testing/util:test_raw_ostream", "@com_google_googletest//:gtest", "@llvm-project//llvm:Support", ], diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index 1c722aedbeaa..ca7ae749639c 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -10,6 +10,7 @@ #include "common/check.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/InitLLVM.h" +#include "testing/util/test_raw_ostream.h" namespace Carbon::Testing { @@ -78,11 +79,9 @@ auto FileTestBase::TestBody() -> void { } // Capture trace streaming, but only when in debug mode. - std::string stdout; - std::string stderr; - llvm::raw_string_ostream stdout_ostream(stdout); - llvm::raw_string_ostream stderr_ostream(stderr); - bool run_succeeded = RunWithFiles(test_files, stdout_ostream, stderr_ostream); + TestRawOstream stdout; + TestRawOstream stderr; + bool run_succeeded = RunWithFiles(test_files, stdout, stderr); if (HasFailure()) { return; } @@ -92,8 +91,8 @@ auto FileTestBase::TestBody() -> void { "is expected to fail."; // Check results. - EXPECT_THAT(SplitOutput(stdout), ElementsAreArray(expected_stdout)); - EXPECT_THAT(SplitOutput(stderr), ElementsAreArray(expected_stderr)); + EXPECT_THAT(SplitOutput(stdout.TakeStr()), ElementsAreArray(expected_stdout)); + EXPECT_THAT(SplitOutput(stderr.TakeStr()), ElementsAreArray(expected_stderr)); } auto FileTestBase::ProcessTestFile( diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 1d89fa65e1a0..865cae7b8ec9 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -49,6 +49,7 @@ cc_fuzz_test( corpus = glob(["fuzzer_corpus/*"]), deps = [ ":driver", + "//testing/util:test_raw_ostream", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/driver/driver_fuzzer.cpp b/toolchain/driver/driver_fuzzer.cpp index 610d4b8e6262..73d944d2f8f1 100644 --- a/toolchain/driver/driver_fuzzer.cpp +++ b/toolchain/driver/driver_fuzzer.cpp @@ -10,6 +10,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" +#include "testing/util/test_raw_ostream.h" #include "toolchain/driver/driver.h" namespace Carbon::Testing { @@ -67,12 +68,10 @@ extern "C" auto LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) } llvm::vfs::InMemoryFileSystem fs; - std::string error_text; - llvm::raw_string_ostream error_stream(error_text); + TestRawOstream error_stream; Driver d(fs, llvm::nulls(), error_stream); if (!d.RunFullCommand(args)) { - error_stream.flush(); - if (error_text.find("ERROR:") == std::string::npos) { + if (error_stream.TakeStr().find("ERROR:") == std::string::npos) { llvm::errs() << "No error message on a failure!\n"; return 1; } diff --git a/toolchain/lexer/BUILD b/toolchain/lexer/BUILD index e2ccd82ae1f7..24b106237c00 100644 --- a/toolchain/lexer/BUILD +++ b/toolchain/lexer/BUILD @@ -209,6 +209,7 @@ cc_test( ":tokenized_buffer", ":tokenized_buffer_test_helpers", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "//toolchain/common:yaml_test_helpers", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:mocks", diff --git a/toolchain/lexer/tokenized_buffer_test.cpp b/toolchain/lexer/tokenized_buffer_test.cpp index 50ed849d8aeb..a8004aad3ee0 100644 --- a/toolchain/lexer/tokenized_buffer_test.cpp +++ b/toolchain/lexer/tokenized_buffer_test.cpp @@ -14,6 +14,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/raw_ostream.h" +#include "testing/util/test_raw_ostream.h" #include "toolchain/common/yaml_test_helpers.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/mocks.h" @@ -1103,12 +1104,10 @@ TEST_F(LexerTest, PrintingAsYaml) { // Test that we can parse this into YAML and verify line and indent data. auto buffer = Lex("\n ;\n\n\n; ;\n\n\n\n\n\n\n\n\n\n\n"); ASSERT_FALSE(buffer.has_errors()); - std::string print_output; - llvm::raw_string_ostream print_stream(print_output); + TestRawOstream print_stream; buffer.Print(print_stream); - print_stream.flush(); - EXPECT_THAT(Yaml::Value::FromText(print_output), + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(Yaml::SequenceValue{ Yaml::MappingValue{{"index", "0"}, {"kind", "Semi"}, diff --git a/toolchain/parser/BUILD b/toolchain/parser/BUILD index c55995813b4f..dddbec1eb3c6 100644 --- a/toolchain/parser/BUILD +++ b/toolchain/parser/BUILD @@ -63,6 +63,7 @@ cc_test( ":parse_tree", "//common:ostream", "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "//toolchain/common:yaml_test_helpers", "//toolchain/diagnostics:diagnostic_emitter", "//toolchain/diagnostics:mocks", diff --git a/toolchain/parser/parse_tree_test.cpp b/toolchain/parser/parse_tree_test.cpp index 65ec2193e23b..388d506ee59b 100644 --- a/toolchain/parser/parse_tree_test.cpp +++ b/toolchain/parser/parse_tree_test.cpp @@ -9,6 +9,7 @@ #include +#include "testing/util/test_raw_ostream.h" #include "toolchain/common/yaml_test_helpers.h" #include "toolchain/diagnostics/diagnostic_emitter.h" #include "toolchain/diagnostics/mocks.h" @@ -51,10 +52,8 @@ TEST_F(ParseTreeTest, PrintPostorderAsYAML) { TokenizedBuffer tokens = GetTokenizedBuffer("fn F();"); ParseTree tree = ParseTree::Parse(tokens, consumer, /*vlog_stream=*/nullptr); EXPECT_FALSE(tree.has_errors()); - std::string print_output; - llvm::raw_string_ostream print_stream(print_output); + TestRawOstream print_stream; tree.Print(print_stream); - print_stream.flush(); auto file = Yaml::SequenceValue{ Yaml::MappingValue{{"kind", "FunctionIntroducer"}, {"text", "fn"}}, @@ -68,17 +67,15 @@ TEST_F(ParseTreeTest, PrintPostorderAsYAML) { Yaml::MappingValue{{"kind", "FileEnd"}, {"text", ""}}, }; - EXPECT_THAT(Yaml::Value::FromText(print_output), ElementsAre(file)); + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(file)); } TEST_F(ParseTreeTest, PrintPreorderAsYAML) { TokenizedBuffer tokens = GetTokenizedBuffer("fn F();"); ParseTree tree = ParseTree::Parse(tokens, consumer, /*vlog_stream=*/nullptr); EXPECT_FALSE(tree.has_errors()); - std::string print_output; - llvm::raw_string_ostream print_stream(print_output); + TestRawOstream print_stream; tree.Print(print_stream, /*preorder=*/true); - print_stream.flush(); auto parameter_list = Yaml::SequenceValue{ Yaml::MappingValue{ @@ -107,7 +104,7 @@ TEST_F(ParseTreeTest, PrintPreorderAsYAML) { {"node_index", "5"}, {"kind", "FileEnd"}, {"text", ""}}, }; - EXPECT_THAT(Yaml::Value::FromText(print_output), ElementsAre(file)); + EXPECT_THAT(Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(file)); } TEST_F(ParseTreeTest, HighRecursion) { diff --git a/toolchain/semantics/BUILD b/toolchain/semantics/BUILD index 93dfac865cf5..c649ebfbfb77 100644 --- a/toolchain/semantics/BUILD +++ b/toolchain/semantics/BUILD @@ -105,6 +105,7 @@ cc_test( srcs = ["semantics_ir_test.cpp"], deps = [ "//testing/util:gtest_main", + "//testing/util:test_raw_ostream", "//toolchain/common:yaml_test_helpers", "//toolchain/driver", "@com_google_googletest//:gtest", diff --git a/toolchain/semantics/semantics_ir_test.cpp b/toolchain/semantics/semantics_ir_test.cpp index ed49450b1a67..cb7596b9a813 100644 --- a/toolchain/semantics/semantics_ir_test.cpp +++ b/toolchain/semantics/semantics_ir_test.cpp @@ -10,6 +10,7 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" +#include "testing/util/test_raw_ostream.h" #include "toolchain/common/yaml_test_helpers.h" #include "toolchain/driver/driver.h" @@ -29,8 +30,7 @@ TEST(SemanticsIRTest, YAML) { llvm::vfs::InMemoryFileSystem fs; CARBON_CHECK(fs.addFile("test.carbon", /*ModificationTime=*/0, llvm::MemoryBuffer::getMemBuffer("var x: i32 = 0;"))); - std::string print_output; - llvm::raw_string_ostream print_stream(print_output); + TestRawOstream print_stream; Driver d(fs, print_stream, llvm::errs()); d.RunFullCommand({"dump", "semantics-ir", "test.carbon"}); @@ -41,7 +41,7 @@ TEST(SemanticsIRTest, YAML) { auto type_id = Yaml::Scalar(MatchesRegex(R"(type\d+)")); EXPECT_THAT( - Yaml::Value::FromText(print_output), + Yaml::Value::FromText(print_stream.TakeStr()), ElementsAre(Yaml::Mapping(ElementsAre( Pair("cross_reference_irs_size", "1"), Pair("functions", Yaml::Sequence(IsEmpty())),