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: unified default timeouts #2801

Merged
merged 2 commits into from
Jul 13, 2023
Merged

Tests: unified default timeouts #2801

merged 2 commits into from
Jul 13, 2023

Conversation

NachoSoto
Copy link
Contributor

I did this as part of looking into issues running tests on CI.
Ultimately I found the root cause, but this is a useful refactor to make sure all tests have a shared timeout.

@NachoSoto NachoSoto added the test label Jul 13, 2023
@NachoSoto NachoSoto requested a review from a team July 13, 2023 03:24
@@ -544,25 +544,28 @@ class PurchasesPurchasingTests: BasePurchasesTests {
try XCTUnwrap(self.offeringsFactory.createOfferings(from: [:], data: .mockResponse))
)

var package: Package!
let result: Package? = waitUntilValue { completion in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This replaces toEventually with waitUntilValue to avoid polling.

@@ -88,3 +98,7 @@ func asyncWait(
throw ConditionFailedError()
}
}

// Higher value required to avoid slow CI failing tests.
let defaultTimeout: DispatchTimeInterval = .seconds(2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also increased the default timeout we were using from 1 second to 2, to avoid flaky failures when CircleCI is being slow.

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #2801 (b128fa2) into main (1e12136) will decrease coverage by 0.07%.
The diff coverage is n/a.

❗ Current head b128fa2 differs from pull request most recent head 9adf052. Consider uploading reports for the commit 9adf052 to get more accurate results

@@            Coverage Diff             @@
##             main    #2801      +/-   ##
==========================================
- Coverage   86.51%   86.45%   -0.07%     
==========================================
  Files         214      214              
  Lines       15396    15396              
==========================================
- Hits        13320    13310      -10     
- Misses       2076     2086      +10     

see 3 files with indirect coverage changes

@@ -908,7 +908,7 @@ final class HTTPClientTests: BaseHTTPClientTests<MockETagManager> {
}
}

self.waitForExpectations(timeout: 1)
self.waitForExpectations(timeout: defaultTimeout.seconds * TimeInterval(serialRequests))
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this will increase the timeout a lot right? (up to 20 seconds). Just want to confirm we want to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed anymore thanks to #2802

@NachoSoto NachoSoto enabled auto-merge (squash) July 13, 2023 12:53
@NachoSoto NachoSoto merged commit dd01d49 into main Jul 13, 2023
@NachoSoto NachoSoto deleted the testing-timeouts branch July 13, 2023 13:06
NachoSoto added a commit that referenced this pull request Jul 19, 2023
**This is an automatic release.**

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#2824) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* `MainThreadMonitor`: don't crash if there is no test in progress
(#2838) via NachoSoto (@NachoSoto)
* `CI`: fixed Fastlane APITester lanes (#2836) via NachoSoto
(@NachoSoto)
* `Integration Tests`: workaround Swift runtime crash (#2826) via
NachoSoto (@NachoSoto)
* `@EnsureNonEmptyArrayDecodable` (#2831) via NachoSoto (@NachoSoto)
* `iOS 17`: added tests for simulating cancellations (#2597) via
NachoSoto (@NachoSoto)
* `CI`: make all `Codecov` jobs `informational` (#2828) via NachoSoto
(@NachoSoto)
* `MainThreadMonitor`: check deadlocks only ever N seconds (#2820) via
NachoSoto (@NachoSoto)
* New `@NonEmptyStringDecodable` (#2819) via NachoSoto (@NachoSoto)
* `MockDeviceCache`: avoid using real `UserDefaults` (#2814) via
NachoSoto (@NachoSoto)
* `throwAssertion`: fixed Xcode 15 compilation (#2813) via NachoSoto
(@NachoSoto)
* `CustomEntitlementsComputation`: fixed API testers (#2815) via
NachoSoto (@NachoSoto)
* `PackageTypeTests`: fixed iOS 12 (#2807) via NachoSoto (@NachoSoto)
* `Tests`: avoid race-condition in leak detection (#2806) via NachoSoto
(@NachoSoto)
* Revert "`Unit Tests`: removed leak detection" (#2805) via NachoSoto
(@NachoSoto)
* `PackageType: Codable` implementation (#2797) via NachoSoto
(@NachoSoto)
* `SystemInfo.init` no longer `throws` (#2803) via NachoSoto
(@NachoSoto)
* `Trusted Entitlements`: add support for signing `POST` body (#2753)
via NachoSoto (@NachoSoto)
* `Tests`: unified default timeouts (#2801) via NachoSoto (@NachoSoto)
* `Tests`: removed forced-unwrap (#2799) via NachoSoto (@NachoSoto)
* `Tests`: added missing `super.setUp()` (#2804) via NachoSoto
(@NachoSoto)
* Replaced `FatalErrorUtil` with `Nimble` (#2802) via NachoSoto
(@NachoSoto)
* `Tests`: fixed another flaky test (#2795) via NachoSoto (@NachoSoto)
* `TimingUtil`: improved tests by using `Clock` (#2794) via NachoSoto
(@NachoSoto)
* `IgnoreDecodeErrors`: log decoding error (#2778) via NachoSoto
(@NachoSoto)
* `TestLogHandler`: changed all tests to explicitly deinitialize it
(#2784) via NachoSoto (@NachoSoto)
* `LocalReceiptParserStoreKitTests`: fixed flaky test failure (#2785)
via NachoSoto (@NachoSoto)
* `Unit Tests`: removed leak detection (#2792) via NachoSoto
(@NachoSoto)
* `Tests`: fixed another flaky failure with asynchronous check (#2786)
via NachoSoto (@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
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