Add FXIOS-15030 [Relay] Unit test updates#32373
Add FXIOS-15030 [Relay] Unit test updates#32373mattreaganmozilla wants to merge 5 commits intomozilla-mobile:mainfrom
Conversation
|
One of the tests is failing on CI, taking a look |
…an potentially run also which I believe is creating a potential race, but we can simply test that at least one of the updates has occurred which is valid
💪 Quality guardian2 tests files modified. You're a champion of test coverage! 🚀 🧹 Tidy commitJust 3 file(s) touched. Thanks for keeping it clean and review-friendly! ✅ New file code coverageNo new file detected so code coverage gate wasn't ran. Client.app: Coverage: 38.43
Generated by 🚫 Danger Swift against d8dd195 |
FilippoZazzeroni
left a comment
FilippoZazzeroni
left a comment
There was a problem hiding this comment.
Hi @mattreaganmozilla LGTM just added a couple of comments to the tests>
firefox-ios/firefox-ios-tests/Tests/ClientTests/RelayControllerTests.swift
Show resolved
Hide resolved
| let subject = createSubject(accountStatus: .unavailable) | ||
| mockProfile.hasSyncableAccountMock = false | ||
| withExtendedLifetime(subject) { | ||
| wait(RelayController.RelayConstants.postLaunchDelay + 1.0) |
There was a problem hiding this comment.
what if we inject the postLaunchDelay in the controller, so we can pass 0.0 in the tests ? we'd reduce the test time by 5 seconds.
There was a problem hiding this comment.
i'm wondering though if waiting for a time could lead to flaky tests ? what do you think ?
There was a problem hiding this comment.
i'm wondering though if waiting for a time could lead to flaky tests ? what do you think ?
I think this is true to some degree of any test that waits for an async expectation, though we do test those in other areas, and sometimes it's unavoidable. I think the value of the test is worth having, but I can add a comment and if we see any flakiness in CI after these are added I can revisit this and/or remove them.
There was a problem hiding this comment.
what if we inject the postLaunchDelay in the controller, so we can pass 0.0 in the tests ? we'd reduce the test time by 5 seconds.
Yeah I was wondering about this also. Since we both had the same thought I'll update this to be part of the configuration that's injected during init.
There was a problem hiding this comment.
sounds great @mattreaganmozilla then i'm going to approve. Let's see for the flaky tests, if u can add a comment that would be great.
3 participants
📜 Tickets
Jira ticket
💡 Description
A couple updates to Relay unit tests, and fixes some prior nits.
📝 Checklist