Skip to content

Conversation

@irfano
Copy link
Contributor

@irfano irfano commented Nov 22, 2024

Closes: woocommerce/woomobile-private#393

Description

ScreenshotTest was failing when the language is set to Arabic or French. This PR fixes these failures.

Important

Do not merge now, we'll merge this as a beta fix for 21.2.

Steps to reproduce

  1. Set the device language to Arabic (ar).
  2. Run ScreenshotTest.
  3. Set the device language to French (fr-FR).
  4. Run ScreenshotTest.

The tests that have been performed

ScreenshotTest

  • 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 Nov 22, 2024
@irfano irfano requested a review from a team as a code owner November 22, 2024 00:34
import org.hamcrest.Matchers

class SingleOrderScreen : Screen(R.id.orderStatus_subtitle) {
class SingleOrderScreen : Screen(R.id.orderDetail_container) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the language is set to Arabic, the test cannot find orderStatus_subtitle. This is likely because we use a different text view for RTL languages. Using orderDetail_container makes sense, as it is also used in goBackToOrdersScreen().

Copy link
Member

Choose a reason for hiding this comment

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

This makes sense, I think the cause that this used to work and not anymore could be linked to the tablet support and how it changed the screen layout.

Another possibility is to use orderRefreshLayout as it's the root layout of the screen, but I think both serve the same purpose here.

while (currentAttempt < maxAttempts) {
try {
composeTestRule.onNodeWithContentDescription(getTranslatedString(R.string.settings))
composeTestRule.onNodeWithContentDescription(getTranslatedString(R.string.more_menu_button_settings))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

R.string.settings was not the correct string for the button but it was working since we use the same string for settings and more_menu_button_settings for all languages except for French.

@irfano irfano changed the title Fix/screenshottest for different locales Fix ScreenshotTest for different locales Nov 22, 2024
@wpmobilebot
Copy link
Collaborator

📲 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
Commit2e5ee71
Direct Downloadwoocommerce-wear-prototype-build-pr12979-2e5ee71.apk

@wpmobilebot
Copy link
Collaborator

📲 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
Commit2e5ee71
Direct Downloadwoocommerce-prototype-build-pr12979-2e5ee71.apk

@irfano irfano added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 22, 2024
@JorgeMucientes JorgeMucientes self-assigned this Nov 22, 2024
Copy link
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this @irfano, ScreenshotTest ran well for both of the languages.

As per Arabic I tested the first option the system settings displayed. I presume any option should do as the issue was with RTL, so any language that would be RTL should do for the test I guess.

@irfano irfano changed the base branch from trunk to release/21.2 November 25, 2024 20:04
@irfano irfano added this to the 21.2 ❄️ milestone Nov 25, 2024
@irfano irfano removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Nov 25, 2024
@irfano irfano merged commit bf58aee into release/21.2 Nov 25, 2024
21 of 22 checks passed
@irfano irfano deleted the fix/screenshottest-for-different-locales branch November 25, 2024 20:07
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.

5 participants