Skip to content
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

Run each test in a separate process so that crashes don't stop the world. #156

Open
dabrahams opened this issue Dec 11, 2023 · 8 comments
Open
Labels
enhancement New feature or request

Comments

@dabrahams
Copy link

dabrahams commented Dec 11, 2023

Description

Forking is cheap, and this will allow a test suite to keep running even if one test unexpectedly fails. The Swift standard library tests work this way.

This requires giving up on "automatically parallelize tests in process," or at least allowing users to opt out, but not on scalable execution. Again, forking is cheap.

Expected behavior

No response

Actual behavior

No response

Steps to reproduce

No response

swift-testing version/commit hash

No response

Swift & OS version (output of swift --version && uname -a)

No response

@dabrahams dabrahams added the enhancement New feature or request label Dec 11, 2023
@grynspan
Copy link
Contributor

Thanks for filing! Some thoughts in no particular order…

Unfortunately, forking is not cheap on all platforms. In fact, it's not possible at all on platforms like iOS where process lifetime is heavily controlled by the system. So while we are interested in using multiple processes to recover from crashing tests, we can't rely on it universally. On the platforms where it is supported (and not prohibitively expensive), we are certainly interested in building a solution.

There's a lot of overlap here with #157. I realize this issue expresses a different concept, but the code required to implement a solution is the same: host tests in a second process and, on crash, recover, handle the crash appropriately, then continue running other tests. It seems to me that if we implement things correctly, then the only difference is whether the crash is expected or not. We can use this issue to track that implementation, and then use #157 to specifically track "and we were hoping it would happen too!" sort of logic. Does that make sense?

@dabrahams
Copy link
Author

Sure, and the overlap is part of why I filed these together.w
Right, can't fork on iOS; sheesh. But maybe you can fork onto separate simulators when not testing on-device.

@grynspan
Copy link
Contributor

Spinning up a separate simulator for each test would definitely disprove the "forking is cheap" theorem. 😁

@dabrahams
Copy link
Author

Depends how you do it. Presumably you can fork the simulator.

@grynspan
Copy link
Contributor

You cannot fork the simulator. 😶

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Jan 1, 2024

+1 for this issue.

Currently for some 3rd party library which want to test the crash call(AssertCrash), we had to fork some hacky implementation from swift-corelibs-foundation.

https://github.com/OpenCombine/OpenCombine/blob/1c6f02c7ed8140c0ba7a783aaddb6e0685a0037b/Tests/OpenCombineTests/Helpers/AssertCrashes.swift#L29-L85

And such logic will constantly fail to pass from time to time. (eg. On Linux platform with TSAN enabled, the test case will timeout in such case due to the recent change in Swift)

#if os(Linux) // FIXME: Skip assertCrashes on Linux when TSAN is enabled. This combination will run timeout on CI now.
if __sanitizeThreadEnabled() {
    return
}
#endif

So we had to add a check for __sanitizeThreadEnabled here which is also very tricky since the implementation of __sanitizeThreadEnabled will be tricky too.

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Jan 2, 2024

You cannot fork the simulator. 😶

xcrun simctl --help
...
spawn               Spawn a process by executing a given executable on a device.

@grynspan
Copy link
Contributor

grynspan commented Jan 2, 2024

That command spawns a process in the simulated environment. It does not fork an existing process (nor the simulator itself.) Spinning up a whole simulator is expensive and would not be helpful here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants