Skip to content

Commit 51bbc68

Browse files
fix: Patch detox to handle errors when injecting into WebViews (#29733)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes Android test/build wiring for Detox (dependency resolution, Gradle module inclusion, flavor dimensions), which could break CI/e2e builds even though it shouldn’t affect production runtime. > > **Overview** > Ensures Detox’s Android WebView HTML extraction doesn’t fail hard by wrapping the injected `GET_HTML_SCRIPT` in a `try/catch` and returning a minimal empty HTML document on error. > > Switches the project to consume a Yarn-patched `detox@20.51.0` and builds Detox from source in Gradle (`include ':detox'` + `androidTestImplementation(project(path: ":detox"))`), removing the old `Detox-android` Maven repo usage. Android app config is updated to resolve Detox’s flavor via `missingDimensionStrategy 'detox', 'full'`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 17e05cb. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e72f6bb commit 51bbc68

6 files changed

Lines changed: 83 additions & 7 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
diff --git a/android/detox/src/full/java/com/wix/detox/espresso/hierarchy/ViewHierarchyGenerator.kt b/android/detox/src/full/java/com/wix/detox/espresso/hierarchy/ViewHierarchyGenerator.kt
2+
index 564838a10114b3dd929b0742d1a99ae28b08717f..8e3dbfa23906e2c02b6d44f273a689670c288272 100644
3+
--- a/android/detox/src/full/java/com/wix/detox/espresso/hierarchy/ViewHierarchyGenerator.kt
4+
+++ b/android/detox/src/full/java/com/wix/detox/espresso/hierarchy/ViewHierarchyGenerator.kt
5+
@@ -19,6 +19,7 @@ import kotlin.coroutines.resume
6+
7+
private const val GET_HTML_SCRIPT = """
8+
(function() {
9+
+ try {
10+
const blacklistedTags = ['script', 'style', 'head', 'meta'];
11+
const blackListedTagsSelector = blacklistedTags.join(',');
12+
13+
@@ -39,6 +40,9 @@ private const val GET_HTML_SCRIPT = """
14+
15+
// Return the serialized HTML as a string
16+
return serializedHtml;
17+
+ } catch {
18+
+ return '<html xmlns="http://www.w3.org/1999/xhtml"><body></body></html>';
19+
+ }
20+
})();
21+
"""
22+

android/app/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ android {
191191
versionCode 4532
192192
testBuildType System.getProperty('testBuildType', 'debug')
193193
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
194+
missingDimensionStrategy 'detox', 'full'
194195
manifestPlaceholders.MM_BRANCH_KEY_TEST = "$System.env.MM_BRANCH_KEY_TEST"
195196
manifestPlaceholders.MM_BRANCH_KEY_LIVE = "$System.env.MM_BRANCH_KEY_LIVE"
196197

@@ -423,7 +424,7 @@ dependencies {
423424
}
424425
// Detox's AAR still reflects Espresso UiControllerImpl.eventInjector, but its POM pulls
425426
// espresso-* 3.6.x where that field was removed. Exclude Espresso from Detox and pin 3.5.1.
426-
androidTestImplementation('com.wix:detox:+') {
427+
androidTestImplementation(project(path: ":detox")) {
427428
exclude module: 'protobuf-lite'
428429
exclude group: 'androidx.test.espresso', module: 'espresso-core'
429430
exclude group: 'androidx.test.espresso', module: 'espresso-web'

android/build.gradle

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ buildscript {
2727
}
2828
allprojects {
2929
repositories {
30-
maven {
31-
url("$rootDir/../node_modules/detox/Detox-android")
32-
}
3330
// Notifee repository
3431
maven {
3532
url(new File(['node', '--print', "require.resolve('@notifee/react-native/package.json')"].execute(null, rootDir).text.trim(), '../android/libs'))

android/settings.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,9 @@ includeBuild('../node_modules/react-native') {
5858
}
5959
}
6060

61+
// Build Detox from source to ensure our patches are applied
62+
include ':detox'
63+
project(':detox').projectDir = new File(rootProject.projectDir, '../node_modules/detox/android/detox')
64+
6165
apply from: new File(["node", "--print", "require.resolve('expo/package.json')"].execute(null, rootDir).text.trim(), "../scripts/autolinking.gradle")
6266
useExpoModules()

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@
636636
"chromedriver": "^123.0.1",
637637
"depcheck": "^1.4.7",
638638
"deprecated-react-native-prop-types": "^5.0.0",
639-
"detox": "^20.43.0",
639+
"detox": "patch:detox@npm%3A20.51.0#~/.yarn/patches/detox-npm-20.51.0-3e13b6e309.patch",
640640
"dotenv": "^16.0.3",
641641
"dpdm": "^3.14.0",
642642
"eas-cli": "^12.6.1",

yarn.lock

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26812,7 +26812,7 @@ __metadata:
2681226812
languageName: node
2681326813
linkType: hard
2681426814

26815-
"detox@npm:^20.43.0":
26815+
"detox@npm:20.51.0":
2681626816
version: 20.51.0
2681726817
resolution: "detox@npm:20.51.0"
2681826818
dependencies:
@@ -26864,6 +26864,58 @@ __metadata:
2686426864
languageName: node
2686526865
linkType: hard
2686626866

26867+
"detox@patch:detox@npm%3A20.51.0#~/.yarn/patches/detox-npm-20.51.0-3e13b6e309.patch":
26868+
version: 20.51.0
26869+
resolution: "detox@patch:detox@npm%3A20.51.0#~/.yarn/patches/detox-npm-20.51.0-3e13b6e309.patch::version=20.51.0&hash=12af19"
26870+
dependencies:
26871+
"@wix-pilot/core": "npm:^3.4.2"
26872+
"@wix-pilot/detox": "npm:^1.0.13"
26873+
ajv: "npm:^8.6.3"
26874+
bunyan: "npm:^1.8.12"
26875+
bunyan-debug-stream: "npm:^3.1.0"
26876+
caf: "npm:^15.0.1"
26877+
chalk: "npm:^4.0.0"
26878+
execa: "npm:^5.1.1"
26879+
find-up: "npm:^5.0.0"
26880+
fs-extra: "npm:^11.0.0"
26881+
funpermaproxy: "npm:^1.1.0"
26882+
glob: "npm:^8.0.3"
26883+
ini: "npm:^1.3.4"
26884+
jest-environment-emit: "npm:^1.2.0"
26885+
json-cycle: "npm:^1.3.0"
26886+
lodash: "npm:^4.17.11"
26887+
multi-sort-stream: "npm:^1.0.3"
26888+
multipipe: "npm:^4.0.0"
26889+
node-ipc: "npm:9.2.1"
26890+
promisify-child-process: "npm:^4.1.2"
26891+
proper-lockfile: "npm:^3.0.2"
26892+
resolve-from: "npm:^5.0.0"
26893+
sanitize-filename: "npm:^1.6.1"
26894+
semver: "npm:^7.0.0"
26895+
serialize-error: "npm:^8.0.1"
26896+
shell-quote: "npm:^1.7.2"
26897+
signal-exit: "npm:^3.0.3"
26898+
stream-json: "npm:^1.7.4"
26899+
strip-ansi: "npm:^6.0.1"
26900+
telnet-client: "npm:1.2.8"
26901+
tmp: "npm:^0.2.1"
26902+
trace-event-lib: "npm:^1.3.1"
26903+
which: "npm:^1.3.1"
26904+
ws: "npm:^7.0.0"
26905+
yargs: "npm:^17.0.0"
26906+
yargs-parser: "npm:^21.0.0"
26907+
yargs-unparser: "npm:^2.0.0"
26908+
peerDependencies:
26909+
jest: 30.x.x || 29.x.x || 28.x.x || ^27.2.5
26910+
peerDependenciesMeta:
26911+
jest:
26912+
optional: true
26913+
bin:
26914+
detox: local-cli/cli.js
26915+
checksum: 10/7adee890378e5f2da21ea3fbef49589eb54c12f611b67d862802aa8272368240fcd3f10cf82c9f872c61dd84f364671342ee6a12b5e5f6184778befc7adaf2de
26916+
languageName: node
26917+
linkType: hard
26918+
2686726919
"dicer@npm:^0.3.1":
2686826920
version: 0.3.1
2686926921
resolution: "dicer@npm:0.3.1"
@@ -36095,7 +36147,7 @@ __metadata:
3609536147
dayjs: "npm:^1.11.13"
3609636148
depcheck: "npm:^1.4.7"
3609736149
deprecated-react-native-prop-types: "npm:^5.0.0"
36098-
detox: "npm:^20.43.0"
36150+
detox: "patch:detox@npm%3A20.51.0#~/.yarn/patches/detox-npm-20.51.0-3e13b6e309.patch"
3609936151
dotenv: "npm:^16.0.3"
3610036152
dpdm: "npm:^3.14.0"
3610136153
eas-cli: "npm:^12.6.1"

0 commit comments

Comments
 (0)