Add UI tests to check the ability to scroll reader content and interact with the items #17536
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -20,6 +20,50 @@ public class ReaderScreen: ScreenObject { | |||
| ) | ||||
| } | ||||
|
|
||||
| public func openTheLastPostInApp() { | ||||
| getTheLastPost().tap() | ||||
| } | ||||
|
|
||||
| public func openTheLastPostInSafari() { | ||||
| getTheLastPost().buttons["More"].tap() | ||||
| app.buttons["Visit"].tap() | ||||
| } | ||||
|
|
||||
| public func getTheLastPost() -> XCUIElement { | ||||
| let postPosition = app.cells.count - 1 | ||||
| let post = app.cells.element(boundBy: postPosition) | ||||
|
There was a problem hiding this comment. Out of curiosity, did you try the There was a problem hiding this comment. I haven't tried it, probably because I started writing a more generic function to get the Nth post but then I realized I was doing premature optimization and also affecting the test readability. I'll use There was a problem hiding this comment. Yeah, no stress. I was just curious. There are good utilities in our XCUITestHelpers library, but one needs to know about them first, and we don't have an easy way to discover them. I guess it's just like with any other library: We get familiar with them by using them and we can all help each other out by sharing suggestions 😄 By the way: +1 for avoiding premature optimization. At the same time, if you stumble upon something that's a bit over-engineered for the current use case but looks general and something that could be useful in other tests feel free to implement it that way and extracting it into XCUITestHelpers as a followup 👍 |
||||
|
|
||||
| scrollDownUntilElementIsHittable(element: post) | ||||
| return post | ||||
| } | ||||
|
|
||||
| private func scrollDownUntilElementIsHittable(element: XCUIElement) { | ||||
|
There was a problem hiding this comment. This is a nice method. Probably better than the I might steal it in the future 😄 There was a problem hiding this comment. I assume (I didn't test this) that this method is designed to scroll and wait for hittability of the last element only, because swiping up with fast velocity will probably will make it not suitable for the ones in the middle; and because it's called by If this is so, what do you think of renaming the method to reflect this? |
||||
| var loopCount = 0 | ||||
| while !element.waitForIsHittable(timeout: 3) && loopCount < 10 { | ||||
| loopCount += 1 | ||||
| app.swipeUp(velocity: .fast) | ||||
| print(loopCount) | ||||
|
There was a problem hiding this comment. This looks like a debug statement. Did you intend to leave it in here? If not:
Suggested change
There was a problem hiding this comment. 🤦 Yeah, don't tell my mother. |
||||
| } | ||||
| } | ||||
|
|
||||
| public func postContentEquals(expected: String) -> Bool { | ||||
tiagomar marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
| let equalsPostContent = NSPredicate(format: "label == %@", expected) | ||||
| let isPostContentEqual = app.staticTexts.element(matching: equalsPostContent).waitForIsHittable(timeout: 3) | ||||
|
|
||||
| return isPostContentEqual | ||||
| } | ||||
|
|
||||
| public func dismissPost() { | ||||
| let backButton = app.buttons["Back"] | ||||
| let dismissButton = app.buttons["Dismiss"] | ||||
|
|
||||
| if dismissButton.isHittable { | ||||
| dismissButton.tap() | ||||
| } else if backButton.isHittable { | ||||
| backButton.tap() | ||||
| } | ||||
|
There was a problem hiding this comment. What would happen in the tests if neither is hittable? There was a problem hiding this comment. The There was a problem hiding this comment. Currently, the method assumes that the reader post will be either opened in a usual way (and then |
||||
| } | ||||
|
|
||||
| public static func isLoaded() -> Bool { | ||||
| (try? ReaderScreen().isLoaded) ?? false | ||||
| } | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import UITestsFoundation | ||
| import XCTest | ||
|
|
||
| class ReaderTests: XCTestCase { | ||
| private var readerScreen: ReaderScreen! | ||
|
|
||
| override func setUpWithError() throws { | ||
| setUpTestSuite() | ||
|
|
||
| _ = try LoginFlow.loginIfNeeded(siteUrl: WPUITestCredentials.testWPcomSiteAddress, email: WPUITestCredentials.testWPcomUserEmail, password: WPUITestCredentials.testWPcomPassword) | ||
tiagomar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| readerScreen = try EditorFlow | ||
| .goToMySiteScreen() | ||
| .tabBar.goToReaderScreen() | ||
| } | ||
|
|
||
| override func tearDownWithError() throws { | ||
| takeScreenshotOfFailedTest() | ||
| if readerScreen != nil && !TabNavComponent.isVisible() { | ||
|
There was a problem hiding this comment. Just thinking aloud: as long as There was a problem hiding this comment.
The idea was to have it more generic but you are right about the |
||
| readerScreen.dismissPost() | ||
| } | ||
| try LoginFlow.logoutIfNeeded() | ||
| try super.tearDownWithError() | ||
|
There was a problem hiding this comment. It should not be necessary to call There was a problem hiding this comment. No, I have just followed what all the other tests were doing. I did try removing the It runs just fine when removing the whole line but I would be more comfortable removing it if I knew why it was added in first place. There was a problem hiding this comment. Ouch! My suggestions was not clear, sorry 😳 "to call If you updated to - try super.tearDownWithError()
+ try tearDownWithError()it will indeed got into an infinite loop. ♻️
Fair enough. Those methods are (should be?) empty in Feel free to ignore this. I plan to audit the test suite for this kind of stuff during my HACK week. 👍 #17569 |
||
| } | ||
|
|
||
| let expectedPostContent = "Aenean vehicula nunc in sapien rutrum, nec vehicula enim iaculis. Aenean vehicula nunc in sapien rutrum, nec vehicula enim iaculis. Proin dictum non ligula aliquam varius. Nam ornare accumsan ante, sollicitudin bibendum erat bibendum nec. Aenean vehicula nunc in sapien rutrum, nec vehicula enim iaculis." | ||
|
|
||
| func testViewPostInApp() { | ||
| readerScreen.openTheLastPostInApp() | ||
| XCTAssert(readerScreen.postContentEquals(expected: expectedPostContent)) | ||
| } | ||
|
|
||
| func testViewPostInSafari() { | ||
| readerScreen.openTheLastPostInSafari() | ||
| XCTAssert(readerScreen.postContentEquals(expected: expectedPostContent)) | ||
| } | ||
| } | ||
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.
Nitpick: Function names don't usually have articles in them. I don't see it spelled out clearly in the Swift API Guidelines and, thinking about it, I never really read this anywhere, but, based on all the code I've read over time, I think my assertion is accurate.
Also, I think it's safe to omit "in app" from the name. I'm guessing you did it to differentiate it from the method below that uses Safari. But, IMHO, if nothing else is specified it's safe to assume the action will occur in the app.
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.
Yeah, the idea was really to differentiate it. I was actually a bit uncomfortable with "InApp" because the "InSafari" is somewhat still in app. =P
I'll get rid of the articles. =)