Skip to content

Commit

Permalink
General warnings and tests cleanup (#3107)
Browse files Browse the repository at this point in the history
* Require newer NIO, fix numerous warnings and the deprecation notices, and explicitly specify complete concurrency checking.

* Fix threading mishaps due to use of EmbeddedEventLoop, avoid port collisions between tests when running tests in parallel, silence warnings when building with the nightlies.
  • Loading branch information
gwynne committed Nov 21, 2023
1 parent 1aeeaaf commit da9c280
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 41 deletions.
2 changes: 1 addition & 1 deletion Package.swift
Expand Up @@ -34,7 +34,7 @@ let package = Package(
.package(url: "https://github.com/swift-server/swift-backtrace.git", from: "1.1.1"),

// Event-driven network application framework for high performance protocol servers & clients, non-blocking.
.package(url: "https://github.com/apple/swift-nio.git", from: "2.59.0"),
.package(url: "https://github.com/apple/swift-nio.git", from: "2.62.0"),

// Bindings to OpenSSL-compatible libraries for TLS support in SwiftNIO
.package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.8.0"),
Expand Down
12 changes: 6 additions & 6 deletions Package@swift-5.9.swift
Expand Up @@ -31,7 +31,7 @@ let package = Package(
.package(url: "https://github.com/vapor/routing-kit.git", from: "4.5.0"),

// Event-driven network application framework for high performance protocol servers & clients, non-blocking.
.package(url: "https://github.com/apple/swift-nio.git", from: "2.59.0"),
.package(url: "https://github.com/apple/swift-nio.git", from: "2.62.0"),

// Bindings to OpenSSL-compatible libraries for TLS support in SwiftNIO
.package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.8.0"),
Expand Down Expand Up @@ -93,7 +93,7 @@ let package = Package(
.product(name: "MultipartKit", package: "multipart-kit"),
.product(name: "Atomics", package: "swift-atomics"),
],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency")]
swiftSettings: [.enableExperimentalFeature("StrictConcurrency=complete")]
),

// Development
Expand All @@ -103,7 +103,7 @@ let package = Package(
.target(name: "Vapor"),
],
resources: [.copy("Resources")],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency")]
swiftSettings: [.enableExperimentalFeature("StrictConcurrency=complete")]
),

// Testing
Expand All @@ -112,7 +112,7 @@ let package = Package(
dependencies: [
.target(name: "Vapor"),
],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency")]
swiftSettings: [.enableExperimentalFeature("StrictConcurrency=complete")]
),
.testTarget(
name: "VaporTests",
Expand All @@ -130,15 +130,15 @@ let package = Package(
.copy("Utilities/expired.crt"),
.copy("Utilities/expired.key"),
],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency")]
swiftSettings: [.enableExperimentalFeature("StrictConcurrency=complete")]
),
.testTarget(
name: "AsyncTests",
dependencies: [
.product(name: "NIOTestUtils", package: "swift-nio"),
.target(name: "XCTVapor"),
],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency")]
swiftSettings: [.enableExperimentalFeature("StrictConcurrency=complete")]
),
]
)
1 change: 1 addition & 0 deletions Sources/Vapor/Concurrency/RequestBody+Concurrency.swift
Expand Up @@ -130,6 +130,7 @@ extension Request.Body: AsyncSequence {
failureType: Error.self,
backPressureStrategy: NIOAsyncSequenceProducerBackPressureStrategies
.HighLowWatermark(lowWatermark: 5, highWatermark: 20),
finishOnDeinit: true,
delegate: delegate
)

Expand Down
2 changes: 1 addition & 1 deletion Sources/Vapor/Request/Request.swift
Expand Up @@ -2,7 +2,7 @@ import Foundation
import NIOCore
import NIOHTTP1
import Logging
@preconcurrency import RoutingKit
import RoutingKit
import NIOConcurrencyHelpers

/// Represents an HTTP request in an application.
Expand Down
2 changes: 1 addition & 1 deletion Sources/Vapor/Routing/Route.swift
@@ -1,5 +1,5 @@
import NIOHTTP1
@preconcurrency import RoutingKit
import RoutingKit
import NIOConcurrencyHelpers

public final class Route: CustomStringConvertible, Sendable {
Expand Down
3 changes: 3 additions & 0 deletions Sources/Vapor/Utilities/RFC1123.swift
@@ -1,3 +1,6 @@
#if canImport(Glibc) && swift(>=5.10)
@preconcurrency import Glibc
#endif
import Foundation
import NIOPosix
import NIOCore
Expand Down
15 changes: 7 additions & 8 deletions Tests/AsyncTests/AsyncClientTests.swift
Expand Up @@ -193,27 +193,26 @@ final class AsyncClientTests: XCTestCase {


final class CustomClient: Client, Sendable {
var eventLoop: EventLoop {
EmbeddedEventLoop()
}
let eventLoop: any EventLoop
let _requests: NIOLockedValueBox<[ClientRequest]>
var requests: [ClientRequest] {
get {
self._requests.withLockedValue { $0 }
}
}

init() {
self._requests = .init([])
init(eventLoop: any EventLoop, _requests: [ClientRequest] = []) {
self.eventLoop = eventLoop
self._requests = .init(_requests)
}

func send(_ request: ClientRequest) -> EventLoopFuture<ClientResponse> {
self._requests.withLockedValue { $0.append(request) }
return self.eventLoop.makeSucceededFuture(ClientResponse())
}

func delegating(to eventLoop: EventLoop) -> Client {
self
func delegating(to eventLoop: any EventLoop) -> Client {
self._requests.withLockedValue { CustomClient(eventLoop: eventLoop, _requests: $0) }
}
}

Expand All @@ -226,7 +225,7 @@ extension Application {
if let existing = self.storage[CustomClientKey.self] {
return existing
} else {
let new = CustomClient()
let new = CustomClient(eventLoop: self.eventLoopGroup.any())
self.storage[CustomClientKey.self] = new
return new
}
Expand Down
3 changes: 2 additions & 1 deletion Tests/VaporTests/RequestTests.swift
Expand Up @@ -277,6 +277,7 @@ final class RequestTests: XCTestCase {
)
}

@available(*, deprecated, message: "Testing deprecated methods; this attribute silences the warnings")
func testRedirect_old() throws {
let app = Application(.testing)
defer { app.shutdown() }
Expand Down Expand Up @@ -324,7 +325,7 @@ final class RequestTests: XCTestCase {
let request = Request(
application: app,
collectedBody: .init(string: ""),
on: EmbeddedEventLoop()
on: app.eventLoopGroup.any()
)

let handleBufferExpectation = XCTestExpectation()
Expand Down
28 changes: 5 additions & 23 deletions Tests/VaporTests/ServerTests.swift
Expand Up @@ -161,9 +161,9 @@ final class ServerTests: XCTestCase {

// start(address: .hostname(..., port: ...)) should set the hostname and port appropriately
oldServer = OldServer()
try oldServer.start(address: .hostname("localhost", port: 8080))
try oldServer.start(address: .hostname("localhost", port: 8081))
XCTAssertEqual(oldServer.hostname, "localhost")
XCTAssertEqual(oldServer.port, 8080)
XCTAssertEqual(oldServer.port, 8081)

// start(address: .unixDomainSocket(path: ...)) should throw
oldServer = OldServer()
Expand Down Expand Up @@ -219,9 +219,9 @@ final class ServerTests: XCTestCase {

// start(address: .hostname(..., port: ...)) should set the hostname and port appropriately
newServer = NewServer()
try newServer.start(address: .hostname("localhost", port: 8080))
try newServer.start(address: .hostname("localhost", port: 8082))
XCTAssertEqual(newServer.hostname, "localhost")
XCTAssertEqual(newServer.port, 8080)
XCTAssertEqual(newServer.port, 8082)
XCTAssertNil(newServer.socketPath)

// start(address: .unixDomainSocket(path: ...)) should throw
Expand Down Expand Up @@ -676,25 +676,7 @@ final class ServerTests: XCTestCase {

XCTAssertNoThrow(try app.start())
}

func testStartWithDefaultHostname() throws {
let app = Application(.testing)
app.http.server.configuration.address = .hostname(nil, port: 0)
defer { app.shutdown() }
app.environment.arguments = ["serve"]

XCTAssertNoThrow(try app.start())
}

func testStartWithDefaultPort() throws {
let app = Application(.testing)
app.http.server.configuration.address = .hostname("0.0.0.0", port: nil)
defer { app.shutdown() }
app.environment.arguments = ["serve"]

XCTAssertNoThrow(try app.start())
}


func testAddressConfigurations() throws {
var configuration = HTTPServer.Configuration()
XCTAssertEqual(configuration.address, .hostname(HTTPServer.Configuration.defaultHostname, port: HTTPServer.Configuration.defaultPort))
Expand Down

0 comments on commit da9c280

Please sign in to comment.