-
Notifications
You must be signed in to change notification settings - Fork 121
[UI Tests] - Add load stats UI test #8310
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ import XCTest | |
| final class StatsTests: XCTestCase { | ||
|
|
||
| override func setUpWithError() throws { | ||
| try skipTillImplemented() | ||
| continueAfterFailure = false | ||
|
|
||
| // UI tests must launch the application that they test. | ||
|
|
@@ -15,12 +14,14 @@ final class StatsTests: XCTestCase { | |
| try LoginFlow.logInWithWPcom() | ||
| } | ||
|
|
||
| func skipped_test_load_stats_screen() throws { | ||
| func test_load_stats_screen() throws { | ||
| try TabNavComponent().goToMyStoreScreen() | ||
| } | ||
|
|
||
| func skipTillImplemented(file: StaticString = #file, line: UInt = #line) throws { | ||
| try XCTSkipIf(true, | ||
| "Skipping until test is properly implemented", file: file, line: line) | ||
| .verifyTodayStatsLoaded() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might look redundant (and it most probably is), but I had an idea of also adding a function that selects the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm leaving this as is too, i think not adding that extra tap ensures that upon signing in the merchant would land on the expected tab (Today) instead of other tabs |
||
| .goToThisWeekTab() | ||
| .verifyThisWeekStatsLoaded() | ||
| .goToThisMonthTab() | ||
| .verifyThisMonthStatsLoaded() | ||
| .goToThisYearTab() | ||
| .verifyThisYearStatsLoaded() | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT of extending this asserting function with one more check: making sure that the tab of interest is selected? Example of
This Yeartab being selected:(For this you'll also need to provide the second argument with cell ID):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm leaving this as is, for now. thinking of a scenario where the tab is not selected (e.g. if tap didn't work), the image wouldn't change too so i think that extra check isn't necessary for this test to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally 👍 to leave it with no changes, just wanted to extend my thoughts about the reason for proposal: as you say, the test will still fail if tab selection did not work, because of incorrect chart name. My proposal was more about increasing the test coverage (e.g. checking for one field in the person details page VS checking for all fields). Also, it would cover a totally made up scenario, when tab switch animation did not work, but the screen itself is updated with correct chart and data. (just my thoughts behind it, let's leave it as it is now).