Fix some bugs

Author: gjcairo

Sources/GRPC/ConnectionManagerChannelProvider.swift

@@ -266,14 +266,6 @@ internal struct DefaultChannelProvider: ConnectionManagerChannelProvider {
           return channel.eventLoop.makeFailedFuture(error)
         }
 
-        #if canImport(Network)
-        if #available(macOS 10.14, iOS 12.0, watchOS 6.0, tvOS 12.0, *),
-           let configurator = self.nwParametersConfigurator,
-           let transportServicesBootstrap = bootstrap as? NIOTSConnectionBootstrap {
-          _ = transportServicesBootstrap.configureNWParameters(configurator)
-        }
-        #endif
-
         // Run the debug initializer, if there is one.
         if let debugInitializer = self.debugChannelInitializer {
           return debugInitializer(channel)
@@ -286,6 +278,14 @@ internal struct DefaultChannelProvider: ConnectionManagerChannelProvider {
       _ = bootstrap.connectTimeout(connectTimeout)
     }
 
+    #if canImport(Network)
+    if #available(macOS 10.14, iOS 12.0, watchOS 6.0, tvOS 12.0, *),
+       let configurator = self.nwParametersConfigurator,
+       let transportServicesBootstrap = bootstrap as? NIOTSConnectionBootstrap {
+      _ = transportServicesBootstrap.configureNWParameters(configurator)
+    }
+    #endif
+
     return bootstrap.connect(to: self.connectionTarget)
   }
 }

Tests/GRPCTests/ConnectionManagerTests.swift

@@ -1421,49 +1421,49 @@ extension ConnectionManagerTests {
 
   #if canImport(Network)
   func testDefaultChannelProvider_NWParametersConfigurator() throws {
+    // For this test, we want an actual connection to be established, since otherwise the parameters
+    // configurator won't be run: NIOTS will only apply the parameters on the NWConnection at the
+    // point of activating it.
+
+    // Start a server
+    let serverConfig = Server.Configuration.default(
+      target: .hostAndPort("localhost", 0),
+      eventLoopGroup: NIOTSEventLoopGroup.singleton,
+      serviceProviders: []
+    )
+    let server = try Server.start(configuration: serverConfig).wait()
+    defer {
+      try? server.close().wait()
+    }
+
+    // Create a connection manager, and configure it to increase a counter in its NWParameters
+    // configurator closure.
     let counter = NIOLockedValueBox(0)
     let group = NIOTSEventLoopGroup.singleton
     var configuration = ClientConnection.Configuration.default(
-      target: .unixDomainSocket("/ignored"),
+      target: .socketAddress(server.channel.localAddress!),
       eventLoopGroup: group
     )
     configuration.nwParametersConfigurator = { _ in
       counter.withLockedValue { $0 += 1 }
     }
-
-    // We need to trigger the connection to apply the parameters configurator.
-    // However, we don't actually care about establishing the connection: only triggering it.
-    // In other tests, we used mocked channel providers to simply return completed futures when "creating"
-    // the channels. However, in this test we want to make sure that the `DefaultChannelProvider`,
-    // which is the one we actually use, correctly sets and executes the configurator.
-    // For this reason, we can wait on a promise that is succeeded after the configurator has been called,
-    // in the debug channel initializer. This promise will always succeed regardless of the actual
-    // connection being established. And because this closure is executed after the parameters configurator,
-    // we know the counter should be updated by the time the promise has been completed.
-    let promise = group.next().makePromise(of: Void.self)
-    configuration.debugChannelInitializer = { channel in
-      promise.succeed()
-      return promise.futureResult
-    }
-
     let manager = ConnectionManager(
       configuration: configuration,
       connectivityDelegate: self.monitor,
       idleBehavior: .closeWhenIdleTimeout,
       logger: self.logger
     )
+    defer {
+      try? manager.shutdown().wait()
+    }
 
-    // Start connecting. We don't care about the actual result of the connection.
-    _ = manager.getHTTP2Multiplexer()
-
-    // Wait until the configurator has been called.
-    try promise.futureResult.wait()
+    // Wait for the connection to be established.
+    _ = try manager.getHTTP2Multiplexer().wait()
 
+    // At this point, the configurator must have been called.
     XCTAssertEqual(1, counter.withLockedValue({ $0 }))
   }
   #endif
-
-  // TODO: add test using the default channel provider that uses the parameter configurator and make sure it gets called
 }
 
 internal struct Change: Hashable, CustomStringConvertible {

Tests/GRPCTests/ConnectionPool/GRPCChannelPoolTests.swift

@@ -69,7 +69,11 @@ final class GRPCChannelPoolTests: GRPCTestCase {
       self.group = MultiThreadedEventLoopGroup(numberOfThreads: threads)
 
     case .transportServicesEventLoopGroup:
+      #if canImport(Network)
       self.group = NIOTSEventLoopGroup(loopCount: threads)
+      #else
+      fatalError("NIOTS is not available on this platform.")
+      #endif
     }
   }
 
@@ -127,9 +131,10 @@ final class GRPCChannelPoolTests: GRPCTestCase {
   private func setUpClientAndServer(
     withTLS tls: Bool,
     threads: Int = System.coreCount,
+    eventLoopGroupType: TestEventLoopGroupType = .multiThreadedEventLoopGroup,
     _ configure: (inout GRPCChannelPool.Configuration) -> Void = { _ in }
   ) {
-    self.configureEventLoopGroup(threads: threads)
+    self.configureEventLoopGroup(threads: threads, eventLoopGroupType: eventLoopGroupType)
     self.startServer(withTLS: tls)
     self.startChannel(withTLS: tls) {
       // We'll allow any number of waiters since we immediately fire off a bunch of RPCs and don't
@@ -639,19 +644,16 @@ final class GRPCChannelPoolTests: GRPCTestCase {
   }
 
   #if canImport(Network)
-  func testNWParametersConfigurator() {
+  func testNWParametersConfigurator() throws {
     let counter = NIOLockedValueBox(0)
-    self.configureEventLoopGroup(threads: 1, eventLoopGroupType: .transportServicesEventLoopGroup)
-    self.startServer(withTLS: false)
-    self.startChannel(withTLS: false) { configuration in
+    self.setUpClientAndServer(withTLS: false, eventLoopGroupType: .transportServicesEventLoopGroup) { configuration in
       configuration.transportServices.nwParametersConfigurator = { _ in
         counter.withLockedValue { $0 += 1 }
       }
     }
 
     // Execute an RPC to make sure a channel gets created/activated and the parameters configurator run.
-    let rpc = self.echo.get(.with { $0.text = "" })
-    XCTAssertNoThrow(try rpc.status.wait())
+    try self.doTestUnaryRPCs(count: 1)
 
     XCTAssertEqual(1, counter.withLockedValue({ $0 }))
   }