-
-
Notifications
You must be signed in to change notification settings - Fork 360
fix(ci): Fix consistent iOS E2E flakiness on Cirrus Labs runners #5752
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
Changes from 8 commits
944e559
e11b770
06792e1
54daf49
b262610
f7cb890
56f999d
4a309b3
4d9b775
f83dd9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,20 +290,50 @@ if (actions.includes('test')) { | |
| if (!sentryAuthToken) { | ||
| console.log('Skipping maestro test due to unavailable or empty SENTRY_AUTH_TOKEN'); | ||
| } else { | ||
| const maxAttempts = 3; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have mixed feelings about it because it basically is an attempt to fix the issue with flakiness by trying it three times to run the same test. I also wonder if it works considering that the flakiness is consistent + it doesn't seem like it guarantees to work.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point @alwx 👍 |
||
| const maestroDir = path.join(e2eDir, 'maestro'); | ||
| const flowFiles = fs.readdirSync(maestroDir) | ||
| .filter(f => f.endsWith('.yml') && !fs.statSync(path.join(maestroDir, f)).isDirectory()) | ||
| .sort(); | ||
|
|
||
| console.log(`Found ${flowFiles.length} test flows: ${flowFiles.join(', ')}`); | ||
|
|
||
| const results = []; | ||
|
|
||
| try { | ||
| execSync( | ||
| `maestro test maestro \ | ||
| --env=APP_ID="${appId}" \ | ||
| --env=SENTRY_AUTH_TOKEN="${sentryAuthToken}" \ | ||
| --debug-output maestro-logs \ | ||
| --flatten-debug-output`, | ||
| { | ||
| stdio: 'inherit', | ||
| cwd: e2eDir, | ||
| }, | ||
| ); | ||
| for (const flowFile of flowFiles) { | ||
| const flowName = flowFile.replace('.yml', ''); | ||
| let passed = false; | ||
|
|
||
| for (let attempt = 1; attempt <= maxAttempts; attempt++) { | ||
| const label = `[${flowName}] Attempt ${attempt}/${maxAttempts}`; | ||
| console.log(`\n${'='.repeat(60)}\n${label}\n${'='.repeat(60)}`); | ||
| try { | ||
| execFileSync('maestro', [ | ||
| 'test', `maestro/${flowFile}`, | ||
| '--env', `APP_ID=${appId}`, | ||
| '--env', `SENTRY_AUTH_TOKEN=${sentryAuthToken}`, | ||
| '--debug-output', 'maestro-logs', | ||
| '--flatten-debug-output', | ||
| ], { | ||
| stdio: 'inherit', | ||
| cwd: e2eDir, | ||
| }); | ||
| console.log(`${label} — PASSED`); | ||
| passed = true; | ||
| break; | ||
| } catch (error) { | ||
| console.error(`${label} — FAILED`); | ||
| if (attempt < maxAttempts) { | ||
| console.log(`Retrying ${flowName}…`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| results.push({ flowName, passed }); | ||
| } | ||
| } finally { | ||
| // Always redact sensitive data, even if the test fails | ||
| // Always redact sensitive data, even if a test fails | ||
| const redactScript = ` | ||
| if [[ "$(uname)" == "Darwin" ]]; then | ||
| find ./maestro-logs -type f -exec sed -i '' "s/${sentryAuthToken}/[REDACTED]/g" {} + | ||
|
|
@@ -320,5 +350,20 @@ if (actions.includes('test')) { | |
| console.warn('Failed to redact sensitive data from logs:', error.message); | ||
| } | ||
| } | ||
|
|
||
| // Print summary | ||
| console.log(`\n${'='.repeat(60)}\nTest Summary\n${'='.repeat(60)}`); | ||
| const failed = []; | ||
| for (const { flowName, passed } of results) { | ||
| const icon = passed ? 'PASS' : 'FAIL'; | ||
| console.log(` ${icon} ${flowName}`); | ||
| if (!passed) failed.push(flowName); | ||
| } | ||
|
|
||
| if (failed.length > 0) { | ||
| console.error(`\n${failed.length}/${results.length} flows failed after ${maxAttempts} attempts: ${failed.join(', ')}`); | ||
| process.exit(1); | ||
| } | ||
| console.log(`\nAll ${results.length} flows passed.`); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation on https://github.com/getsentry/sentry-react-native/pull/5755/changes#diff-c5263171d4b9aca7af53d70a3ee9f8423c7f4bebd6941e590c5610208f95c8bcR310 is different.
Sleep time is diffferent and we ignore when it fails.
Which pattern should we follow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.