Code review fixes.

grpc · Jun 18, 2018 · 5f0531d · 5f0531d
1 parent 3130eea
commit 5f0531d
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 20 deletions.
diff --git a/Tests/SwiftGRPCTests/MetadataTests.swift b/Tests/SwiftGRPCTests/MetadataTests.swift
@@ -20,11 +20,11 @@ import XCTest
 class MetadataTests: XCTestCase {
   static var allTests: [(String, (MetadataTests) -> () throws -> Void)] {
     return [
-      ("testCopy", testCopy)
+      ("testCopyDoesNotCorruptData", testCopyDoesNotCorruptData)
     ]
   }
 
-  func testCopy() {
+  func testCopyDoesNotCorruptData() {
     let metadata = try! Metadata(["foo": "bar"])
     XCTAssertEqual(["foo": "bar"], metadata.copy().dictionaryRepresentation)
   }

diff --git a/Tests/SwiftGRPCTests/ServerThrowingTests.swift b/Tests/SwiftGRPCTests/ServerThrowingTests.swift
@@ -44,6 +44,7 @@ class ServerThrowingTests: BasicEchoTestCase {
   static var allTests: [(String, (ServerThrowingTests) -> () throws -> Void)] {
     return [
       ("testServerThrowsUnary", testServerThrowsUnary),
+      ("testServerThrowsUnary_noTrailingMetadataCorruption", testServerThrowsUnary_noTrailingMetadataCorruptionAfterOriginalTrailingMetadataGetsReleased),
       ("testServerThrowsClientStreaming", testServerThrowsClientStreaming),
       ("testServerThrowsServerStreaming", testServerThrowsServerStreaming),
       ("testServerThrowsBidirectionalStreaming", testServerThrowsBidirectionalStreaming)
@@ -54,29 +55,38 @@ class ServerThrowingTests: BasicEchoTestCase {
 }
 
 extension ServerThrowingTests {
-  func testServerThrowsUnary() {
-    let caughtError: RPCError
+  func testServerThrowsUnary() throws {
     do {
       let result = try client.get(Echo_EchoRequest(text: "foo")).text
       XCTFail("should have thrown, received \(result) instead")
       return
-    } catch {
-      caughtError = error as! RPCError
+    } catch let error as RPCError {
+      guard case let .callError(callResult) = error
+        else { XCTFail("unexpected error \(error)"); return }
+
+      XCTAssertEqual(.permissionDenied, callResult.statusCode)
+      XCTAssertEqual("custom status message", callResult.statusMessage)
+      XCTAssertEqual(["some_long_key": "bar"], callResult.trailingMetadata?.dictionaryRepresentation)
+    }
+  }
+
+  func testServerThrowsUnary_noTrailingMetadataCorruptionAfterOriginalTrailingMetadataGetsReleased() throws {
+    do {
+      let result = try client.get(Echo_EchoRequest(text: "foo")).text
+      XCTFail("should have thrown, received \(result) instead")
+      return
+    } catch let error as RPCError {
+      guard case let .callError(callResult) = error
+        else { XCTFail("unexpected error \(error)"); return }
+
+      // Send another RPC to cause the original metadata array to be deallocated.
+      // If we were not copying the metadata array correctly, this would cause the metadata of the call result to become
+      // corrupted.
+      _ = try? client.get(Echo_EchoRequest(text: "foo")).text
+      // It seems like we need this sleep for the memory corruption to occur.
+      Thread.sleep(forTimeInterval: 0.01)
+      XCTAssertEqual(["some_long_key": "bar"], callResult.trailingMetadata?.dictionaryRepresentation)
     }
-
-    guard case let .callError(callResult) = caughtError
-      else { XCTFail("unexpected error \(caughtError)"); return }
-
-    // Send another RPC to cause the original metadata array to be deallocated.
-    // If we were not copying the metadata array correctly, this would cause the metadata of the call result to become
-    // corrupted.
-    _ = try? client.get(Echo_EchoRequest(text: "foo")).text
-    // It seems like we need this sleep for the memory corruption to occur.
-    Thread.sleep(forTimeInterval: 0.01)
-    XCTAssertEqual(["some_long_key": "bar"], callResult.trailingMetadata?.dictionaryRepresentation)
-
-    XCTAssertEqual(.permissionDenied, callResult.statusCode)
-    XCTAssertEqual("custom status message", callResult.statusMessage)
   }
 
   func testServerThrowsClientStreaming() {