Skip to content

Commit df389a9

Browse files
Add flaky-test-detection skill (#61)
## Description Adds a new `flaky-test-detection` skill for MetaMask Mobile that detects, prevents, and fixes flaky Jest unit tests. It provides a review mode for scanning changed test files against known flakiness risk patterns (async, timing, isolation, mock, state), and an audit mode that queries GitHub Actions run history to rank historically flaky tests by failure rate. ## Type of Change - [x] New skill ## Skill Details (if adding a new skill) **Provider Name:** MetaMask **Skill Name:** flaky-test-detection **Brief Description:** Detect, prevent, and fix flaky Jest unit tests in MetaMask Mobile before they reach CI, via PR/diff review against a risk-pattern table and a GitHub Actions history audit mode. ## Checklist - [x] I have read the [CONTRIBUTING.md](../CONTRIBUTING.md) guidelines - [x] My skill follows the [SKILL_TEMPLATE.md](.github/SKILL_TEMPLATE.md) format - [x] I have tested this skill with an AI agent - [x] My skill does not contain any secrets, private keys, or sensitive data - [x] I have added appropriate documentation - [x] My changes don't break existing skills ## Testing Ran the skill against MetaMask Mobile PR diffs and CI run history to validate both review mode (pattern classification) and audit mode (ranked flaky test output). Example result from running on Metamask Mobile repo: --- ### Flaky test audit — Jest unit tests (`ci.yml`, last 150 CI runs) Of 150 recent `ci.yml` runs, 31 failed overall; only 3 failed due to actual Jest failures (the rest were infra: yarn install retries, build issues, etc.). All 3 are confirmed flaky — same commit passed on a separate re-run. | Rank | Test file | Failures | Runs sampled | Rate | Category | |---|---|---:|---:|---:|---| | 1 | `app/components/UI/Tokens/TokenList/TokenListItem/TokenListItem.test.tsx` | 1 | 150 | <1% | J9/J10 (see below) | | 1 | `app/components/Views/confirmations/components/gas/selected-gas-fee-token/selected-gas-fee-token.test.tsx` | 1 | 150 | <1% | J9/J10 | | 1 | `app/components/Views/confirmations/components/info/contract-deployment/contract-deployment.test.tsx` | 1 | 150 | <1% | J9/J10 | | 2 | `app/selectors/assets/balances.test.ts` | 1 | 150 | <1% | J1/J3 | | 2 | `app/components/Views/confirmations/components/confirm/confirm-component.test.tsx` | 1 | 150 | <1% | J1/J3 | | 2 | `app/components/Views/confirmations/hooks/gas/useGasFeeToken.test.ts` | 1 | 150 | <1% | J1/J3 | | 2 | `app/components/Views/confirmations/components/gas/gas-fee-token-toast/gas-fee-token-toast.test.tsx` | 1 | 150 | <1% | J1/J3 | | 2 | `app/components/Views/confirmations/components/gas/gas-fee-token-list-item/gas-fee-token-list-item.test.tsx` | 1 | 150 | <1% | J1/J3 | | 3 | `app/core/Engine/Engine.test.ts` | 1 | 150 | <1% | untriaged (unrelated PR retry) | Individually every file's rate is below the 5% "monitor" threshold, but the two ranked incidents matter more than the per-file rate: **Incident 1** — `feat/stellar/ramp-onramp-support` PR, commit `9953242`: run [28582173232](https://github.com/MetaMask/metamask-mobile/actions/runs/28582173232) failed, run [28582184123](https://github.com/MetaMask/metamask-mobile/actions/runs/28582184123) on the **exact same commit** passed. All 3 unrelated files threw `TypeError: selectAccountByScope is not a function` in the same shard. **Incident 2** — same PR, earlier commit `ff7051b`: run [28577001112](https://github.com/MetaMask/metamask-mobile/actions/runs/28577001112) — 5 unrelated files (selectors, a confirm hook, gas-fee-token components) all threw the identical `TypeError: Cannot read properties of undefined (reading 'some')`. In the same job log there's also `ReferenceError: You are trying to import a file after the Jest environment has been torn down` from `DeeplinkProtocolService.test.ts` right before the cascade starts. Both incidents share a signature: several **unrelated** test files in the same worker/shard fail simultaneously with the identical generic error, and the same commit is green elsewhere. That's not per-file flakiness — it's cross-test contamination inside a shared Jest worker, consistent with skill categories J1 (dangling async work bleeding past test teardown — the "torn down" error is the smoking gun) and J9/J10 (module-level mock state not reset between files in the same worker, causing `selectAccountByScope`/array-selector mocks to be undefined for the next test). Given the low per-file rate, I'd flag this as a worker-isolation issue to dig into (likely a shared mock or setup file) rather than something to fix per test file. Root cause found in `app/core/SDKConnect/SDKDeeplinkProtocol/DeeplinkProtocolService.ts`: ```65:77:app/core/SDKConnect/SDKDeeplinkProtocol/DeeplinkProtocolService.ts public constructor() { if (!this.isInitialized) { this.init() .then(() => { this.isInitialized = true; DevLogger.log('DeeplinkProtocolService:: initialized'); }) .catch((err) => { this.isInitialized = false; Logger.log(err, 'DeeplinkProtocolService:: error initializing'); }); } } ``` The constructor fires an un-awaited `init()` promise chain as a side effect. `DeeplinkProtocolService.test.ts`'s `beforeEach` calls `service = new DeeplinkProtocolService();` 14+ times, and a constructor return value can't be awaited — so every dangling `.then()/.catch()` callback (referencing `DevLogger.log`/`Logger.log`, both `jest.mock()`'d) resolves on a later microtask/macrotask. When the last of these fires after this test file's suite is done and Jest has already torn down its module environment (and possibly moved the worker to the next test file), it throws exactly the observed `ReferenceError: You are trying to import a file after the Jest environment has been torn down`, and can corrupt shared worker state for whatever unrelated test runs next in that worker — matching the cascading `TypeError: Cannot read properties of undefined (reading 'some')` / `selectAccountByScope is not a function` failures across totally unrelated files. This is category J1 (missing await on async side effects) from the skill. **Proposed fix**: add an `afterEach` in `DeeplinkProtocolService.test.ts` that flushes pending promises (using the existing `flushPromises` helper from `app/util/test/utils.js`) so the constructor's dangling `init()` chain always settles before the test/file ends, instead of leaking into the next test file's environment. --- > [!NOTE] > created https://consensyssoftware.atlassian.net/browse/MCWP-699 for the above issue that the skill found. ## Additional Context Includes `references/gh-analysis.md` (GitHub Actions audit procedure) and `repos/metamask-mobile.md` (repo-specific flakiness pattern catalog, J1–J10). fixes https://consensyssoftware.atlassian.net/browse/MCWP-473
1 parent 066e101 commit df389a9

3 files changed

Lines changed: 448 additions & 0 deletions

File tree

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# Flaky Test Audit — GitHub Actions
2+
3+
## Prerequisites
4+
5+
```bash
6+
gh auth status
7+
git remote get-url origin # must contain MetaMask/metamask-mobile
8+
```
9+
10+
## Step 1 — Find the exact unit-test workflow name
11+
12+
```bash
13+
# List recent runs across all workflows — note the name column
14+
gh run list --repo MetaMask/metamask-mobile --limit 50
15+
16+
# Confirm the correct name by filtering (adjust if different from "Unit Tests")
17+
gh run list --repo MetaMask/metamask-mobile --workflow "Unit Tests" --limit 5
18+
```
19+
20+
Use the **exact** workflow name returned by `gh run list` (e.g. `"Unit Tests"`, `"Jest"`, `"CI"`). Substitute it in every `--workflow` flag in the steps below. If the filter returns 0 results, the name is wrong — re-check Step 1.
21+
22+
## Step 2 — Collect IDs of failed runs
23+
24+
Replace `"Unit Tests"` with the name found in Step 1.
25+
26+
```bash
27+
FAILED_RUN_IDS=$(gh run list \
28+
--repo MetaMask/metamask-mobile \
29+
--workflow "Unit Tests" \
30+
--limit 200 \
31+
--json databaseId,conclusion \
32+
--jq '.[] | select(.conclusion == "failure") | .databaseId')
33+
```
34+
35+
## Step 3 — Extract failing test file names
36+
37+
```bash
38+
gh run view <run-id> --repo MetaMask/metamask-mobile --log-failed \
39+
| grep -oE 'FAIL .+\.test\.(ts|tsx|js|jsx)' \
40+
| sed 's/^FAIL //'
41+
```
42+
43+
## Step 4 — Compute per-file failure count and rate
44+
45+
`TOTAL_RUNS` is all runs in the sampled window (not just failures) — the denominator for rate calculation. It is fetched separately at the end of the script.
46+
47+
> Requires Bash 4+ (for `declare -A`). macOS ships Bash 3.2 by default — run this with `brew install bash` or via `gh`'s bundled environment, or invoke as `bash script.sh` with a Bash 4+ binary on `PATH`.
48+
49+
```bash
50+
declare -A fail_count
51+
52+
for run_id in $FAILED_RUN_IDS; do
53+
files=$(gh run view "$run_id" --repo MetaMask/metamask-mobile --log-failed 2>/dev/null \
54+
| grep -oE 'FAIL .+\.test\.(ts|tsx|js|jsx)' \
55+
| sed 's/^FAIL //')
56+
for f in $files; do
57+
fail_count[$f]=$(( ${fail_count[$f]:-0} + 1 ))
58+
done
59+
done
60+
61+
# Count total runs sampled (all runs in the window, not just failures)
62+
TOTAL_RUNS=$(gh run list \
63+
--repo MetaMask/metamask-mobile \
64+
--workflow "Unit Tests" \
65+
--limit 200 \
66+
--json databaseId \
67+
--jq 'length')
68+
69+
# Print: failures total rate% file
70+
for f in "${!fail_count[@]}"; do
71+
count=${fail_count[$f]}
72+
rate=$(echo "scale=2; $count * 100 / $TOTAL_RUNS" | bc)
73+
echo "$count $TOTAL_RUNS ${rate}% $f"
74+
done | sort -rn
75+
```
76+
77+
> Rate limit: ~5,000 gh API requests/hour. Add `sleep 1` between loop iterations for large histories (>100 runs).
78+
79+
## Step 5 — Ranked output format
80+
81+
Columns: rank, test file path, failure count, total runs sampled, failure rate, pattern category (from symptom mapping below).
82+
83+
```
84+
Rank | Test file | Failures | Runs | Rate | Category
85+
-----|-------------------------------------------------------|---------:|-----:|------:|---------
86+
1 | app/components/Views/BankDetails/BankDetails.test.tsx | 12 | 50 | 24 % | J1
87+
2 | app/util/transactions/transactions.test.ts | 9 | 50 | 18 % | J3
88+
```
89+
90+
Assign the category by matching the failure log symptoms in the table below, then open the pattern section in the skill for the fix.
91+
92+
## Symptom → category mapping
93+
94+
| Symptom in failed log | Category |
95+
|---|---|
96+
| `TypeError: terminated` / `SocketError: other side closed` | J1 |
97+
| Timer / timeout mismatch | J2 |
98+
| `toHaveBeenCalledTimes` wrong count | J3 |
99+
| `waitFor` never resolved | J4 or J8 |
100+
| `Cannot read property of undefined` from store selector | J5 |
101+
| Test timed out (5 s default) | J6 |
102+
| Assertion on `Date` or random value fails | J7 |
103+
| Test hangs indefinitely under fake timers | J8 |
104+
| Test passes in isolation but fails in suite | J9 |
105+
| Unexpected function called / wrong return value | J10 |
106+
107+
## Thresholds
108+
109+
| Rate | Action |
110+
|------|--------|
111+
| ≥ 10 % | Actively flaky — fix before merging new tests in that file |
112+
| 5–10 % | At-risk — review for patterns in its category |
113+
| < 5 % | Monitor |

0 commit comments

Comments
 (0)