Skip to content

Add @TaskLocal to Container.shared, create ContainerTrait#272

Merged
hmlongco merged 23 commits intohmlongco:mainfrom
agrabz:tasklocal
Apr 16, 2025
Merged

Add @TaskLocal to Container.shared, create ContainerTrait#272
hmlongco merged 23 commits intohmlongco:mainfrom
agrabz:tasklocal

Conversation

@agrabz
Copy link
Copy Markdown
Contributor

@agrabz agrabz commented Apr 7, 2025

Motivation

With the new TestTrait, TestScoping features of Swift 6.1 it became possible to write parallelizable tests that are depending on the same dependencies (ST-007). When tests are run in parallel the execution becomes significantly faster, which is a huge help for big projects.

Implementation

To leverage this feature by Factory the simplest way is to attach the @TaskLocal macro to the Container.shared instance, which then enables us to also create the ContainerTrait so our Swift Testing tests can be scoped to new unique Container instances one-by-one and thus can run in parallel.
To use it in Swift Testing you have to use @Test(.container). As an example, please see ParallelTests.swift, which would fail randomly without this feature.

CustomContainers

It is not possible to attach the @TaskLocal macro to a protocol requirement and as such we cannot require every SharedContainer to add the @TaskLocal macro to their shared variables. This is probably better like this, because then this PR doesn't introduce a breaking change. However the docs are updated to suggest this approach to users.

Decision

To use TaskLocal, I had to bump the min support Mac version to v10_15 from v10_14.

XCTest

Parallel testing in XCTests is not enabled by default, however it is still possible to leverage the TaskLocal annotated shared Container in XCTests. If your XCTests are running in parallel then you can override the invokeTest() function and implement it like below, which will give you the same functionality as the Swift Testing TestTrait of this PR. However it is important to mention that this approach is way less flexible than the one that Swift Testing introduced (see Future Directions).

    override func invokeTest() {
        Container.$shared.withValue(Container()) {
            super.invokeTest()
        }
    }

As a different example, please see ParallelXCTest.swift, which would fail randomly without this feature.

Docs

The documentation had to be adjusted in several places to be aligned with this change. I hope I covered everything, please let me know if you think something else has to be adjusted.
One part that is probably more important to mention here is that it was suggested that the static shared variable of a SharedContainer has to be let and not var, which is not possible if we're using it with the @TaskLocal macro, I tried to be cautious with the wording.

In House Reference

I tried to find in Factory's repo if this approach was ever discussed and only found one Discussion #223

While I fully agree with @hmlongco with his below response. I think that the topic had to be revisited with the release of Swift 6.1.

...
Switching to concurrent testing is at odds with the behavior of the application itself, which means that swapping out shared resources no longer works. This also breaks the various property wrappers that depend on the shared containers.
...

Future Directions

SuiteTrait

Note that the ContainerTrait is not conforming to SuiteTrait so it cannot be added to a full test suite like: @Suite(.container), only to individual tests. It is definitely worth it to investigate that direction as well for the future. The challenge here is to make this implementation more fool proof.

Dependency Builder

With the API changes of this PR it is possible to use a fresh default Container for every Swift Testing test, however we could further enhance it with a syntax like below. Which would not just create a new container for the unit test, but also register the defined values to it.

@Test(.container {
    $0.myDependency1 = MockDependency()
    $0.myDependency2 = AnotherMockDependency()
})

Comment thread Package.swift
Comment thread Sources/Factory/Factory/ContainerTrait.swift
Comment thread Sources/Factory/Factory/ContainerTrait.swift
@hmlongco hmlongco merged commit c28d8b9 into hmlongco:main Apr 16, 2025
@hmlongco
Copy link
Copy Markdown
Owner

@agrabz Do you want to take a stab at updating the documentation accordingly?

https://github.com/hmlongco/Factory/blob/main/Sources/Factory/Factory.docc/Development/Testing.md

@agrabz
Copy link
Copy Markdown
Contributor Author

agrabz commented Apr 17, 2025

@agrabz Do you want to take a stab at updating the documentation accordingly?

https://github.com/hmlongco/Factory/blob/main/Sources/Factory/Factory.docc/Development/Testing.md

@hmlongco Sure, thank you. If you don't mind, I'd wait for the fate of the PRs #281 and #280 and include those as well.

@hmlongco
Copy link
Copy Markdown
Owner

@agrabz I think the code for test traits in 2.4.6 is as stable as it's going to be for awhile. Documentation?

@agrabz
Copy link
Copy Markdown
Contributor Author

agrabz commented Apr 28, 2025

@agrabz I think the code for test traits in 2.4.6 is as stable as it's going to be for awhile. Documentation?

On it. I'm a little swamped, but you can expect the PR today or tomorrow.

@tahirmt
Copy link
Copy Markdown
Contributor

tahirmt commented Apr 29, 2025

@hmlongco I didn't want to make a new issue for this but I was just trying out the main branch in an application project and because the import Testing statement and TestTrait is added to the Factory target, it is not allowing the application to compile anymore with an error Undefined symbol: associated type descriptor for Testing.Trait.TestScopeProvider.

I believe Testing works the same way as XCTest such that you cannot import it in the main application target and it will not allow you to embed it in an iOS application. Would you consider making a new framework FactoryTestSupport or something that can provide the traits alongside Factory?

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.

3 participants