From 3be1287e7da6c8971ba9654ed8e89055dfb9adcf Mon Sep 17 00:00:00 2001 From: OJ Kwon <1210596+kwonoj@users.noreply.github.com> Date: Tue, 14 Feb 2023 21:56:08 -0800 Subject: [PATCH] test(integration): fix skip retry count logic (#45930) This is improved followup for https://github.com/vercel/next.js/pull/45914, I realized I applied retry count logic only for the teardown, not for the actual execution. PR changes whole retrycount if predicate matches, also changes minor ergonomics for the turbopack output with custom binary. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --------- --- packages/next/src/build/swc/index.ts | 14 +++++++++++--- packages/next/src/cli/next-dev.ts | 14 ++++++++++---- run-tests.js | 21 ++++++++++++++------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/packages/next/src/build/swc/index.ts b/packages/next/src/build/swc/index.ts index 2e7fe8528f..eb75aa997d 100644 --- a/packages/next/src/build/swc/index.ts +++ b/packages/next/src/build/swc/index.ts @@ -8,6 +8,7 @@ import { getParserOptions } from './options' import { eventSwcLoadFailure } from '../../telemetry/events/swc-load-failure' import { patchIncorrectLockfile } from '../../lib/patch-incorrect-lockfile' import { downloadWasmSwc } from '../../lib/download-wasm-swc' +import { spawn } from 'child_process' const nextVersion = process.env.__NEXT_VERSION as string @@ -403,7 +404,6 @@ function loadNative(isCustomTurbopack = false) { ) return new Promise((resolve, reject) => { - const spawn = require('next/dist/compiled/cross-spawn') const args: any[] = [] Object.entries(devOptions).forEach(([key, value]) => { @@ -422,8 +422,8 @@ function loadNative(isCustomTurbopack = false) { console.warn(`Running turbopack with args: [${args.join(' ')}]`) - const child = spawn(__INTERNAL_CUSTOM_TURBOPACK_BINARY, args, { - stdio: 'inherit', + let child = spawn(__INTERNAL_CUSTOM_TURBOPACK_BINARY, args, { + stdio: 'pipe', env: { ...process.env, }, @@ -442,6 +442,14 @@ function loadNative(isCustomTurbopack = false) { } resolve(0) }) + + process.on('beforeExit', () => { + if (child) { + console.log('Killing turbopack process') + child.kill() + child = null as any + } + }) }) } else if (!!__INTERNAL_CUSTOM_TURBOPACK_BINDINGS) { console.warn( diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index 7ab780ac36..b09ff81019 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -307,7 +307,9 @@ const nextDev: CliCommand = async (argv) => { if (!hasWarningOrError) { thankYouMsg = chalk.dim(thankYouMsg) } - console.log(turbopackGradient + thankYouMsg) + if (!isCustomTurbopack) { + console.log(turbopackGradient + thankYouMsg) + } let feedbackMessage = `Learn more about Next.js v13 and Turbopack: ${chalk.underline( 'https://nextjs.link/with-turbopack' @@ -336,7 +338,7 @@ const nextDev: CliCommand = async (argv) => { )} ` } - if (unsupportedParts) { + if (unsupportedParts && !isCustomTurbopack) { const pkgManager = getPkgManager(dir) console.error( @@ -354,9 +356,10 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit )}\n cd with-turbopack-app\n ${pkgManager} run dev ` ) - console.warn(feedbackMessage) if (!isCustomTurbopack) { + console.warn(feedbackMessage) + process.exit(1) } else { console.warn( @@ -366,7 +369,10 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit ) } } - console.log(feedbackMessage) + + if (!isCustomTurbopack) { + console.log(feedbackMessage) + } return rawNextConfig } diff --git a/run-tests.js b/run-tests.js index 22a731d383..93547d3c0f 100644 --- a/run-tests.js +++ b/run-tests.js @@ -365,6 +365,7 @@ async function main() { const directorySemas = new Map() + const originalRetries = numRetries await Promise.all( testNames.map(async (test) => { const dirName = path.dirname(test) @@ -375,10 +376,22 @@ async function main() { await sema.acquire() let passed = false + const shouldSkipRetries = skipRetryTestManifest.find((t) => + t.includes(test) + ) + const numRetries = shouldSkipRetries ? 0 : originalRetries + if (shouldSkipRetries) { + console.log(`Skipping retry for ${test} due to skipRetryTestManifest`) + } + for (let i = 0; i < numRetries + 1; i++) { try { console.log(`Starting ${test} retry ${i}/${numRetries}`) - const time = await runTest(test, i === numRetries, i > 0) + const time = await runTest( + test, + shouldSkipRetries || i === numRetries, + shouldSkipRetries || i > 0 + ) timings.push({ file: test, time, @@ -390,12 +403,6 @@ async function main() { break } catch (err) { if (i < numRetries) { - if (skipRetryTestManifest.find((t) => t.includes(test))) { - console.log( - `Skipping retry for ${test} due to skipRetryTestManifest` - ) - break - } try { let testDir = path.dirname(path.join(__dirname, test))