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

[tests] Fix flaky tests #1466

Merged
merged 9 commits into from
Aug 6, 2024
Merged

[tests] Fix flaky tests #1466

merged 9 commits into from
Aug 6, 2024

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Jul 29, 2024

Description

One Line Summary

Fix flaky tests so the CI can be meaningful.

Details

Add OneSignalOSCoreMocks module

  • Reset the Operation Repo between tests

Introduce 2 separate Test Plans

  • There are now two Test Plans in the UnitTestApp, a UnitTestApp_TestPlan_Full which is the Default test plan and can be run in Xcode, and a UnitTestApp_TestPlan_Reduced with concurrency tests disabled that is run by the CI script.

Other Misc Changes

  • Move concurrency tests into 1 file
  • Add some more "waits" related fixes in tests
  • Fix bug that didn't clear user defaults between tests

Motivation

Fix flaky tests so the CI can be meaningful.

Scope

Tests

Testing

Unit testing

  • See PR description, since PR is about tests.
  • Ran CI 9 times in a row

Manual testing

Minimal

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

@nan-li nan-li changed the base branch from main to fix/user_executor_starts_first July 29, 2024 20:38
@nan-li nan-li force-pushed the tests_fix_flaky_tests branch 7 times, most recently from ce22a6c to b70faf7 Compare July 31, 2024 22:41
* Add new framework called `OneSignalOSCoreMocks ` to access the Operation Repo and reset it between tests.
* Part of the cause for flaky tests is state carrying over in the Operation Repo singleton. For example, Deltas that have not been flushed yet from the preceding test are flushed in the next one.
* These tests check the mock client's number of requests executed, but it was counting the user setup request as well (such as Fetch Identity By Subscription). Add a waiting period for this before creating the Live Activities requests)
* An alternative is to set the user and push subscription ID manually without triggering a request to be made.
@nan-li nan-li force-pushed the tests_fix_flaky_tests branch 2 times, most recently from e16237e to 9a28a65 Compare August 1, 2024 03:58
* Instead of resetting after tests, reset before tests for cleanest starting state
* Currently the Operation Repo is a singleton and shared between tests, so a pending flush could be on the DispatchQueue from a previous test and it will flush unexpectedly.
* This test tests combining operations and intentionally increases the `pollIntervalMilliseconds` to `300`, but a previous flush could interrupt this.
* The UnitTestApp which runs all tests now has 2 test plans named "Full" and "Reduced"
* The default is the Full test
* We may want to disable certain long-running tests from running in the CI under the Reduced plan
* There are no tests disabled in this commit, only adding 2 identical test plans
* Under the Reduced Test Plan, concurrency tests are disabled.
* These tests spam calls and can cause carry over to following tests
* Fix bug that made the helper method to clear user defaults between tests return early.
@nan-li nan-li changed the title [wip] Fix flaky tests [tests] Fix flaky tests Aug 1, 2024
@nan-li nan-li requested a review from emawby August 1, 2024 18:08
@emawby emawby self-assigned this Aug 1, 2024
@nan-li
Copy link
Contributor Author

nan-li commented Aug 1, 2024

After 9 consecutive CI successful runs, the 10th failed on a test I am pushing a commit for.
Re-running CIs.

NVM, that commit made it worse, removed it.

Copy link
Contributor

@emawby emawby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Base automatically changed from fix/user_executor_starts_first to main August 2, 2024 00:22
@nan-li nan-li merged commit d63855d into main Aug 6, 2024
3 of 4 checks passed
@nan-li nan-li deleted the tests_fix_flaky_tests branch August 6, 2024 17:28
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.

2 participants