From ed60e405e16e6103b4493adc5af41ac3d37b0ee2 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Mon, 6 May 2024 11:57:19 +0200 Subject: [PATCH] feat!: remove --segfault-retry (#5514) --- .github/workflows/ci.yml | 1 - packages/vitest/rollup.config.js | 1 - packages/vitest/src/node/cli/cli-wrapper.ts | 147 -------------------- packages/vitest/vitest.mjs | 2 +- test/config/test/exec-args.test.ts | 12 +- 5 files changed, 2 insertions(+), 161 deletions(-) delete mode 100644 packages/vitest/src/node/cli/cli-wrapper.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index abf5d4ae7a05..0c51e1cec92f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,6 @@ concurrency: cancel-in-progress: true env: - VITEST_SEGFAULT_RETRY: 3 PLAYWRIGHT_BROWSERS_PATH: ${{ github.workspace }}/.cache/ms-playwright CYPRESS_CACHE_FOLDER: ${{ github.workspace }}/.cache/Cypress diff --git a/packages/vitest/rollup.config.js b/packages/vitest/rollup.config.js index 06cd3da01cec..7ecdfac71298 100644 --- a/packages/vitest/rollup.config.js +++ b/packages/vitest/rollup.config.js @@ -19,7 +19,6 @@ const entries = { 'path': 'src/paths.ts', 'index': 'src/index.ts', 'cli': 'src/node/cli.ts', - 'cli-wrapper': 'src/node/cli/cli-wrapper.ts', 'node': 'src/node.ts', 'suite': 'src/suite.ts', 'browser': 'src/browser.ts', diff --git a/packages/vitest/src/node/cli/cli-wrapper.ts b/packages/vitest/src/node/cli/cli-wrapper.ts deleted file mode 100644 index c2b9901efd44..000000000000 --- a/packages/vitest/src/node/cli/cli-wrapper.ts +++ /dev/null @@ -1,147 +0,0 @@ -/* eslint-disable no-console */ -/** - * Wrapper of the CLI with child process to manage segfaults and retries. - */ -import { fileURLToPath } from 'node:url' -import c from 'picocolors' -import { execa } from 'execa' -import { EXIT_CODE_RESTART } from '../../constants' - -const ENTRY = new URL('./cli.js', import.meta.url) - -/** Arguments passed to Node before the script */ -const NODE_ARGS = [ - '--trace-deprecation', - '--experimental-wasm-threads', - '--wasm-atomics-on-non-shared-memory', -] - -interface ErrorDef { - trigger: string - url: string -} - -const SegfaultErrors: ErrorDef[] = [ - { - trigger: 'Check failed: result.second.', - url: 'https://github.com/nodejs/node/issues/43617', - }, - { - trigger: 'FATAL ERROR: v8::FromJust Maybe value is Nothing.', - url: 'https://github.com/vitest-dev/vitest/issues/1191', - }, - { - trigger: 'FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal.', - url: 'https://github.com/nodejs/node/issues/42407', - }, -] - -main() - -async function main() { - // default exit code = 100, as in retries were exhausted - let retries = 0 - const args = process.argv.slice(2) - - if (process.env.VITEST_SEGFAULT_RETRY) { - retries = +process.env.VITEST_SEGFAULT_RETRY - } - else { - for (let i = 0; i < args.length; i++) { - if (args[i].startsWith('--segfault-retry=') || args[i].startsWith('--segfaultRetry=')) { - retries = +args[i].split('=')[1] - break - } - else if ((args[i] === '--segfault-retry' || args[i] === '--segfaultRetry') && args[i + 1]?.match(/^\d+$/)) { - retries = +args[i + 1] - break - } - } - } - - // if not specified, don't run through spawn, - // because it prints stderr messages in the wrong order compared to stdout - if (retries <= 0) { - await import('../cli') - return - } - - const nodeArgs: string[] = [] - const vitestArgs: string[] = [] - - // move node args to the front - for (let i = 0; i < args.length; i++) { - let matched = false - for (const nodeArg of NODE_ARGS) { - if (args[i] === nodeArg || args[i].startsWith(`${nodeArg}=`)) { - matched = true - nodeArgs.push(args[i]) - break - } - } - if (!matched) - vitestArgs.push(args[i]) - } - - retries = Math.max(1, retries || 1) - - for (let i = 1; i <= retries; i++) { - const result = await start(nodeArgs, vitestArgs) - - if (result === 'restart') { - i -= 1 - continue - } - - if (i === 1 && retries === 1) { - console.log(c.yellow(`It seems to be an upstream bug of Node.js. To improve the test stability, -you could pass ${c.bold(c.green('--segfault-retry=3'))} or set env ${c.bold(c.green('VITEST_SEGFAULT_RETRY=3'))} to -have Vitest auto retries on flaky segfaults.\n`)) - } - - if (i !== retries) - console.log(`${c.inverse(c.bold(c.magenta(' Retrying ')))} vitest ${args.join(' ')} ${c.gray(`(${i + 1} of ${retries})`)}`) - } - - // retry out - process.exit(1) -} - -async function start(preArgs: string[], postArgs: string[]) { - const child = execa( - 'node', - [ - ...preArgs, - fileURLToPath(ENTRY), - ...postArgs, - ], - { - reject: false, - stderr: 'pipe', - stdout: 'inherit', - stdin: 'inherit', - env: { - ...process.env, - VITEST_CLI_WRAPPER: 'true', - }, - }, - ) - child.stderr?.pipe(process.stderr) - const { stderr = '' } = await child - - if (child.exitCode === EXIT_CODE_RESTART) - return 'restart' - - for (const error of SegfaultErrors) { - if (stderr.includes(error.trigger)) { - if (process.env.GITHUB_ACTIONS) - console.log(`::warning:: Segmentfault Error Detected: ${error.trigger}\nRefer to ${error.url}`) - const RED_BLOCK = c.inverse(c.red(' ')) - console.log(`\n${c.inverse(c.bold(c.red(' Segmentfault Error Detected ')))}\n${RED_BLOCK} ${c.red(error.trigger)}\n${RED_BLOCK} ${c.red(`Refer to ${error.url}`)}\n`) - return 'error' - } - } - - // no segmentfault found - process.exit(child.exitCode!) -} diff --git a/packages/vitest/vitest.mjs b/packages/vitest/vitest.mjs index b4de875a07e6..02dd4714b903 100755 --- a/packages/vitest/vitest.mjs +++ b/packages/vitest/vitest.mjs @@ -1,2 +1,2 @@ #!/usr/bin/env node -import './dist/cli-wrapper.js' +import './dist/cli.js' diff --git a/test/config/test/exec-args.test.ts b/test/config/test/exec-args.test.ts index ad22f4942172..add2796a5da3 100644 --- a/test/config/test/exec-args.test.ts +++ b/test/config/test/exec-args.test.ts @@ -1,17 +1,7 @@ -import { afterAll, beforeAll, expect, test } from 'vitest' +import { expect, test } from 'vitest' import { execa } from 'execa' import { runVitest } from '../../test-utils' -// VITEST_SEGFAULT_RETRY messes with the node flags, as can be seen in packages/vitest/src/node/cli-wrapper.ts -// so here we remove it to make sure the tests are not affected by it -const ORIGIN_VITEST_SEGFAULT_RETRY = process.env.VITEST_SEGFAULT_RETRY -beforeAll(() => { - delete process.env.VITEST_SEGFAULT_RETRY -}) -afterAll(() => { - process.env.VITEST_SEGFAULT_RETRY = ORIGIN_VITEST_SEGFAULT_RETRY -}) - test.each([ { pool: 'forks', execArgv: ['--hash-seed=1', '--random-seed=1', '--no-opt'] }, { pool: 'threads', execArgv: ['--inspect-brk'] },