Skip to content

Commit ab3999d

Browse files
antonisclaude
andcommitted
fix(e2e): Address review feedback
- Add platform-check skip guard to warm-up steps in both e2e-v2 and sample-application workflows to avoid wasting CI time when skipped - Write maestro debug logs to per-flow/per-attempt dirs so failed attempt logs are preserved for debugging - Use path.parse() for flow name extraction - Add empty results guard in cli.mjs - Remove retry logic from sample app maestro.ts to avoid mock server envelope accumulation across retries (retries stay in cli.mjs only) - Revert unrelated expo/app.json formatting change Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 15597f9 commit ab3999d

File tree

5 files changed

+18
-36
lines changed

5 files changed

+18
-36
lines changed

.github/workflows/e2e-v2.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ jobs:
517517
erase_before_boot: false
518518

519519
- name: Warm up iOS simulator
520-
if: ${{ matrix.platform == 'ios' }}
520+
if: ${{ steps.platform-check.outputs.skip != 'true' && matrix.platform == 'ios' }}
521521
run: |
522522
# Tart VMs are very slow right after boot. Launch a stock app so
523523
# that SpringBoard, backboardd, and other system services finish

.github/workflows/sample-application.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ jobs:
362362
erase_before_boot: false
363363

364364
- name: Warm up iOS Simulator
365-
if: ${{ matrix.platform == 'ios' }}
365+
if: ${{ steps.platform-check.outputs.skip != 'true' && matrix.platform == 'ios' }}
366366
run: |
367367
# Tart VMs are very slow right after boot. Launch a stock app so
368368
# that SpringBoard, backboardd, and other system services finish

dev-packages/e2e-tests/cli.mjs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ if (actions.includes('test')) {
302302

303303
try {
304304
for (const flowFile of flowFiles) {
305-
const flowName = flowFile.replace('.yml', '');
305+
const flowName = path.parse(flowFile).name;
306306
let passed = false;
307307

308308
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
@@ -313,7 +313,7 @@ if (actions.includes('test')) {
313313
'test', `maestro/${flowFile}`,
314314
'--env', `APP_ID=${appId}`,
315315
'--env', `SENTRY_AUTH_TOKEN=${sentryAuthToken}`,
316-
'--debug-output', 'maestro-logs',
316+
'--debug-output', `maestro-logs/${flowName}/attempt-${attempt}`,
317317
'--flatten-debug-output',
318318
], {
319319
stdio: 'inherit',
@@ -360,6 +360,11 @@ if (actions.includes('test')) {
360360
if (!passed) failed.push(flowName);
361361
}
362362

363+
if (results.length === 0) {
364+
console.error('No test results collected — flow discovery may have failed');
365+
process.exit(1);
366+
}
367+
363368
if (failed.length > 0) {
364369
console.error(`\n${failed.length}/${results.length} flows failed after ${maxAttempts} attempts: ${failed.join(', ')}`);
365370
process.exit(1);

samples/expo/app.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
"resizeMode": "contain",
1414
"backgroundColor": "#ffffff"
1515
},
16-
"assetBundlePatterns": ["**/*"],
16+
"assetBundlePatterns": [
17+
"**/*"
18+
],
1719
"ios": {
1820
"supportsTablet": true,
1921
"bundleIdentifier": "io.sentry.expo.sample",
@@ -106,4 +108,4 @@
106108
"url": "https://u.expo.dev/00000000-0000-0000-0000-000000000000"
107109
}
108110
}
109-
}
111+
}
Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { spawn } from 'node:child_process';
22
import path from 'node:path';
33

4-
const MAX_RETRIES = 3;
5-
64
/**
7-
* Run a single Maestro test attempt.
5+
* Run a Maestro test and return a promise that resolves when the test is finished.
6+
*
7+
* @param test - The path to the Maestro test file relative to the `e2e` directory.
8+
* @returns A promise that resolves when the test is finished.
89
*/
9-
const runMaestro = (test: string): Promise<void> => {
10+
export const maestro = async (test: string) => {
1011
return new Promise((resolve, reject) => {
1112
const process = spawn('maestro', ['test', test, '--format', 'junit'], {
1213
cwd: path.join(__dirname, '..'),
@@ -21,29 +22,3 @@ const runMaestro = (test: string): Promise<void> => {
2122
});
2223
});
2324
};
24-
25-
/**
26-
* Run a Maestro test with retries to handle transient app crashes on slow CI VMs.
27-
*
28-
* Note: Retries happen at the Maestro flow level. If a failed attempt sends partial
29-
* envelopes to the mock server before crashing, they will accumulate across retries.
30-
* In practice, crashes occur on app launch before any SDK transactions are sent,
31-
* so this does not cause issues with test assertions.
32-
*
33-
* @param test - The path to the Maestro test file relative to the `e2e` directory.
34-
* @returns A promise that resolves when the test passes.
35-
*/
36-
export const maestro = async (test: string) => {
37-
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
38-
try {
39-
await runMaestro(test);
40-
return;
41-
} catch (error) {
42-
if (attempt < MAX_RETRIES) {
43-
console.warn(`Maestro attempt ${attempt}/${MAX_RETRIES} failed, retrying...`);
44-
} else {
45-
throw error;
46-
}
47-
}
48-
}
49-
};

0 commit comments

Comments
 (0)