Merge pull request #178 from microsoft/user/corbinphipps/address-revi…

…ew-comments-2 Address review comments
microsoft · Feb 27, 2024 · ae27ad0 · ae27ad0
2 parents 1fb4563 + 2f73c40
commit ae27ad0
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 4 deletions.
diff --git a/protocol/protos/NetRemoteDataStream.proto b/protocol/protos/NetRemoteDataStream.proto
@@ -3,8 +3,6 @@ syntax = "proto3";
 
 package Microsoft.Net.Remote.DataStream;
 
-import "google/protobuf/any.proto";
-
 enum DataStreamOperationStatusCode
 {
     DataStreamOperationStatusCodeUnknown = 0;

diff --git a/tests/unit/TestNetRemoteDataStreamingReactors.cxx b/tests/unit/TestNetRemoteDataStreamingReactors.cxx
@@ -37,11 +37,18 @@ grpc::Status
 DataStreamWriter::Await(DataStreamUploadResult* result)
 {
     std::unique_lock lock(m_writeStatusGate);
-    static constexpr auto timeoutValue = std::chrono::seconds(10);
+    static constexpr auto timeoutValue = 10s;
 
-    m_writesDone.wait_for(lock, timeoutValue, [this] {
+    const auto isDone = m_writesDone.wait_for(lock, timeoutValue, [this] {
         return m_done;
     });
+
+    if (!isDone) {
+        DataStreamOperationStatus status{};
+        status.set_code(DataStreamOperationStatusCode::DataStreamOperationStatusCodeFailed);
+        status.set_message("Timeout occurred while waiting for all writes to be completed");
+        *m_result.mutable_status() = std::move(status);
+    }
     *result = m_result;
 
     return m_status;

diff --git a/tests/unit/TestNetRemoteDataStreamingServiceClient.cxx b/tests/unit/TestNetRemoteDataStreamingServiceClient.cxx
@@ -64,5 +64,9 @@ TEST_CASE("DataStreamUpload API", "[basic][rpc][client][remote][stream]")
                 REQUIRE(result.status().code() == DataStreamOperationStatusCodeSucceeded);
             });
         }
+
+        for (auto& clientThread : clientThreads) {
+            REQUIRE_NOTHROW(clientThread.join());
+        }
     }
 }