Swift Testing support: ServeCommand did not shutdown before deinit #3236

Dean151 · 2024-09-16T10:33:14Z

Describe the issue

As of today, it seems hard to convert XCTest to Swift Testing as is, as the tearDown/deinit upgrade changes a lot the behavior!

Vapor version

4.105.2

Operating system and version

macOS 15

Swift version

Swift 6.0

Steps to reproduce

Convert the test example to Swift Testing:

@testable import App
import Testing
import Vapor
import XCTVapor

final class AppTests {
    var app: Application

    init() async throws {
        self.app = try await Application.make(.testing)
        try await configure(app)
    }

    deinit {
        app.shutdown()
    }

    @Test
    func helloWorld() async throws {
        try await self.app.test(.GET, "hello", afterResponse: { res async in
            #expect(res.status == .ok)
            #expect(res.body.string == "Hello, world!")
        })
    }
}

Then run the tests

Outcome

Assertion failed: ServeCommand did not shutdown before deinit

Additional notes

Running on my M1 Pro Mac with no virtualization (Xcode 16 RC)

Dean151 · 2024-09-16T10:42:40Z

Note that similar issue occurs on Vapor 5.
Therefore, it's harder, because there is no synchronous shutdown in Vapor 5, and:

defer does not support throws nor async calls…
deinit does not support throws nor async calls…

The only workaround seems to call shutdown manually at the end of each test, which is not ideal:

@testable import App
import Testing
import Vapor
import XCTVapor

final class AppTests {
    var app: Application

    init() async throws {
        self.app = await Application(.testing)
        try await configure(app)
    }

    @Test
    func helloWorld() async throws {
        try await self.app.test(.GET, "hello", afterResponse: { res async in
            #expect(res.status == .ok)
            #expect(res.body.string == "Hello, world!")
        })

        try await app.shutdown()
    }
}

0xTim · 2024-09-16T15:37:56Z

Yeah the deinit dance isn't going to work for Vapor. What you'll need is a function that can shut the app down and do something like:

    @Test
    func helloWorld() async throws {
        withApp(/* whatever set up needed*/) { app in
            try await self.app.test(.GET, "hello", afterResponse: { res async in
                #expect(res.status == .ok)
                #expect(res.body.string == "Hello, world!")
            })
        }
    }

This is the recommended way of supporting structured concurrency in Swift Testing. I suspect Vapor 5 will have some helpers for this which we might back port.

Dean151 · 2024-09-21T19:44:28Z

Because tests run by default in parallel, I've been changing my implementation a bit:

import App
import Lock
import Vapor
import XCTVapor

func withApp(_ block: @Sendable (Application) async throws -> Void) async throws {
    try await AppActor.shared.run(block: block)
}

private actor AppActor {
    static let shared = AppActor()
    private let lock = AsyncLock()

    func run(block: (Application) async throws -> Void) async throws {
        await lock.lock()
        let app = await Application(.testing)
        do {
            try await configure(app)
            try await block(app)
            try await app.shutdown()
        } catch {
            try? await app.shutdown()
            lock.unlock()
            throw error
        }
        lock.unlock()
    }
}

This is using the awesome https://github.com/mattmassicotte/Lock from @mattmassicotte ensuring that only one app is used at the same time, and others waits for the previous to be uninitialized before being resumed.
Not sure if it's possible to really parallelize tests because of DB.

0xTim · 2024-09-23T00:56:03Z

Why not just use the .serialized trait on the suite? That avoids any use of locks etc.

In general, anything that touches a DB will need to be sequential - there isn't really any way around that. For tests that don't need a database (such as if you architect your app with the repository pattern) you should be able to run these in parallel without any issue with the .testable() application, either using the in-memory option or binding to port 0 if you really need a real connection.

Dean151 · 2024-09-23T09:52:41Z

Why not just use the .serialized trait on the suite? That avoids any use of locks etc.

In general, anything that touches a DB will need to be sequential - there isn't really any way around that. For tests that don't need a database (such as if you architect your app with the repository pattern) you should be able to run these in parallel without any issue with the .testable() application, either using the in-memory option or binding to port 0 if you really need a real connection.

Maybe I misunderstood the doc, but for me, serialized only applies on a Suite, meaning that you have to wrap your tests.
Also, the serialized will also apply for that specific suite. And I'm not sure multiple suites will be serialized with each other, to be tested!
Using a lock is a safety that ensure that serialization, and allows inline tests like the framework provide.

See https://mastodon.social/@deanatoire/113177506464630372
And the doc reference: https://developer.apple.com/documentation/testing/parallelizationtrait

0xTim · 2024-09-23T15:06:15Z

You can apply the trait to an individual test but yes the easiest way is a @Suite that covers all the tests (you can use extensions to break them up). This will become the recommended way for any Swift Testing stuff and fit it with all the documentation etc

Dean151 added the bug Something isn't working label Sep 16, 2024

This comment was marked as outdated.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Swift Testing support: ServeCommand did not shutdown before deinit #3236

Swift Testing support: ServeCommand did not shutdown before deinit #3236

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Swift Testing support: ServeCommand did not shutdown before deinit #3236

Swift Testing support: ServeCommand did not shutdown before deinit #3236

Comments

Describe the issue

Vapor version

Operating system and version

Swift version

Steps to reproduce

Outcome

Additional notes

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!