Skip to content

Commit e632cc6

Browse files
jvbrionesjvbriones
authored
refactor(tests): reorganize CI tests scripts and update QA CODEOWNERS file (#29181)
Updates the repo to treat test helper scripts as part of the test suite by moving references from ./scripts/* to ./tests/scripts/* across GitHub Actions (performance E2E workflow), Bitrise E2E workflows, and performance test docs. Expands QA ownership in .github/CODEOWNERS to include tests/scripts/ plus a set of QA-owned workflows, composite actions, and GitHub scripts, and adjusts .eslintignore accordingly. <!-- 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. --> - [x] 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). - [x] 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 - [x] 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`. --> - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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** > Mostly path and ownership/config changes, but they touch GitHub Actions and Bitrise workflows; incorrect paths/ownership patterns could break CI runs or review enforcement. > > **Overview** > Updates CI and docs to treat test helper scripts as part of the test suite by switching multiple references from `./scripts/*` to `./tests/scripts/*` (GitHub Actions performance E2E workflow, Bitrise E2E flows, and performance test READMEs), and aligns a few in-test error/help messages accordingly. > > Expands QA ownership rules in `.github/CODEOWNERS` to cover a broader set of `tests/**` content (including `tests/scripts/`) and adds explicit QA ownership for several E2E/performance-related GitHub workflows/actions/scripts; also tweaks ESLint config/ignore entries to match the new test-script location and remove the `appwright` override coverage. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0c30c7a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 9355fcc commit e632cc6

15 files changed

Lines changed: 59 additions & 26 deletions

‎.eslintignore‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,4 @@ wdio
1616
/app/util/test/testSetup.js
1717
junitProperties.js
1818
/tests/scripts/ai-e2e-tags-selector.ts
19-
scripts/aggregate-performance-reports.mjs
19+
tests/scripts/aggregate-performance-reports.mjs

‎.eslintrc.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ module.exports = {
2222
],
2323
overrides: [
2424
{
25-
files: ['tests/**/*.{js,ts}', 'appwright/**/*.{js,ts}'],
25+
files: ['tests/**/*.{js,ts}'],
2626
extends: ['./tests/framework/.eslintrc.js'],
2727
},
2828
{

‎.github/CODEOWNERS‎

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,46 @@ app/reducers/onboarding @MetaMask/web3auth
301301
app/core/Engine/controllers/gator-permissions-controller @MetaMask/delegation
302302
app/core/Engine/messengers/gator-permissions-controller-messenger @MetaMask/delegation
303303

304-
# QA Team - E2E Framework
305-
tests/api-mocking/ @MetaMask/qa
306-
tests/framework/ @MetaMask/qa
307-
tests/page-objects/ @MetaMask/qa
308-
tests/flows/ @MetaMask/qa
309-
# Note: Test builds (main-test, flask-test) in build/builds.yml are owned by QA team
310-
# but the file itself is protected by mobile-platform for consistency
304+
# QA Team - E2E framework
305+
tests/**/*.md @MetaMask/qa
306+
tests/*.js @MetaMask/qa
307+
tests/*.ts @MetaMask/qa
308+
tests/api-mocking/ @MetaMask/qa
309+
tests/component-view/ @MetaMask/qa
310+
tests/controller-mocking/ @MetaMask/qa
311+
tests/feature-flags/ @MetaMask/qa
312+
tests/flows/ @MetaMask/qa
313+
tests/framework/ @MetaMask/qa
314+
tests/module-mocking/ @MetaMask/qa
315+
tests/page-objects/ @MetaMask/qa
316+
tests/resources/ @MetaMask/qa
317+
tests/scripts/ @MetaMask/qa
318+
tests/seeder/ @MetaMask/qa
319+
tests/tools/ @MetaMask/qa
320+
tests/websocket/ @MetaMask/qa
321+
322+
# QA Team - CI
323+
.github/actions/smart-e2e-selection/ @MetaMask/qa
324+
.github/workflows/needs-e2e-build.yml @MetaMask/qa
325+
.github/workflows/run-e2e-workflow.yml @MetaMask/qa
326+
.github/workflows/run-e2e-api-specs.yml @MetaMask/qa
327+
.github/workflows/run-e2e-regression-tests-android.yml @MetaMask/qa
328+
.github/workflows/run-e2e-regression-tests-ios.yml @MetaMask/qa
329+
.github/workflows/run-e2e-smoke-tests-android.yml @MetaMask/qa
330+
.github/workflows/run-e2e-smoke-tests-android-flask.yml @MetaMask/qa
331+
.github/workflows/run-e2e-smoke-tests-ios.yml @MetaMask/qa
332+
.github/workflows/run-e2e-smoke-tests-ios-flask.yml @MetaMask/qa
333+
.github/workflows/rerun-ci-on-skipped-e2e-labels.yml @MetaMask/qa
334+
.github/workflows/update-e2e-fixtures.yml @MetaMask/qa
335+
.github/workflows/qa-stats.yml @MetaMask/qa
336+
.github/workflows/flaky-test-report.yml @MetaMask/qa
337+
.github/workflows/performance-test-runner.yml @MetaMask/qa
338+
.github/workflows/run-performance-e2e.yml @MetaMask/qa
339+
.github/workflows/run-performance-e2e-experimental.yml @MetaMask/qa
340+
.github/workflows/run-performance-e2e-release.yml @MetaMask/qa
341+
.github/scripts/e2e-*.mjs @MetaMask/qa
342+
.github/scripts/collect-qa-stats.mjs @MetaMask/qa
343+
.github/scripts/generate-regression-slack-summary.mjs @MetaMask/qa
311344

312345
# Co-owned by Swaps and Money Movement teams
313346
app/util/parseAmount.ts @MetaMask/swaps-engineers @MetaMask/money-movement

‎.github/workflows/run-performance-e2e.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ jobs:
344344
env:
345345
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
346346
RN_PLAYGROUND_APK_VERSION: ${{ vars.RN_PLAYGROUND_APK_VERSION || '' }}
347-
run: ./scripts/fetch-rn-playground-apk.sh --output ./tmp/rn-playground.apk
347+
run: ./tests/scripts/fetch-rn-playground-apk.sh --output ./tmp/rn-playground.apk
348348

349349
- name: Upload playground APK to BrowserStack
350350
id: upload-playground
@@ -422,7 +422,7 @@ jobs:
422422
run: |
423423
echo "Processing all test results..."
424424
echo "Running aggregation script..."
425-
node scripts/aggregate-performance-reports.mjs
425+
node tests/scripts/aggregate-performance-reports.mjs
426426
echo "Aggregation completed"
427427
428428
- name: Upload Final Combined Results
@@ -466,7 +466,7 @@ jobs:
466466
run: |
467467
{
468468
echo "summary<<EOF"
469-
./scripts/generate-slack-summary.sh aggregated-reports/summary.json
469+
./tests/scripts/generate-slack-summary.sh aggregated-reports/summary.json
470470
echo "EOF"
471471
} >> "$GITHUB_OUTPUT"
472472

‎bitrise.yml‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,7 @@ workflows:
10331033
inputs:
10341034
- content: |
10351035
#!/usr/bin/env bash
1036-
./scripts/download-android-qa-app.sh
1036+
./tests/scripts/download-android-qa-app.sh
10371037
# APK_PATH is already set by the download script using envman
10381038
build_flask_e2e_android:
10391039
after_run:
@@ -1361,7 +1361,7 @@ workflows:
13611361
IGNORE_BOXLOGS_DEVELOPMENT="true" yarn test:e2e:android:run:qa-release $E2E_TEST_FILE
13621362
elif [ -n "${TEST_SUITE_TAG:-}" ]; then
13631363
echo "[INFO] Running tests matching TEST_SUITE_TAG: $TEST_SUITE_TAG"
1364-
./scripts/run-e2e-tags.sh
1364+
./tests/scripts/run-e2e-tags.sh
13651365
fi
13661366
- custom-test-results-export@1:
13671367
title: Export test results
@@ -1491,7 +1491,7 @@ workflows:
14911491
IGNORE_BOXLOGS_DEVELOPMENT="true" yarn test:e2e:android:$METAMASK_BUILD_TYPE:prod $E2E_TEST_FILE
14921492
elif [ -n "${TEST_SUITE_TAG:-}" ]; then
14931493
echo "[INFO] Running tests matching TEST_SUITE_TAG: $TEST_SUITE_TAG"
1494-
./scripts/run-e2e-tags.sh
1494+
./tests/scripts/run-e2e-tags.sh
14951495
fi
14961496
- custom-test-results-export@1:
14971497
title: Export test results
@@ -1990,14 +1990,14 @@ workflows:
19901990
# if [ "$METAMASK_BUILD_TYPE" = "flask" ]; then
19911991
# IS_TEST='true' METAMASK_BUILD_TYPE='flask' yarn test:e2e:ios:run:qa-release e2e/specs/flask/
19921992
# else
1993-
# ./scripts/run-e2e-tags.sh
1993+
# ./tests/scripts/run-e2e-tags.sh
19941994
# fi
19951995
if [ -n "${E2E_TEST_FILE:-}" ]; then
19961996
echo "[INFO] Running only specified E2E_TEST_FILE(s): $E2E_TEST_FILE"
19971997
IGNORE_BOXLOGS_DEVELOPMENT="true" yarn test:e2e:ios:run:qa-release $E2E_TEST_FILE
19981998
elif [ -n "${TEST_SUITE_TAG:-}" ]; then
19991999
echo "[INFO] Running tests matching TEST_SUITE_TAG: $TEST_SUITE_TAG"
2000-
./scripts/run-e2e-tags.sh
2000+
./tests/scripts/run-e2e-tags.sh
20012001
fi
20022002
- custom-test-results-export@1:
20032003
is_always_run: true
@@ -2142,7 +2142,7 @@ workflows:
21422142
IGNORE_BOXLOGS_DEVELOPMENT="true" yarn test:e2e:ios:$METAMASK_BUILD_TYPE:prod $E2E_TEST_FILE
21432143
elif [ -n "${TEST_SUITE_TAG:-}" ]; then
21442144
echo "[INFO] Running tests matching TEST_SUITE_TAG: $TEST_SUITE_TAG"
2145-
./scripts/run-e2e-tags.sh
2145+
./tests/scripts/run-e2e-tags.sh
21462146
fi
21472147
- custom-test-results-export@1:
21482148
is_always_run: true

‎tests/performance/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ For CI/CD pipelines, the aggregation script combines results from multiple test
624624
### Running Aggregation
625625
626626
```bash
627-
node scripts/aggregate-performance-reports.mjs
627+
node tests/scripts/aggregate-performance-reports.mjs
628628
```
629629
630630
### Generated Files

‎tests/performance/mm-connect/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ connect-monorepo release. You can download it automatically or build locally.
4747

4848
```bash
4949
# From the metamask-mobile root
50-
./scripts/fetch-rn-playground-apk.sh
50+
./tests/scripts/fetch-rn-playground-apk.sh
5151
```
5252

5353
This downloads the latest `rn-playground-<version>.apk` to `./tmp/rn-playground.apk`.
@@ -56,14 +56,14 @@ The test's `beforeAll` hook automatically finds APKs in this location.
5656
To pin a specific version:
5757

5858
```bash
59-
./scripts/fetch-rn-playground-apk.sh --version 17.0.0
59+
./tests/scripts/fetch-rn-playground-apk.sh --version 17.0.0
6060
```
6161

6262
Or set the environment variable:
6363

6464
```bash
6565
export RN_PLAYGROUND_APK_VERSION=17.0.0
66-
./scripts/fetch-rn-playground-apk.sh
66+
./tests/scripts/fetch-rn-playground-apk.sh
6767
```
6868

6969
You can also point to any APK directly:

‎tests/performance/mm-connect/utils.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export function cleanupAdbReverse(port) {
109109

110110
// Candidate paths for the playground release APK, checked in priority order:
111111
// 1. Explicitly set via RN_PLAYGROUND_APK_PATH env var
112-
// 2. Downloaded by scripts/fetch-rn-playground-apk.sh
112+
// 2. Downloaded by tests/scripts/fetch-rn-playground-apk.sh
113113
// 3. Locally built in sibling connect-monorepo
114114
const PLAYGROUND_APK_CANDIDATES = [
115115
process.env.RN_PLAYGROUND_APK_PATH,
@@ -136,7 +136,7 @@ function resolvePlaygroundApkPath() {
136136
(p) => ` - ${path.resolve(process.cwd(), p)}`,
137137
).join('\n') +
138138
'\n\nTo fix this, either:\n' +
139-
' 1. Run: ./scripts/fetch-rn-playground-apk.sh\n' +
139+
' 1. Run: ./tests/scripts/fetch-rn-playground-apk.sh\n' +
140140
' (downloads the latest APK from connect-monorepo GitHub Releases)\n' +
141141
' 2. Build locally:\n' +
142142
' cd connect-monorepo && yarn install && yarn build\n' +

scripts/aggregate-performance-reports.mjs renamed to tests/scripts/aggregate-performance-reports.mjs

File renamed without changes.

scripts/download-android-qa-app.sh renamed to tests/scripts/download-android-qa-app.sh

File renamed without changes.

0 commit comments

Comments
 (0)