Skip to content

[UI Tests] - Part 1 - Update structure of Login and Editor related screens#21122

Merged
jostnes merged 3 commits intotrunkfrom
test/update-login-editor-screens
Jul 21, 2023
Merged

[UI Tests] - Part 1 - Update structure of Login and Editor related screens#21122
jostnes merged 3 commits intotrunkfrom
test/update-login-editor-screens

Conversation

@jostnes
Copy link
Copy Markdown
Contributor

@jostnes jostnes commented Jul 18, 2023

Description

While working more on the UI tests recently, I noticed multiple screens are structured differently. And when going through some flows (especially login and editor) it took a while for me to understand them individually, even though they aim to do the same thing. While all of the screens work (nothing wrong with the different implementations), I wondered if it was necessary, the main reason it happened was that different people were working on them at different times. There wasn't a standard for how the screens were defined.

To make it easier for contributors (especially first-time contributors to UI tests), I'm updating the screens to follow the same structure (especially for getters, elements, and screen initialization init). While at it, I also did some cleanup to remove unused code and some comments that are no longer relevant on the screens I updated.

This first cleanup is for Login and Editor-related screens. As these two are used in most of the tests Setup() I figured it would be good to combine them.

Testing

CI should be 🟢

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jul 18, 2023

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr21122-f7dec26
Version22.8
Bundle IDcom.jetpack.alpha
Commitf7dec26
App Center Buildjetpack-installable-builds #5434
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Jul 18, 2023

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr21122-f7dec26
Version22.8
Bundle IDorg.wordpress.alpha
Commitf7dec26
App Center BuildWPiOS - One-Offs #6407
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@jostnes jostnes changed the title login and editor screens changes [UI Tests] - Clean up Login and Editor related screens Jul 19, 2023
@jostnes jostnes marked this pull request as ready for review July 19, 2023 04:58
@jostnes jostnes requested a review from a team as a code owner July 19, 2023 04:58
@jostnes jostnes added Testing Unit and UI Tests and Tooling [Type] Task labels Jul 19, 2023
@jostnes jostnes changed the title [UI Tests] - Clean up Login and Editor related screens [UI Tests] - Part 1 - Update structure of Login and Editor related screens Jul 19, 2023
@peril-wordpress-mobile
Copy link
Copy Markdown

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

Copy link
Copy Markdown
Contributor

@pachlava pachlava left a comment

Choose a reason for hiding this comment

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

Kudos for this impressive refactoring, @jostnes

After your changes, the code looks much more uniform, tidy and even shorter in some cases. You've even put extra work into ordering the elements alphabetically. :shipit:

Comment on lines -35 to +49
let actualPostTitle = app.staticTexts["postTitle"].label
let actualSiteAddress = app.staticTexts["siteUrl"].label
let actualPostTitle = postTitle.label
let actualSiteUrl = siteUrl.label

XCTAssertEqual(expectedPostTitle, actualPostTitle, "Post title doesn't match expected title")
XCTAssertEqual(expectedSiteAddress, actualSiteAddress, "Site address doesn't match expected address")
XCTAssertEqual(expectedSiteAddress, actualSiteUrl, "Site address doesn't match expected address")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not saying this is something critical, but with newer styling, this would look even shorter / arguably easier to read:

XCTAssertEqual(expectedPostTitle, postTitle.label, "Post title doesn't match expected title")
XCTAssertEqual(expectedSiteAddress, siteUrl.label, "Site address doesn't match expected address")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, that's the prize for choosing meaningful element names. =)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I'll make the update in Part 2 (after merging the changes from here to that branch). Thank you for taking a look at this @pachlava @tiagomar!

@jostnes jostnes merged commit e27a286 into trunk Jul 21, 2023
@jostnes jostnes deleted the test/update-login-editor-screens branch July 21, 2023 02:43
mokagio added a commit that referenced this pull request Jul 21, 2023
…act-native-0.71.11

I addressed the conflicts in `BlockEditorScreen.swift` by keeping the
new query syntax from
#21122 and the
typing workaround from
#21143
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing Unit and UI Tests and Tooling [Type] Task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants