[Crashlytics] Fix flaky tests

[paulb777]

This PR addresses several sources of test flakiness within the Crashlytics unit test suite, improving the overall stability of CI. The fixes focus on resolving race
conditions, improving test isolation, and handling filesystem interactions more robustly.

Key Changes

• FIRCLSMockFileManager Thread Safety:

  • Introduced @synchronized blocks for all methods in FIRCLSMockFileManager that access or modify its internal shared state. This resolves a root cause of
    flakiness by preventing race conditions when the mock file manager is accessed from multiple threads concurrently.

• FIRCLSSettingsTests Timeout Adjustments:

  • Increased the timeout values for asynchronous expectations in cacheSettingsWithGoogleAppID and reloadFromCacheWithGoogleAppID. This makes the tests more
    resilient to timing variations in different environments, particularly CI.

• FIRCLSUserDefaultsTests Test Isolation:

  • Enhanced the tearDown method to thoroughly clean up all test keys from NSUserDefaults. This prevents state from leaking between tests and ensures each test
    runs in a clean, predictable environment.

• FIRCLSExistingReportManagerTests Cleanup Logic:

  • Restructured the setUp method to guarantee a clean file system state. The test directory is now rigorously cleaned and re-created before the
    FIRCLSExistingReportManager is initialized, preventing it from loading stale data from previous test runs.

• FIRCLSContextManagerTests Filesystem Interaction:

  • Fixed a recurring mkstemp: No such file or directory error by refactoring the test to use a real temporary directory created with mkdtemp. This ensures that
    all components under test that interact with the filesystem have valid paths, resolving the crash.

[gemini-code-assist]

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

[google-oss-bot]

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses flaky tests by refactoring the asynchronous testing logic. The previous approach of directly managing XCTestExpectation in the mock object was prone to race conditions. The new implementation using NSNotificationCenter is a much cleaner and more robust solution that decouples the mock from the test's expectation management. The changes also include correcting the expected number of file removals in testCorruptCache and cleaning up unused code in other tests, which improves the overall quality of the test suite.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses flaky tests by refactoring the asynchronous testing logic. The move from direct XCTestExpectation management within the mock object to a more decoupled NSNotificationCenter-based approach is a significant improvement for test robustness and maintainability. Adding synchronization to FIRCLSMockFileManager is a crucial fix for thread-safety issues that were likely a primary source of flakiness. The other adjustments, such as correcting expected values and improving test cleanup, further enhance the quality of the test suite. I have one minor suggestion to make the test cleanup even more robust.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great improvement for the test suite's stability. The refactoring from direct XCTestExpectation management in the mock to using NSNotificationCenter is a much cleaner and more robust approach to handling asynchronous testing, effectively decoupling the mock from the test's expectations. The addition of @synchronized blocks in FIRCLSMockFileManager to ensure thread-safety is a critical fix for potential race conditions. The other changes, like improving test cleanup in FIRCLSUserDefaultsTests and correcting the expected file removal count in testCorruptCache, further enhance the quality and reliability of the tests. I have one minor suggestion to fully complete the thread-safety improvements in the mock file manager.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of addressing flaky tests by refactoring the asynchronous testing logic. The move from managing XCTestExpectation within the mock object to using NSNotificationCenter is a significant improvement, promoting better decoupling and reducing the chances of race conditions. The introduction of @synchronized blocks in FIRCLSMockFileManager to ensure thread-safety is a good defensive measure. Additionally, the corrections to the expected file removal counts and the improved test cleanup in FIRCLSUserDefaultsTests enhance the overall quality and reliability of the test suite. The changes are well-executed and thoughtfully implemented.

[paulb777]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively addresses flaky tests by refactoring the asynchronous testing logic. The move from directly managing XCTestExpectation in the mock object to using NSNotificationCenter is a significant improvement, decoupling the mock and making the tests cleaner and more robust. The introduction of thread safety in FIRCLSMockFileManager with @synchronized blocks is a crucial fix for potential race conditions. Additionally, the cleanup of test state in FIRCLSUserDefaultsTests and the correction of assertion values contribute to a higher quality and more reliable test suite. The changes are excellent.

[gemini-code-assist]

The timeout for waiting on this expectation has been increased significantly to 16.0 seconds (and similarly to 14.0 seconds in reloadFromCacheWithGoogleAppID). While this is effective at preventing timeouts on slower CI environments and fixing flakiness, it's worth noting that it can also increase the overall test suite execution time. This is a reasonable and pragmatic approach to improve test stability, but if these operations aren't expected to take this long, it might be worth investigating if there's an underlying performance issue that could be addressed in the future.