diff --git a/google/cloud/internal/resumable_streaming_read_rpc_test.cc b/google/cloud/internal/resumable_streaming_read_rpc_test.cc index 22d0fe7c9234b..839b6f3591167 100644 --- a/google/cloud/internal/resumable_streaming_read_rpc_test.cc +++ b/google/cloud/internal/resumable_streaming_read_rpc_test.cc @@ -42,9 +42,6 @@ struct FakeResponse { std::string token; }; -using MockReturnType = - std::unique_ptr>; - using ReadReturn = absl::variant; class MockStreamingReadRpc : public StreamingReadRpc { diff --git a/google/cloud/internal/streaming_read_rpc_test.cc b/google/cloud/internal/streaming_read_rpc_test.cc index 3db6a4dee54f8..405c52dcf95e1 100644 --- a/google/cloud/internal/streaming_read_rpc_test.cc +++ b/google/cloud/internal/streaming_read_rpc_test.cc @@ -38,9 +38,6 @@ struct FakeResponse { std::string value; }; -using MockReturnType = - std::unique_ptr>; - class MockReader : public grpc::ClientReaderInterface { public: MOCK_METHOD(bool, Read, (FakeResponse*), (override)); diff --git a/google/cloud/internal/streaming_write_rpc_test.cc b/google/cloud/internal/streaming_write_rpc_test.cc index 622f7f21bf57e..e06ca340ee88a 100644 --- a/google/cloud/internal/streaming_write_rpc_test.cc +++ b/google/cloud/internal/streaming_write_rpc_test.cc @@ -35,9 +35,6 @@ struct FakeResponse { std::string value; }; -using MockReturnType = - std::unique_ptr>; - class MockWriter : public grpc::ClientWriterInterface { public: MOCK_METHOD(bool, Write, (FakeRequest const&, grpc::WriteOptions), diff --git a/google/cloud/stream_range.h b/google/cloud/stream_range.h index 75bdab70f2e49..247ae9bb2c7d0 100644 --- a/google/cloud/stream_range.h +++ b/google/cloud/stream_range.h @@ -154,6 +154,11 @@ class StreamRange { */ StreamRange() = default; + ~StreamRange() { + internal::OptionsSpan span(options_); + reader_ = nullptr; + } + //@{ // @name Move-only StreamRange(StreamRange const&) = delete; @@ -226,8 +231,8 @@ class StreamRange { Next(); } - internal::StreamReader reader_; Options options_ = internal::CurrentOptions(); + internal::StreamReader reader_; StatusOr current_; bool current_ok_ = false; bool is_end_ = true; diff --git a/google/cloud/stream_range_test.cc b/google/cloud/stream_range_test.cc index 496f1e3294498..8978a0b3f9720 100644 --- a/google/cloud/stream_range_test.cc +++ b/google/cloud/stream_range_test.cc @@ -27,11 +27,22 @@ namespace { using ::google::cloud::testing_util::StatusIs; using ::testing::ElementsAre; +using ::testing::UnitTest; struct StringOption { using Type = std::string; }; +std::string CurrentTestName() { + return UnitTest::GetInstance()->current_test_info()->name(); +} + +template +StreamRange MakeStreamRange(internal::StreamReader reader) { + internal::OptionsSpan span(Options{}.set(CurrentTestName())); + return internal::MakeStreamRange(std::move(reader)); +} + TEST(StreamRange, DefaultConstructed) { StreamRange sr; auto it = sr.begin(); @@ -43,14 +54,14 @@ TEST(StreamRange, DefaultConstructed) { TEST(StreamRange, MoveOnly) { auto const reader = [] { return Status{}; }; - StreamRange sr = internal::MakeStreamRange(reader); - StreamRange move_construct = std::move(sr); - StreamRange move_assign = internal::MakeStreamRange(reader); + auto sr = MakeStreamRange(reader); + auto move_construct = std::move(sr); + auto move_assign = MakeStreamRange(reader); move_assign = std::move(move_construct); } TEST(StreamRange, EmptyRange) { - StreamRange sr = internal::MakeStreamRange([] { return Status{}; }); + auto sr = MakeStreamRange([] { return Status{}; }); auto it = sr.begin(); auto end = sr.end(); EXPECT_EQ(it, end); @@ -59,16 +70,16 @@ TEST(StreamRange, EmptyRange) { } TEST(StreamRange, OneElement) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); auto counter = 0; auto reader = [&counter]() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "OneElement"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); if (counter++ < 1) return 42; return Status{}; }; - internal::OptionsSpan span(Options{}.set("OneElement")); - StreamRange sr = internal::MakeStreamRange(std::move(reader)); - internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto sr = MakeStreamRange(std::move(reader)); auto it = sr.begin(); EXPECT_NE(it, sr.end()); EXPECT_TRUE(*it); @@ -78,14 +89,14 @@ TEST(StreamRange, OneElement) { } TEST(StreamRange, OneError) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); auto reader = []() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "OneError"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); return Status(StatusCode::kUnknown, "oops"); }; - internal::OptionsSpan span(Options{}.set("OneError")); - StreamRange sr = internal::MakeStreamRange(std::move(reader)); - internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto sr = MakeStreamRange(std::move(reader)); auto it = sr.begin(); EXPECT_NE(it, sr.end()); EXPECT_FALSE(*it); @@ -95,16 +106,16 @@ TEST(StreamRange, OneError) { } TEST(StreamRange, FiveElements) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); auto counter = 0; auto reader = [&counter]() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "FiveElements"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); if (counter++ < 5) return counter; return Status{}; }; - internal::OptionsSpan span(Options{}.set("FiveElements")); - StreamRange sr = internal::MakeStreamRange(std::move(reader)); - internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto sr = MakeStreamRange(std::move(reader)); std::vector v; for (StatusOr& x : sr) { EXPECT_TRUE(x); @@ -114,17 +125,16 @@ TEST(StreamRange, FiveElements) { } TEST(StreamRange, PostFixIteration) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); auto counter = 0; auto reader = [&counter]() -> internal::StreamReader::result_type { EXPECT_EQ(internal::CurrentOptions().get(), - "PostFixIteration"); + CurrentTestName()); if (counter++ < 5) return counter; return Status{}; }; - internal::OptionsSpan span(Options{}.set("PostFixIteration")); - StreamRange sr = internal::MakeStreamRange(std::move(reader)); - internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto sr = MakeStreamRange(std::move(reader)); std::vector v; // NOLINTNEXTLINE(modernize-loop-convert) for (auto it = sr.begin(); it != sr.end(); it++) { @@ -135,46 +145,46 @@ TEST(StreamRange, PostFixIteration) { } TEST(StreamRange, Distance) { + internal::OptionsSpan overlay1(Options{}.set("uh-oh")); + // Empty range - StreamRange sr = internal::MakeStreamRange([] { return Status{}; }); + auto sr = MakeStreamRange([] { return Status{}; }); EXPECT_EQ(0, std::distance(sr.begin(), sr.end())); // Range of one element auto counter = 0; - internal::OptionsSpan span1(Options{}.set("Distance1")); - StreamRange one = internal::MakeStreamRange( + auto one = MakeStreamRange( [&counter]() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "Distance1"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); if (counter++ < 1) return counter; return Status{}; }); - internal::OptionsSpan overlay1(Options{}.set("uh-oh")); EXPECT_EQ(1, std::distance(one.begin(), one.end())); // Range of five elements counter = 0; - internal::OptionsSpan span5(Options{}.set("Distance5")); - StreamRange five = internal::MakeStreamRange( + auto five = MakeStreamRange( [&counter]() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "Distance5"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); if (counter++ < 5) return counter; return Status{}; }); - internal::OptionsSpan overlay5(Options{}.set("uh-oh")); EXPECT_EQ(5, std::distance(five.begin(), five.end())); } TEST(StreamRange, StreamError) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); auto counter = 0; auto reader = [&counter]() -> internal::StreamReader::result_type { - EXPECT_EQ(internal::CurrentOptions().get(), "StreamError"); + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); if (counter++ < 2) return counter; return Status(StatusCode::kUnknown, "oops"); }; - internal::OptionsSpan span(Options{}.set("StreamError")); - StreamRange sr = internal::MakeStreamRange(std::move(reader)); - internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto sr = MakeStreamRange(std::move(reader)); auto it = sr.begin(); EXPECT_NE(it, sr.end()); @@ -197,6 +207,41 @@ TEST(StreamRange, StreamError) { EXPECT_EQ(it, sr.end()); } +template +class FakeResumableStreamingReadRpc { + public: + ~FakeResumableStreamingReadRpc() { + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); + EXPECT_TRUE(read_called_); + } + + typename internal::StreamReader::result_type Read() { + EXPECT_EQ(internal::CurrentOptions().get(), + CurrentTestName()); + EXPECT_FALSE(read_called_); + read_called_ = true; + return ResponseType{}; + } + + private: + bool read_called_ = false; +}; + +TEST(StreamRange, ReaderDestructorOptionsSpan) { + internal::OptionsSpan overlay(Options{}.set("uh-oh")); + auto reader = [] { + auto resumable = std::make_shared>(); + return [resumable]() -> internal::StreamReader::result_type { + return resumable->Read(); + }; + }(); + auto sr = MakeStreamRange(std::move(reader)); + // `~StreamRange()` will now delete the reader, which will drop the last + // reference to the `FakeResumableStreamingReadRpc`, and all of that should + // happen with `CurrentOptions()` matching those at `StreamRange` ctor time. +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace cloud