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

chore: Move E2E tests to Maestro #1905

Closed
wants to merge 16 commits into from
Closed

chore: Move E2E tests to Maestro #1905

wants to merge 16 commits into from

Conversation

tboba
Copy link
Member

@tboba tboba commented Sep 28, 2023

@khagesh
Copy link

khagesh commented Oct 3, 2023

I see that you are also running into same issue of identifying back button. Is there a way we can pass testID or accessibilityIdentifier to back button on Stack header config props?

@khagesh
Copy link

khagesh commented Oct 3, 2023

I added this patch to make maestro pick testID

diff --git a/node_modules/react-native-screens/ios/RNSScreenStackHeaderConfig.mm b/node_modules/react-native-screens/ios/RNSScreenStackHeaderConfig.mm
index a5eb013..0e4cd3c 100644
--- a/node_modules/react-native-screens/ios/RNSScreenStackHeaderConfig.mm
+++ b/node_modules/react-native-screens/ios/RNSScreenStackHeaderConfig.mm
@@ -495,8 +495,11 @@ + (void)updateViewController:(UIViewController *)vc
                                                                              target:nil
                                                                              action:nil];
   [backBarButtonItem setMenuHidden:config.disableBackButtonMenu];
+  // add accessibilityIdentifier to backBarButtonItem
+  [backBarButtonItem setAccessibilityLabel:@"go back"];
 
   if (config.isBackTitleVisible) {
+    [backBarButtonItem setAccessibilityLabel:@"go back"];
     if (config.backTitleFontFamily || config.backTitleFontSize) {
       NSMutableDictionary *attrs = [NSMutableDictionary new];
       NSNumber *size = config.backTitleFontSize ?: @17;
@@ -521,6 +524,7 @@ + (void)updateViewController:(UIViewController *)vc
     // of backButtonTitle
     [backBarButtonItem setTitle:nil];
     prevItem.backButtonTitle = resolvedBackTitle;
+    [backBarButtonItem setAccessibilityLabel:@"go back"];
   }
   prevItem.backBarButtonItem = backBarButtonItem;
 
@@ -579,6 +583,7 @@ + (void)updateViewController:(UIViewController *)vc
         navitem.leftItemsSupplementBackButton = config.backButtonInCustomView;
 #endif
         UIBarButtonItem *buttonItem = [[UIBarButtonItem alloc] initWithCustomView:subview];
+        [buttonItem setAccessibilityLabel:@"go back"];
         navitem.leftBarButtonItem = buttonItem;
         break;
       }

At first, I added accessibilityIdentifier instead of accessibilityLabel, but maestro does not pick accessibilityIdentifier if accessibilityLabel is set. So, I had to change it to accessibilityLabel.
I know this is a bad practice to override accessibilityLabel, but for our purpose we are fine screen readers read "go back". So, if someone wants they can use this patch for maestro tests to use same text for back button on every screen.

@tboba
Copy link
Member Author

tboba commented Oct 5, 2023

Hi @khagesh, I was facing the issue of identifying back button, but I just needed that for pressing back button on iOS - I think that currently we don't need to set accessibility label for that 😄 If you'd like to press back button on iOS, I'd suggest to just tap on the "Back" or set the condition to do that. We're not planning to set the accessibility labels just to make our E2E tests work, but if this patch matches your requirements - keep it up! Patching it locally should be sufficient.

@tboba
Copy link
Member Author

tboba commented Oct 5, 2023

Closing this PR, as currently Maestro doesn't meet our needs and is lacking support of running the tests on runners from Github Actions 🥲
Let's come back to the case of moving our tests to Maestro in the near future when it will just match our requirements.

@tboba tboba closed this Oct 5, 2023
@khagesh
Copy link

khagesh commented Oct 5, 2023

The problem here is that back button's accessibilityLabel is always different for every screen. And hence I wanted to add accessibilityIdentifier on back button. testID from react native transforms to accessibilityIdentifier on iOS. So, adding testID support is not just of rn-screen's own tests. It would help others as well. Also, the reason I patched with accessibilityLabel is because of maestro. It only picks accessibilityLabel if both (label and identifier) are set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants