From 5f0531dcdaa2d45749cc5b893ce60763737c09c1 Mon Sep 17 00:00:00 2001 From: Daniel Alm Date: Mon, 18 Jun 2018 18:34:48 +0200 Subject: [PATCH] Code review fixes. --- Tests/SwiftGRPCTests/MetadataTests.swift | 4 +- .../SwiftGRPCTests/ServerThrowingTests.swift | 46 +++++++++++-------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/Tests/SwiftGRPCTests/MetadataTests.swift b/Tests/SwiftGRPCTests/MetadataTests.swift index 48af84b42..7db8a5a4d 100644 --- 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 index 7537bc98f..b5ccb69fa 100644 --- 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() {