Skip to content

Conversation

@agrabz
Copy link
Contributor

@agrabz agrabz commented May 27, 2025

Sorry, I know that this is going to be a long one. Hope it won't waste your time.

Motivation

I finished the refactoring of our project to use the ContainerTrait in swift-testing, but I realized that it is quite easy to achieve this behavior in XCTests as well by providing a subclass of XCTestCase in the form of XCContainerTestCase that basically overrides the invokeTest() function and adds the TaskLocal provided scoping mechanism there. As our project is still heavily using XCTests (~80:20 ratio), it was important to enable the parallelization of not just Swift Testing but XCTests as well. What is proposed in this PR is already in our project and improved our test execution time from 10min 30sec to 1min 40 sec.

Implementation

The implementation required the outsourcing of our scoping logic from ContainerTrait to a global function which I named withContainer, it has an async and a sync variant. Async is used in ContainerTrait, but due to the nature of XCTest (namely, that invokeTest() cannot be async) the sync variant is used in XCContainerTestCase. Albeit the async variant can be used in XCTests as well if used straight (not through XCContainerTestCase), see Not inheriting from the proposed custom XCTestCase but using withContainer below.

With that adjustment XCContainerTrait is proposed as below:

/// Provides a unique `Container` and a singleton scope to every unit test function in the class. Mimicking the behavior of `ContainerTrait` for `swift-testing`.
open class XCContainerTestCase: XCTestCase {

    /// The optional transformation to apply to the `Container` before invoking the test.
    /// Due to the nature of XCTest, this is not async and should be used for synchronous transformations only.
    open var transform: (@Sendable (Container) -> Void)?

    /// Scopes the unit test function to a unique singleton scope and a `Container` instance transformed via the `transform` variable (if overridden to non-nil).
    public override func invokeTest() {
        withContainer(
            shared: Container.$shared,
            container: Container(),
            operation: super.invokeTest,
            transform: self.transform
        )
    }
}

Usage

There are multiple new usages proposed by this PR.

Inheriting XCContainerTest instead of XCTestCase

The most typical usage will probably be something like below:

final class ParallelXCTests: XCContainerTestCase {

Working with multiple containers

It is easy to use multiple containers inside the custom XCTestCase where at the end we might end up with something like below:

final class ParallelContainerAndCustomContainerTest: XCContainerAndCustomContainerTestCase {

Using the transform sugar

Sugar might not be the best phrase here (at least for me) but it is possible to add it like below:

    override var transform: (@Sendable (Container) -> Void)? {
        get {
            {
                $0.fooBarBaz.register { Baz() }
                $0.fooBarBazCached.register { Baz() }
                $0.fooBarBazSingleton.register { Baz() }
            }
        }
        set { }
    }

Not inheriting from the proposed custom XCTestCase but using withContainer

One might be afraid of using anything else than XCTestCase to inherit from, but they can also leverage the power that it brings by using the withContainer function as below:

    func testFoo() {
        let fooExpectation = expectation(description: "foo")

        withContainer(
            shared: Container.$shared,
            container: Container()
        ) {
            let c = Container.shared
            c.fooBarBaz.register { Foo() }
            c.fooBarBazCached.register { Foo() }
            c.fooBarBazSingleton.register { Foo() }

            commonTests("foo")
            fooExpectation.fulfill()
        }

        wait(for: [fooExpectation], timeout: 60)
    }

It also has an async overload (as reasync is not yet a thing in Swift), which allows the usage of it like this:

    func testFooWithContainerAsync() async {
        let fooExpectation = expectation(description: "foo")

        await withContainer(
            shared: Container.$shared,
            container: Container()
        ) {
            Container.shared.fooBarBaz.register { Foo() }

            await Container.shared.isolatedToMainActor.register { @MainActor in MainActorFooBarBaz(value: "foo") }

            await isolatedAsyncTests("foo")
            fooExpectation.fulfill()
        }

        await fulfillment(of: [fooExpectation], timeout: 60)
    }

withContainer + transform sugar

The withContainer also has another closure to transform our Container:

    func testFooWithContainerSyncTransform() {
        let fooExpectation = expectation(description: "foo")

        withContainer(
            shared: Container.$shared,
            container: Container()
        ) {
            commonTests("foo")
            fooExpectation.fulfill()
        } transform: {
            $0.fooBarBaz.register { Foo() }
            $0.fooBarBazCached.register { Foo() }
            $0.fooBarBazSingleton.register { Foo() }
        }

        wait(for: [fooExpectation], timeout: 60)
    }

withMultipleContainer custom version

One can simply create their own version of withContainer to combine multiple containers:

func withContainerAndCustomContainer(
    operation: @Sendable () -> Void,
    containerTransform: (@Sendable (Container) -> Void)? = nil,
    customContainerTransform: (@Sendable (CustomContainer) -> Void)? = nil
) {
    withContainer(
        shared: CustomContainer.$shared,
        container: CustomContainer(),
        operation: {
            withContainer(
                shared: Container.$shared,
                container: Container(),
                operation: operation,
                transform: containerTransform
            )
        },
        transform: customContainerTransform
    )
}

withContainer in swift-testing

If ContainerTrait cannot be used then withContainer can be used in swift-testing as well, straight in the function body.

Notes

I thought that Factory should use this feature as well in its tests, so I changed almost (see FactoryContext.shared below) all of the test files to inherit from the right XCContainerTestCase (when custom containers were used I created their XCContainerTestCase variant accordingly. I used package modifier because they used to be fileprivate, but maybe just internal would be enough).

What we win with this (other than the parallelization) is that we won't need to call Container.shared.reset() anymore in setup funcs.

FactoryContext.shared

While I was doing these changes I realized that FactoryContext.shared also would need to be attached with @TaskLocal because right now the strictPromise tests are failing if they inherit from some XCContainerTrait. I looked into adding task local to FactoryContext but that is somewhat harder so I plan to work on that based on the fate of this PR.

Considerations with the usage of XCContainerTestCase

While using XCContainerTestCase helps a lot there are some things that the users should be aware of.

Too DRY

Devs tend to have many members in XCTestCases (var sut: SomeUseCase!) being used in setup and maybe in tearDown funcs. These don't always behave that well with TaskLocal's withValue scoping mechanism so it's suggested to try to minimize their usage.

XCTest parallelization

XCTest tests do not run in parallel, in the same way as swift-testing tests. Take the below as an example:

final class XCTest1: XCTestCase {
    func one() { ... }
    func two() { ... }
}
final class XCTest2: XCTestCase {
    func three() { ... }
    func four() { ... }
}

It's important to be aware that XCTest1.one() and XCTest1.two() will not run in parallel with each other, just as XCTest2.three() and XCTest2.four() will also not run in parallel with each other.

However, it's entirely possible that XCTest1.one() runs in parallel with XCTest2.three() or XCTest2.four().

In other words, tests inside the same XCTestCase subclass do not run in parallel with each other. XCTest parallelization occurs at the test case class level, not the individual method level. So test methods within the same test class are executed serially, while methods from different test classes can be executed in parallel.

Therefore, to achieve the best performance through test parallelization, it makes sense to break large test classes into smaller ones. This allows more test classes to be executed concurrently, especially when the test suite is run with multiple parallel workers.

Remember: your parallel tests will only be as fast as your slowest XCTestCase class, because methods within each class are run sequentially. If one class takes significantly longer than others, it can become the bottleneck in the parallel execution process.

Docs

If this PR gets accepted then I'm obviously okay to update the docs accordingly in a subsequent PR.

@hmlongco
Copy link
Owner

"it was important to enable the parallelization of not just Swift Testing but XCTests as well "

So... let me walk through this so we're on the same page.

From my understanding, XCTestCases do not run in parallel by default. So if you have XCTestCaseA, XCTestCaseB, and XCTestCaseC, each of the tests in each of the classes runs sequentially, one after the other.

And if you turn parallelization on... that doesn't change.

Unless...

If a test or tests seem to be taking awhile, then Xcode could spin up clones of the working environment so that it could run the tests in A, B, and C in parallel.

But each one of those clones is a separate, distinct instance of the application. And the tests in each of the instances still run sequentially, one after the other.

Further (and from what I've observed) Xcode runs the all of the XCTestCases first, and then runs Swift Testing second, such that any thing that occurs in one shouldn't affect the other.

And if that's the case, I'd need to view this not as something that's needed in order to make parallel XCTests work, but as a convenience class people could use if desired. Correct? Or am I missing something?

@hmlongco
Copy link
Owner

hmlongco commented May 27, 2025

"strictPromise tests are failing"

Is that related to _promiseTriggersError or something else? I do note that reset doesn't reset that value or _defaultScope back to default if called, and it probably should.

@agrabz
Copy link
Contributor Author

agrabz commented May 28, 2025

@hmlongco my day was really full sorry. I'll provide a meaningful answer early tomorrow

@agrabz
Copy link
Contributor Author

agrabz commented May 29, 2025

Yes, exactly. I should have written this clearer in the PR description that this convenience class only helps if you have so many XCTests that XCode decides to open up simulator clones to run them in parallel.

Based on your response (especially the section where you highlighted simulator clones), I've rechecked our project.
I have to admit that I misinterpreted how Xcode deals with these clones. Just by looking at the below screenshot I thought that each parallel test execution uses the same clone. However based on your reply and a second look I realized that the PIDs are different. Xcode not rebuilding the project for the clones also fitted nicely to my assumption that these clones run their tests in parallel.
Screenshot 2025-05-29 at 9 59 11

The anonimized Tests on the screenshot all belong to the same test target.

Having different PIDs means that these test executions are already isolated and we don't need to isolate them further with TaskLocals.

So I checked what caused our XCTests to fail when run in parallel. Now it turned out that our tests can succeed if above the super.invokeTest() I just call Container.shared.reset() without any TaskLocal. So probably we had some issues in our project by not resetting the Container properly. I also have to admit that for me it was not clear that resetting a container resets the singleton scope as well.

So probably this PR could be changed as well:

    public override func invokeTest() {
        Container.shared.reset()
        transform?(Container.shared)
        super.invokeTest()
        Container.shared.reset() //only needed if there are tests that are yet to inherit from XCContainerTestCase because of a possible gradual migration. This avoids test leaks, while still eliminates the need to call Container.shared.reset in setups and teardowns.
    }

However I feel that task local is still a better choice here as it keeps XCTest and SwifTesting support of FactoryTesting unified (via the withContainer). And if FactoryContext will also be task localized then we have to add it to only one place.

It's again a wall of text I know, I felt it necessary to write down my thinking.

@agrabz
Copy link
Contributor Author

agrabz commented May 29, 2025

"Further (and from what I've observed) Xcode runs the all of the XCTestCases first, and then runs Swift Testing second, such that any thing that occurs in one shouldn't affect the other."
that was also my thought and I'm sure that it used to work like that, but now when in my test plan each of my test target has the same parallelization option, they seem to run in parallel. it'd be nice to have better docs from Apple about XCTest parallelization and the mix of XCTest plus SwiftTesting

@agrabz
Copy link
Contributor Author

agrabz commented May 29, 2025

"strictPromise tests are failing"

Is that related to _promiseTriggersError or something else? I do note that reset doesn't reset that value or _defaultScope back to default if called, and it probably should.

If I change this: final class FactoryCoreStrictPromiseTests: XCTestCase {
to this: final class FactoryCoreStrictPromiseTests: XCContainerTestCase {

then I'll have a test failure on line expectNonFatalError. inside expectNonFatalError it fails at:

        Thread(block: {
            testcase()
            expectation.fulfill()
        }).start()

        waitForExpectations(timeout: 0.1) { _ in
            XCTAssertEqual(assertionMessage, "") //testStrictParameterPromise(): XCTAssertEqual failed: ("ParameterService was not registered") is not equal to ("")
// testStrictPromise(): XCTAssertEqual failed: ("MyServiceType was not registered") is not equal to ("")
            triggerFatalError = Swift.fatalError
        }

but now looking at it again it might rather be because of thread blocking above the assertions. it might not be correct anymore as we're inside a TaskLocal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants