From 2359d3d2754a368647548cb70888368b5fd927f2 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 1 Apr 2024 13:15:43 -0700 Subject: [PATCH] Add job to test flakiness of added/changed tests (#63943) To help detect when newly added/changed assertions are flakey this adds a job to our build and test workflow to re-run them 3 times. If the changed test is an E2E test it will re-run in both development and production mode to ensure it's not flakey specifically in one of those modes. Test run with changed test can be seen here https://github.com/vercel/next.js/actions/runs/8511797725/job/23312158523?pr=63943 Closes NEXT-2973 --- .github/workflows/build_and_test.yml | 14 ++- run-tests.js | 6 +- scripts/test-new-tests.mjs | 143 +++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 scripts/test-new-tests.mjs diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 9f6e05aa4b24e..1d77d818d1a48 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -256,6 +256,18 @@ jobs: secrets: inherit + test-new-tests: + name: Test new tests for flakes + needs: ['changes', 'build-native', 'build-next'] + if: ${{ needs.changes.outputs.docs-only == 'false' }} + + uses: ./.github/workflows/build_reusable.yml + with: + afterBuild: node scripts/test-new-tests.mjs + stepName: 'test-new-tests' + + secrets: inherit + test-dev: name: test dev needs: ['changes', 'build-native', 'build-next'] @@ -332,8 +344,6 @@ jobs: needs: ['changes', 'build-native', 'build-next'] if: ${{ needs.changes.outputs.docs-only == 'false' }} - strategy: - fail-fast: false uses: ./.github/workflows/build_reusable.yml with: nodeVersion: 18.17.0 diff --git a/run-tests.js b/run-tests.js index 4bb9acdd36667..32e0a5a441346 100644 --- a/run-tests.js +++ b/run-tests.js @@ -20,6 +20,7 @@ let argv = require('yargs/yargs')(process.argv.slice(2)) .string('test-pattern') .boolean('timings') .boolean('write-timings') + .number('retries') .boolean('debug') .string('g') .alias('g', 'group') @@ -185,8 +186,6 @@ async function getTestTimings() { } async function main() { - let numRetries = DEFAULT_NUM_RETRIES - // Ensure we have the arguments awaited from yargs. argv = await argv @@ -198,8 +197,9 @@ async function main() { group: argv.group ?? false, testPattern: argv.testPattern ?? false, type: argv.type ?? false, + retries: argv.retries ?? DEFAULT_NUM_RETRIES, } - + let numRetries = options.retries const hideOutput = !options.debug let filterTestsBy diff --git a/scripts/test-new-tests.mjs b/scripts/test-new-tests.mjs new file mode 100644 index 0000000000000..2306d478773f8 --- /dev/null +++ b/scripts/test-new-tests.mjs @@ -0,0 +1,143 @@ +// @ts-check +import fs from 'fs/promises' +import execa from 'execa' +import path from 'path' + +async function main() { + let eventData = {} + + /** @type import('execa').Options */ + const EXECA_OPTS = { shell: true } + /** @type import('execa').Options */ + const EXECA_OPTS_STDIO = { ...EXECA_OPTS, stdio: 'inherit' } + + try { + eventData = + JSON.parse( + await fs.readFile(process.env.GITHUB_EVENT_PATH || '', 'utf8') + )['pull_request'] || {} + } catch (_) {} + + // detect changed test files + const branchName = + eventData?.head?.ref || + process.env.GITHUB_REF_NAME || + (await execa('git rev-parse --abbrev-ref HEAD', EXECA_OPTS)).stdout + + const remoteUrl = + eventData?.head?.repo?.full_name || + process.env.GITHUB_REPOSITORY || + (await execa('git remote get-url origin', EXECA_OPTS)).stdout + + const isCanary = + branchName.trim() === 'canary' && remoteUrl.includes('vercel/next.js') + + if (isCanary) { + console.error(`Skipping flake detection for canary`) + return + } + + try { + await execa('git remote set-branches --add origin canary', EXECA_OPTS_STDIO) + await execa('git fetch origin canary --depth=20', EXECA_OPTS_STDIO) + } catch (err) { + console.error(await execa('git remote -v', EXECA_OPTS_STDIO)) + console.error(`Failed to fetch origin/canary`, err) + } + + const changesResult = await execa( + `git diff origin/canary --name-only`, + EXECA_OPTS + ).catch((err) => { + console.error(err) + return { stdout: '', stderr: '' } + }) + console.error( + { + branchName, + remoteUrl, + isCanary, + }, + `\ngit diff:\n${changesResult.stderr}\n${changesResult.stdout}` + ) + const changedFiles = changesResult.stdout.split('\n') + + // run each test 3 times in each test mode (if E2E) with no-retrying + // and if any fail it's flakey + const devTests = [] + const prodTests = [] + + for (let file of changedFiles) { + // normalize slashes + file = file.replace(/\\/g, '/') + const fileExists = await fs + .access(path.join(process.cwd(), file), fs.constants.F_OK) + .then(() => true) + .catch(() => false) + + if (fileExists && file.match(/^test\/.*?\.test\.(js|ts|tsx)$/)) { + if (file.startsWith('test/e2e/')) { + devTests.push(file) + prodTests.push(file) + } else if (file.startsWith('test/prod')) { + prodTests.push(file) + } else if (file.startsWith('test/development')) { + devTests.push(file) + } + } + } + + console.log( + 'Detected tests:', + JSON.stringify( + { + devTests, + prodTests, + }, + null, + 2 + ) + ) + + if (prodTests.length === 0 && devTests.length === 0) { + console.log(`No added/changed tests detected`) + return + } + + const RUN_TESTS_ARGS = ['run-tests.js', '-c', '1', '--retries', '0'] + + async function invokeRunTests({ mode, testFiles }) { + await execa('node', [...RUN_TESTS_ARGS, ...testFiles], { + ...EXECA_OPTS_STDIO, + env: { + ...process.env, + NEXT_TEST_MODE: mode, + }, + }) + } + + if (devTests.length > 0) { + for (let i = 0; i < 3; i++) { + console.log(`\n\nRun ${i + 1} for dev tests`) + await invokeRunTests({ + mode: 'dev', + testFiles: devTests, + }) + } + } + + if (prodTests.length > 0) { + for (let i = 0; i < 3; i++) { + console.log(`\n\nRun ${i + 1} for production tests`) + await invokeRunTests({ + mode: 'start', + testFiles: prodTests, + }) + } + } +} + +main().catch((err) => { + console.error(err) + process.exit(1) +})