Skip to content

Conversation

@jostnes
Copy link
Contributor

@jostnes jostnes commented Nov 4, 2022

Description

This PR updates the folder structure for UI tests on the repo to make them easier to follow when writing new tests. Currently, all the tests and screens are in the screenshots folder, I guess it was because the first UI test that was created was the screenshots test, it was all grouped under that folder. Now that we are starting to write more UI tests it doesn't make much sense to keep them all in the screenshot folder anymore.

The ScreenshotTest is also still excluded from the test run in this PR, it seems to be broken (last change was in this PR) but we should look into fixing that soon.

Testing instructions

Tests should still work in CI and local.

Images/gif

Before change:
image

After change:
image

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 4, 2022

You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:

@jostnes jostnes added the category: ui tests Related to UI testing. label Nov 8, 2022
@jostnes jostnes marked this pull request as ready for review November 8, 2022 14:39
@jostnes jostnes requested a review from a team as a code owner November 8, 2022 14:39
Copy link
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.

Thanks for this PR @jostnes - the new structure makes a lot more sense 👍

Since the changes are only about the packages structure, thus affecting imports and excluded package on Buildkite, there's not a lot to review. I can see the tests passing on CI, locally, and the new structure itself is more meaningful :shipit:

The ScreenshotTest is also still excluded from the test run in this PR, it seems to be broken (last change was in wordpress-mobile/WordPress-Android#17063) but we should look into fixing that soon.

The Screenshot tests are not meant to be run on CI the same way as "usual" UI tests do. Firstly, their execution is needed seldom, when the changes in the app appearance are so big that the refresh of screenshots for the Play / App Store is needed. AFAIK, they are usually executed locally by the person responsible for this. Secondly, they are not tests per se, because they contain no explicit assertions, so executing them won't bring much value. This approach is consistent between iOS / Android for WP, WC and Simplenote (at least at the time when I checked last).

@jostnes
Copy link
Contributor Author

jostnes commented Nov 9, 2022

The Screenshot tests are not meant to be run on CI the same way as "usual" UI tests do. Firstly, their execution is needed seldom, when the changes in the app appearance are so big that the refresh of screenshots for the Play / App Store is needed. AFAIK, they are usually executed locally by the person responsible for this. Secondly, they are not tests per se, because they contain no explicit assertions, so executing them won't bring much value. This approach is consistent between iOS / Android for WP, WC and Simplenote (at least at the time when I checked last).

Thanks for the explanation and the review @pachlava! Will keep them excluded from the test run then 👍

@jostnes jostnes merged commit 568fcf3 into trunk Nov 9, 2022
@jostnes jostnes deleted the reorder-screen-objects-and-tests branch November 9, 2022 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: ui tests Related to UI testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants