Skip to content
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

Enable e2eRealApiOrderDetails UI test #13066

Merged
merged 5 commits into from
Dec 10, 2024
Merged

Conversation

irfano
Copy link
Contributor

@irfano irfano commented Dec 4, 2024

Description

OrdersRealAPi.e2eRealApiOrderDetails() was ignored before since it was failing. This test was fixed in #12963 and #12906 before and this PR removes Ignore annotation from it. I also removed @Retry(numberOfTimes = 1) from it since it's not flaky.

Testing information

Run OrdersRealAPi.e2eRealApiOrderDetails() a couple times ti ensure it's passing every time.

The tests that have been performed

Ran OrdersRealAPi.e2eRealApiOrderDetails() a couple of times.

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@irfano irfano added the category: ui tests Related to UI testing. label Dec 4, 2024
@irfano irfano added this to the 21.3 milestone Dec 4, 2024
@irfano irfano requested a review from a team as a code owner December 4, 2024 14:49
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 4, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commitac5537e
Direct Downloadwoocommerce-wear-prototype-build-pr13066-ac5537e.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 4, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commitac5537e
Direct Downloadwoocommerce-prototype-build-pr13066-ac5537e.apk

@hafizrahman hafizrahman self-assigned this Dec 5, 2024
Copy link
Contributor

@hafizrahman hafizrahman left a comment

Choose a reason for hiding this comment

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

I manually re-run the test five times both on a phone simulator API 35, and they all pass fine.

However on a tablet simulator (Pixel tablet API 32) it keeps failing. I'm sending you the recording in Slack p1733375323312509-slack-C03L1NF1EA3

Aside:
I was curious if there's a way to do this repeat tests automatically, because in Xcode there is such an option:

image
(source)

I found an option in Android Studio but its only for JUnit tests:
Screenshot 2024-12-05 at 11 53 50

We only fill the search bar in the e2eRealApiOrdersSearch() function, and it already clears the search bar after filling it. Therefore, it is unnecessary to clear it again in the After function.
@irfano
Copy link
Contributor Author

irfano commented Dec 5, 2024

Good catch on the tablet issue, Hafiz! I've implemented some fixes:

  • Removed clearing search bar in the After function since it was unnecessary and was causing failures on tablets. 374bd26
  • Updated assertOrderId(). Previously, it relied on R.id.toolbar, but on tablets, two R.id.toolbar instances were visible. I've updated it to use a more specific matcher to identify the detail screen's toolbar. f64820c
  • Updated goBackToOrdersScreen(). If the order list is already open, it won't press back. Because on tablets, the order list is displayed alongside the detail screen. ac5537e

Please also test ScreenshotTest and OrdersUITest to ensure these changes don't break them. Note that these tests might still fail on tablets, but they are already failing on the trunk. Fixing those failures is outside the scope of this PR.

I was curious if there's a way to do this repeat tests automatically

I’m not sure if it’s possible; I always run it manually multiple times.

@irfano irfano requested a review from hafizrahman December 5, 2024 21:41
Copy link
Contributor

@hafizrahman hafizrahman 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 the update. Tested the new update a few times in tablet and they're running nicely.

Please also test ScreenshotTest and OrdersUITest to ensure these changes don't break them. Note that these tests might still fail on tablets, but they are already failing on the trunk. Fixing those failures is outside the scope of this PR.

Got it. I tested these on phones and they're going fine, I believe that's OK for now.

@hafizrahman hafizrahman merged commit d638711 into trunk Dec 10, 2024
15 checks passed
@hafizrahman hafizrahman deleted the enable-e2erealapiorderdetails branch December 10, 2024 07:49
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.

3 participants