-
-
Notifications
You must be signed in to change notification settings - Fork 170
Add XCContainerTestCase to support parallel XCTests better #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
and update accordingly
This reverts commit cc7ff241efda18140a5efb6784011e6a78b3c8e8.
FactoryContext is not yet tasklocalized
|
"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? |
|
"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. |
|
@hmlongco my day was really full sorry. I'll provide a meaningful answer early tomorrow |
|
"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." |
If I change this: then I'll have a test failure on line 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 |

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
XCContainerTestCasethat basically overrides theinvokeTest()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), seeNot inheriting from the proposed custom XCTestCase but using withContainerbelow.With that adjustment XCContainerTrait is proposed as below:
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:
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:
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:
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
withContainerfunction as below:It also has an async overload (as
reasyncis not yet a thing in Swift), which allows the usage of it like this:withContainer + transform sugar
The withContainer also has another closure to transform our Container:
withMultipleContainer custom version
One can simply create their own version of withContainer to combine multiple containers:
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
packagemodifier because they used to befileprivate, 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
XCContainerTestCasehelps 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:
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.