From 19403414baa3f1c19a880154c81cbdeda9668acd Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Fri, 17 Sep 2021 20:50:29 +0100 Subject: [PATCH] Get unit tests running again on nightly builds. (#1961) Motivation: In February, @weissi noticed that the nightly builds of Swift could no longer compile the unit tests. We disabled them at that time and reported an upstream bug, https://bugs.swift.org/browse/SR-14268. This is not an ideal situation, but the Swift core team has provided a workaround we can use to re-enable the tests. Modifications: Provide free functions that wrap some specific extension methods that are called in unit tests, and amend the call sites to use them. Result: We can re-enable the nightly tests. --- Sources/NIOCore/SocketAddresses.swift | 23 +++++++++++++++++++++ Sources/NIOPosix/BaseSocket.swift | 22 ++++++++++++++++++++ Tests/NIOPosixTests/SocketAddressTest.swift | 14 ++++++------- Tests/NIOPosixTests/TestUtils.swift | 4 ++-- docker/docker-compose.2004.main.yaml | 1 - 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/Sources/NIOCore/SocketAddresses.swift b/Sources/NIOCore/SocketAddresses.swift index 76c3cda9..b6be1c53 100644 --- a/Sources/NIOCore/SocketAddresses.swift +++ b/Sources/NIOCore/SocketAddresses.swift @@ -638,3 +638,26 @@ extension sockaddr_storage: SockAddrProtocol { } } +// MARK: Workarounds for SR-14268 +// We need these free functions to expose our extension methods, because otherwise +// the compiler falls over when we try to access them from test code. As these functions +// exist purely to make the behaviours accessible from test code, we name them truly awfully. +func __testOnly_addressDescription(_ addr: inout sockaddr_in) -> String { + return addr.addressDescription() +} + +func __testOnly_addressDescription(_ addr: inout sockaddr_in6) -> String { + return addr.addressDescription() +} + +func __testOnly_withSockAddr( + _ addr: inout sockaddr_in, _ body: (UnsafePointer, Int) throws -> ReturnType +) rethrows -> ReturnType { + return try addr.withSockAddr(body) +} + +func __testOnly_withSockAddr( + _ addr: inout sockaddr_in6, _ body: (UnsafePointer, Int) throws -> ReturnType +) rethrows -> ReturnType { + return try addr.withSockAddr(body) +} diff --git a/Sources/NIOPosix/BaseSocket.swift b/Sources/NIOPosix/BaseSocket.swift index ae4c34ac..7f85ee7c 100644 --- a/Sources/NIOPosix/BaseSocket.swift +++ b/Sources/NIOPosix/BaseSocket.swift @@ -411,3 +411,25 @@ extension BaseSocket: CustomStringConvertible { return "BaseSocket { fd=\(self.descriptor) }" } } + +// MARK: Workarounds for SR-14268 +// We need these free functions to expose our extension methods, because otherwise +// the compiler falls over when we try to access them from test code. As these functions +// exist purely to make the behaviours accessible from test code, we name them truly awfully. +func __testOnly_convertSockAddr(_ addr: inout sockaddr_storage) -> sockaddr_in { + return addr.convert() +} + +func __testOnly_convertSockAddr(_ addr: inout sockaddr_storage) -> sockaddr_in6 { + return addr.convert() +} + +func __testOnly_convertSockAddr(_ addr: inout sockaddr_storage) -> sockaddr_un { + return addr.convert() +} + +func __testOnly_withMutableSockAddr( + _ addr: inout sockaddr_storage, _ body: (UnsafeMutablePointer, Int) throws -> ReturnType +) rethrows -> ReturnType { + return try addr.withMutableSockAddr(body) +} diff --git a/Tests/NIOPosixTests/SocketAddressTest.swift b/Tests/NIOPosixTests/SocketAddressTest.swift index 8a4e5bb1..94d0dd04 100644 --- a/Tests/NIOPosixTests/SocketAddressTest.swift +++ b/Tests/NIOPosixTests/SocketAddressTest.swift @@ -87,7 +87,7 @@ class SocketAddressTest: XCTestCase { $0.baseAddress!.bindMemory(to: in6_addr.self, capacity: 1).pointee } - let s = address.addressDescription() + let s = __testOnly_addressDescription(&address) XCTAssertEqual(s.count, sampleString.count, "Address description has unexpected length 😱") XCTAssertEqual(s, sampleString, @@ -183,19 +183,19 @@ class SocketAddressTest: XCTestCase { _ = withUnsafeMutableBytes(of: &storage) { temp in memcpy(temp.baseAddress!, outer.baseAddress!, MemoryLayout.size) } - return storage.convert() + return __testOnly_convertSockAddr(&storage) } var secondCopy: sockaddr_in6 = withUnsafeBytes(of: &secondIPAddress) { outer in _ = withUnsafeMutableBytes(of: &storage) { temp in memcpy(temp.baseAddress!, outer.baseAddress!, MemoryLayout.size) } - return storage.convert() + return __testOnly_convertSockAddr(&storage) } var thirdCopy: sockaddr_un = withUnsafeBytes(of: &thirdIPAddress) { outer in _ = withUnsafeMutableBytes(of: &storage) { temp in memcpy(temp.baseAddress!, outer.baseAddress!, MemoryLayout.size) } - return storage.convert() + return __testOnly_convertSockAddr(&storage) } XCTAssertEqual(memcmp(&firstIPAddress, &firstCopy, MemoryLayout.size), 0) @@ -226,14 +226,14 @@ class SocketAddressTest: XCTestCase { var thirdIPAddress = thirdAddress.address first.withSockAddr { outerAddr, outerSize in - firstIPAddress.withSockAddr { innerAddr, innerSize in + __testOnly_withSockAddr(&firstIPAddress) { innerAddr, innerSize in XCTAssertEqual(outerSize, innerSize) XCTAssertEqual(memcmp(innerAddr, outerAddr, min(outerSize, innerSize)), 0) XCTAssertNotEqual(outerAddr, innerAddr) } } second.withSockAddr { outerAddr, outerSize in - secondIPAddress.withSockAddr { innerAddr, innerSize in + __testOnly_withSockAddr(&secondIPAddress) { innerAddr, innerSize in XCTAssertEqual(outerSize, innerSize) XCTAssertEqual(memcmp(innerAddr, outerAddr, min(outerSize, innerSize)), 0) XCTAssertNotEqual(outerAddr, innerAddr) @@ -363,7 +363,7 @@ class SocketAddressTest: XCTestCase { func testCanMutateSockaddrStorage() throws { var storage = sockaddr_storage() XCTAssertEqual(storage.ss_family, 0) - storage.withMutableSockAddr { (addr, _) in + __testOnly_withMutableSockAddr(&storage) { (addr, _) in addr.pointee.sa_family = sa_family_t(NIOBSDSocket.AddressFamily.unix.rawValue) } XCTAssertEqual(storage.ss_family, sa_family_t(NIOBSDSocket.AddressFamily.unix.rawValue)) diff --git a/Tests/NIOPosixTests/TestUtils.swift b/Tests/NIOPosixTests/TestUtils.swift index 6a0ebcea..ac529727 100644 --- a/Tests/NIOPosixTests/TestUtils.swift +++ b/Tests/NIOPosixTests/TestUtils.swift @@ -274,10 +274,10 @@ func resolverDebugInformation(eventLoop: EventLoop, host: String, previouslyRece return "uds" case .v4(let sa): var addr = sa.address - return addr.addressDescription() + return __testOnly_addressDescription(&addr) case .v6(let sa): var addr = sa.address - return addr.addressDescription() + return __testOnly_addressDescription(&addr) } } let res = GetaddrinfoResolver(loop: eventLoop, aiSocktype: .stream, aiProtocol: CInt(IPPROTO_TCP)) diff --git a/docker/docker-compose.2004.main.yaml b/docker/docker-compose.2004.main.yaml index 889e7e90..f7af308f 100644 --- a/docker/docker-compose.2004.main.yaml +++ b/docker/docker-compose.2004.main.yaml @@ -56,7 +56,6 @@ services: - MAX_ALLOCS_ALLOWED_udp_1000_reqs_1_conn=12200 - MAX_ALLOCS_ALLOWED_udp_1_reqs_1000_conn=188050 # - SANITIZER_ARG=--sanitize=thread # TSan broken still - - SWIFT_TEST_VERB=build # WARNING: THIS DISABLES ALL TESTS. Please remove (workaround https://bugs.swift.org/browse/SR-14268) performance-test: image: swift-nio:20.04-main