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

test: Add android detox coverage with new RN upgrade #6384

Merged
merged 278 commits into from
Jul 12, 2023

Conversation

cortisiko
Copy link
Member

@cortisiko cortisiko commented May 13, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

This PR enables Detox testing with Android on the react native upgrade PR. Initially, our test runs were limited to iOS devices, where we relied primarily on TestIDs to identify elements. However, when extending our test coverage to Android, we had to incorporate different detox matchers. To address these minor variations in platforms, we employ a straightforward IF-ELSE statement. We first verify whether the tests are executed on an iOS platform. If so, we utilize the byId matcher approach; otherwise, we resort to the byLabel matcher.

An example of this would be tapping on the confirm button on the transaction confirmation modal:

 if (device.getPlatform() === 'ios') {
      await TestHelpers.waitAndTap(CONFIRM_TRANSACTION_BUTTON_ID);
    } else {
      await TestHelpers.waitAndTapByLabel(CONFIRM_TRANSACTION_BUTTON_ID);
    }

In the above code block, we are verifying whether the current test is running on an iOS device. If it is an iOS device, we perform a tap action on the confirm button using the testID. However, if it is not an iOS device, we tap the confirm button byLabel.

NOTE

  • We have identified an ongoing visibility issue in React Native, which becomes more pronounced when running tests on devices with smaller screens. This issue is currently an open ticket in the react-native repo. Tests failed with a visibility error similar to Test Failed: 15.0sec timeout expired without matching of given matcher: (view has effective visibility <VISIBLE> and view.getGlobalVisibleRect() covers at least <75> percent of the view's area)

    • The recommendation here is to use pixel 5 or 6 devices when running tests.
  • There is currently an issue when running detox on android 12 and 13 (API 32/33) which prevents the tests from running. The issue is, the tap() action is treated like a tapAndHold() action. See the open issue in wix/detox here

    • The recommendation here is to use any android OS version lower than 12. API 31 or lower. Once detox addresses the issue with API 32/33 we will make the necessary adjustments on our end.
  • There are intermitted failures on bitrise due to network latency during the yarn setup step. See [Android] When executing the "yarn setup" command on Bitrise, intermittent failures related to the "Sharp" package occur. #6778

    • If you do encounter this please rerun the build.
  • Detox has an intermittent crash on iOS which looks like:

Signal 11 was raised
(
	0   Detox                               0x0000000105e220f4 +[NSThread(DetoxUtils) dtx_demangledCallStackSymbols] + 36
	1   Detox                               0x0000000105e24d04 __DTXHandleCrash + 440
	2   Detox                               0x0000000105e25334 __DTXHandleSignal + 72
	3   libsystem_platform.dylib            0x00000001bca38560 _sigtramp + 52
	4   JavaScriptCore                      0x0000000108f6c824 WTF::RunLoop::TimerBase::start(WTF::Seconds, bool)::$_1::__invoke(__CFRunLoopTimer*, void*) + 52
	5   DetoxSync                           0x000000013fb5c5fc -[_DTXTimerTrampoline fire:] + 200
	6   DetoxSync                           0x000000013fb432fc _DTXCFTimerTrampoline + 68
	7   CoreFoundation                      0x000000010d198afc __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 28
	8   CoreFoundation                      0x000000010d19870c __CFRunLoopDoTimer + 1000
	9   CoreFoundation                      0x000000010d197c00 __CFRunLoopDoTimers + 324
	10  CoreFoundation                      0x000000010d192018 __CFRunLoopRun + 1868
	11  CoreFoundation                      0x000000010d1913bc CFRunLoopRunSpecific + 572
	12  MetaMask                            0x000000010269ec68 +[RCTCxxBridge runRunLoop] + 736
	13  DetoxSync                           0x000000013fb4b568 swz_runRunLoopThread + 276
	14  Foundation                          0x0000000107c13ca0 __NSThread__start__ + 848
	15  libsystem_pthread.dylib             0x00000001bca444e4 _pthread_start + 116
	16  libsystem_pthread.dylib             0x00000001bca3f6cc thread_start + 8

If you do encounter this error, try kicking off another test run. A short-term solution until this is addressed is to have test retries.

To trigger Android tests

Run this command:
yarn test:e2e:android:debug --testNamePattern="Smoke"

Screenshots/Recordings

I was able to get tests running and passing on my local machine, see comment here: #6384 (comment)

As well as bitrise:

https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8ac3dc28-a4e7-4d17-be6a-7e4ccd244eba
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1a34585a-5f4b-48cc-8730-3282f2b89f93
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/70280740-3d02-45f4-9e6e-b836738e176b
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d4d4cf3a-f6ed-40bf-9f1a-33e8b24a07de

Issue

Progresses #5913

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

joaoloureirop and others added 30 commits March 28, 2023 23:00
- Bump RN to 0.71.6 and update pods
- Add @ethereumjs/util
- Bump metro-react-native-babel-preset
- Bump rn-nodeify
- Bump package.json node engine
- Update Yarn lockfile
disable android keyboard learning feature
@cortisiko cortisiko changed the title Add android detox coverage with new RN upgrade test: Add android detox coverage with new RN upgrade Jul 6, 2023
@socket-security
Copy link

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
detox 20.9.0...20.11.0 None +0/-0 8.26 MB wix.mobile

Cal-L
Cal-L previously approved these changes Jul 11, 2023
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@cortisiko cortisiko requested a review from a team as a code owner July 12, 2023 00:36
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM

@cortisiko cortisiko added E2E and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jul 12, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jul 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@cortisiko cortisiko merged commit 0aeef85 into main Jul 12, 2023
14 checks passed
@cortisiko cortisiko deleted the add-android-detox-coverage branch July 12, 2023 02:18
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2023
@metamaskbot metamaskbot added the release-7.4.0 Issue or pull request that will be included in release 7.4.0 label Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E2E release-7.4.0 Issue or pull request that will be included in release 7.4.0 team-mobile-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.