rules_apple
rules_apple copied to clipboard
Bundle libMainThreadChecker.dylib to detect main thread violations
Context
This PR introduces the integration of libMainThreadChecker.dylib to enhance quality checks by detecting main thread violations. By passing the feature=include_main_thread_checker flag, the library libMainThreadChecker.dylib will be embedded. Additionally, modifications to the test runner implementation ensure the DYLD_INSERT_LIBRARIES is updated to recognize this dylib. This aims to bolster both unit and UI test reliability.
https://developer.apple.com/documentation/xcode/diagnosing-memory-thread-and-crash-issues-early
For the following test-case,
func testTriggerMainThreadChecker() {
let expectation = self.expectation(description: "Background operation")
DispatchQueue.global().async {
// This will trigger the Main Thread Checker because we are
// trying to update the UI from a background thread.
let label = UILabel()
label.text = "Test"
expectation.fulfill()
}
waitForExpectations(timeout: 5) { (error) in
if let error = error {
XCTFail("waitForExpectations errored: \(error)")
}
}
}
bazel build //examples/ios/HelloWorld --features=include_main_thread_checker
Discuss:
- The tests are successful but the test logs report the violation.
- Now, this is in line with what Xcode does. Even Xcode reports the violation but doesn't fail the test suite, which is reasonable. Still, from a quality assurance angle where we need structured data of where the violation happened, it might not be super convenient as parsing of the logs is needed.
TODO:
- Documentation
@chiragramani Tested this today and its working nicely! One thing left would be failing (crashing) when the main thread checker is hit, I was able to get this by adding MTC_CRASH_ON_REPORT=1 to the TEST_ENV in the test runner template.
TEST_ENV="$TEST_ENV,DYLD_INSERT_LIBRARIES=$DYLD_INSERT_LIBRARIES_VALUE,MTC_CRASH_ON_REPORT=1"
We'd need to conditionalize it on that feature though
should it always crash when this is enabled? otherwise it's just in the log which seems like it would more likely get lost in the noise?
I think a use case for not crashing is potentially logging these issues somehow.
@chiragramani Do you have time to finish this PR up? If not, I could look at finishing any remaining work. Thank you!
@chiragramani Do you have time to finish this PR up? If not, I could look at finishing any remaining work. Thank you!
Hey, I'm back from medical leave and want to apologize for not addressing this PR sooner. I'll be catching up on the discussions now. Thanks for your understanding and patience.
One thing left would be failing (crashing) when the main thread checker is hit, I was able to get this by adding MTC_CRASH_ON_REPORT=1 to the TEST_ENV in the test runner template. We'd need to conditionalize it on that feature though
I added the following in the PR with a test-case, are you referring to something similar? --features=apple.fail_on_main_thread_checker.
I did not enable it by default to have parity with what Xcode does today.
@chiragramani I'm going to merge this once CI passes on the latest master
Tested end-to-end on our project and got the expected crash and checks when using the new features. Thanks @chiragramani!