chore: Tests compatible to iPad - WPB-20998 #3746
Conversation
findms
changed the title
findms
changed the title
Test Results2 362 tests 2 334 ✅ 3m 14s ⏱️ Results for commit 4cd87fd. ♻️ This comment has been updated with latest results. |
johnxnguyen
left a comment
johnxnguyen
left a comment
There was a problem hiding this comment.
Looks good, just one question. Also please try to fill in the PR description with something to help the reviewer understand what he changes are about.
There was a problem hiding this comment.
question: were these changes part of automatic linting?
There was a problem hiding this comment.
Some warnings were flagging, tried resolving them.
netbe
left a comment
netbe
left a comment
There was a problem hiding this comment.
The iPad looks good, though I wonder how this is run on CI: modifying TestPlan configuration, happy to help on that part
| let (_, _) = try await networkService.executeRequest(request) | ||
|
|
||
| try ResponseParser() | ||
| _ = ResponseParser() |
There was a problem hiding this comment.
issue: so here we don't do anything if at least the response is not passed to the parser
There was a problem hiding this comment.
it wasn't returning anything but fixed with parsing.
| text = Text(Strings.Settings.title) | ||
| icon = "gearshape" | ||
| accessibilityLabel = Text(Labels.Settings.description) | ||
| accessibilityIdentifier = "bottomBarSettingsButton" |
There was a problem hiding this comment.
this will be updated accordingly once upcoming Locators.swift will be merged
WireNetwork/Sources/WireNetwork/APIs/Rest/AuthenticationAPI/AuthenticationAPIV0.swift
Show resolved
Hide resolved
| } | ||
|
|
||
| @discardableResult | ||
| func onPad(_ block: (() -> Void)? = nil) -> Bool { |
There was a problem hiding this comment.
nit-pic: trying to find a better name...
| func onPad(_ block: (() -> Void)? = nil) -> Bool { | |
| func iPadOnly(_ block: (() -> Void)? = nil) -> Bool { |
| if sideBarPanel.exists { sideBarPanel.tap() } | ||
| if allConversations.exists { allConversations.tap() } |
There was a problem hiding this comment.
question: is this formatted ? I would expect to have this on separate lines
There was a problem hiding this comment.
yes inline formatted but I changed it to separate lines.
Yes ..either we change the test plan or if we plan to run only on CI, we can create a separate job with runtime iPad to run only on nightly. We can discuss it in one of the knowledge sharing session once this PR is ready to merge. |
|
|
||
| @discardableResult | ||
| func iPadOnly(_ block: (() -> Void)? = nil) -> Bool { | ||
| guard UIDevice.current.userInterfaceIdiom == .pad else { return false } |
There was a problem hiding this comment.
Btw, checking userInterfaceIdiom is not future proof, since we might allow the iPad app to be resized.
Then the iPad layout can collapse to the iPhone layout and if I'm not mistaken it will still be userInterfaceIdiom == .pad. Actually I will doublecheck this.
findms
force-pushed
the
chore/test-compatibility-to-ipad-WPB-20998
branch
from
ff464b7 to
1cd717d
Compare
Summary
Briefly describe what this PR does.
This PR is about the critical flow tests making it compatible to iPad so that we can run on nightly if all green for ipad as well.
To Achieve this we needed to put some conditions, elements where flow is different from iPhone on iPad.
Testing
Describe how to verify the changes locally. Attach screenshots or reports if relevant.
Ran test locally via Xcode and here are the results:Note: 1 Test is still pending to be fixed due to issue https://wearezeta.atlassian.net/browse/WPB-21169
Additional Information
any more info, add here.