From 2e3c7f84e09b6f3213e1be208f25e214ab549db6 Mon Sep 17 00:00:00 2001 From: Benoit Zugmeyer Date: Tue, 5 May 2026 15:11:22 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=B7=20parallelize=20test=20app=20build?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `runAsync()` to the `command` builder and use it in `build-test-apps.ts` to build all apps concurrently. Dependencies are resolved via a memoized `ensureBuild()` function that waits for each app's deps before starting it. Git commands are automatically serialized via a mutex to avoid index lock conflicts. Reduces wall-clock build time from ~1m22s to ~37s. --- .gitlab-ci.yml | 1 + eslint-local-rules/secureCommandExecution.js | 2 +- scripts/build/build-test-apps.ts | 51 +++++----- scripts/lib/command.ts | 98 +++++++++++++++++--- 4 files changed, 118 insertions(+), 34 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7475bdac4a..fa2f569e84 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -227,6 +227,7 @@ e2e: extends: - .base-configuration - .test-allowed-branches + - .resource-allocation-4-cpus interruptible: true artifacts: when: always diff --git a/eslint-local-rules/secureCommandExecution.js b/eslint-local-rules/secureCommandExecution.js index 6baf9a721a..628680a71b 100644 --- a/eslint-local-rules/secureCommandExecution.js +++ b/eslint-local-rules/secureCommandExecution.js @@ -50,7 +50,7 @@ function isCommandExecuted(node) { currentMethodCall = currentMethodCall.parent.parent } - return methodCallNames.includes('run') + return methodCallNames.includes('run') || methodCallNames.includes('runAsync') } function isCommandContainsShellCharacters(node) { diff --git a/scripts/build/build-test-apps.ts b/scripts/build/build-test-apps.ts index 863be3af4f..647afda438 100644 --- a/scripts/build/build-test-apps.ts +++ b/scripts/build/build-test-apps.ts @@ -33,7 +33,7 @@ const APPS: AppConfig[] = [ // React Router apps { name: 'react-router-v6-app' }, { name: 'tanstack-router-app' }, - { name: 'react-router-v7-app', builderFn: buildReactRouterv7App }, + { name: 'react-router-v7-app', builderFn: buildReactRouterv7App, deps: ['react-router-v6-app'] }, // browser extensions { name: 'base-extension' }, @@ -79,22 +79,29 @@ runMain(async () => { printLog('Packing packages...') command`yarn run pack`.run() - const built = new Set() - for (const app of appsToBuild) { - for (const dep of app.deps ?? []) { - if (!built.has(dep)) { - buildApp(dep) - built.add(dep) - } + const buildPromises = new Map>() + + function ensureBuild(app: AppConfig): Promise { + let promise = buildPromises.get(app.name) + if (!promise) { + promise = (async () => { + // Ensure all dependencies are built + const dependenciesToBuild = (app.deps ?? []).map((name) => APPS.find((a) => a.name === name)!) + await Promise.all(dependenciesToBuild.map(ensureBuild)) + + if ('builderFn' in app) { + await app.builderFn(app.name, app.options) + } else { + await buildApp(app.name) + } + })() + buildPromises.set(app.name, promise) } - if ('builderFn' in app) { - await app.builderFn(app.name, app.options) - } else { - buildApp(app.name) - } - built.add(app.name) + return promise } + await Promise.all(appsToBuild.map(ensureBuild)) + printLog('Test apps and extensions built successfully.') }) @@ -112,10 +119,10 @@ function showHelpAndExit() { process.exit(0) } -function buildApp(appName: string) { +async function buildApp(appName: string) { const appPath = `test/apps/${appName}` printLog(`Building app at ${appPath}...`) - command`yarn install --no-immutable`.withCurrentWorkingDirectory(appPath).run() + await command`yarn install --no-immutable`.withCurrentWorkingDirectory(appPath).runAsync() // install peer dependencies if any // intent: renovate does not allow to generate local packages before install @@ -126,16 +133,18 @@ function buildApp(appName: string) { for (const [name] of Object.entries(packageJson.peerDependencies)) { const resolution = packageJson.resolutions?.[name] const specifier = resolution ? `${name}@${resolution}` : name - command`yarn add -D ${specifier}`.withCurrentWorkingDirectory(appPath).run() + await command`yarn add -D ${specifier}`.withCurrentWorkingDirectory(appPath).runAsync() } // revert package.json & yarn.lock changes if they are versioned - const areFilesVersioned = command`git ls-files package.json yarn.lock`.withCurrentWorkingDirectory(appPath).run() + const areFilesVersioned = await command`git ls-files package.json yarn.lock` + .withCurrentWorkingDirectory(appPath) + .runAsync() if (areFilesVersioned) { - command`git checkout package.json yarn.lock`.withCurrentWorkingDirectory(appPath).run() + await command`git checkout package.json yarn.lock`.withCurrentWorkingDirectory(appPath).runAsync() } } - command`yarn build`.withCurrentWorkingDirectory(appPath).run() + await command`yarn build`.withCurrentWorkingDirectory(appPath).runAsync() } async function buildReactRouterv7App() { @@ -163,7 +172,7 @@ async function buildReactRouterv7App() { .replace('react-router-v6-app.js', 'react-router-v7-app.js') ) - buildApp('react-router-v7-app') + await buildApp('react-router-v7-app') } async function buildExtension(appName: string, options?: { runAt?: string }): Promise { diff --git a/scripts/lib/command.ts b/scripts/lib/command.ts index 872a614627..4364aaaff4 100644 --- a/scripts/lib/command.ts +++ b/scripts/lib/command.ts @@ -1,5 +1,8 @@ import childProcess from 'node:child_process' -import { printDebug, printError } from './executionUtils.ts' +import { printDebug, printError, printWarning } from './executionUtils.ts' + +// Git operations share a single index lock, so they must not run concurrently. +let gitMutex = Promise.resolve(undefined) interface CommandOptions { cwd?: string @@ -12,6 +15,7 @@ interface CommandBuilder { withCurrentWorkingDirectory(newCurrentWorkingDirectory: string): CommandBuilder withLogs(): CommandBuilder run(): string + runAsync(): Promise } /** @@ -31,6 +35,7 @@ interface CommandBuilder { export function command(...templateArguments: [TemplateStringsArray, ...any[]]): CommandBuilder { const [commandName, ...commandArguments] = parseCommandTemplateArguments(...templateArguments) + const formattedCommand = `${commandName} ${commandArguments.join(' ')}` let input = '' let env: Record | undefined const extraOptions: CommandOptions = {} @@ -57,7 +62,6 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]): }, run(): string { - const formattedCommand = `${commandName} ${commandArguments.join(' ')}` printDebug(`Running command: ${formattedCommand}`) const commandResult = childProcess.spawnSync(commandName, commandArguments, { @@ -68,16 +72,9 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]): }) if (commandResult.status !== 0) { - const formattedStderr = commandResult.stderr ? `\n---- stderr: ----\n${commandResult.stderr}\n----` : '' - const formattedStdout = commandResult.stdout ? `\n---- stdout: ----\n${commandResult.stdout}\n----` : '' - const exitCause = - commandResult.signal !== null - ? ` due to signal ${commandResult.signal}` - : commandResult.status !== null - ? ` with exit status ${commandResult.status}` - : '' - throw new Error(`Command failed${exitCause}: ${formattedCommand}${formattedStderr}${formattedStdout}`, { - cause: commandResult.error, + throw buildCommandError({ + formattedCommand, + ...commandResult, }) } @@ -87,9 +84,86 @@ export function command(...templateArguments: [TemplateStringsArray, ...any[]]): return commandResult.stdout }, + + runAsync(): Promise { + if (extraOptions.stdio === 'inherit') { + printWarning( + `runAsync() ignores withLogs() for command: ${formattedCommand} (running multiple commands concurrently would produce interleaved output)` + ) + } + printDebug(`Running command: ${formattedCommand}`) + + const run = () => + new Promise((resolve, reject) => { + const child = childProcess.spawn(commandName, commandArguments, { + env: { ...process.env, ...env }, + ...extraOptions, + stdio: 'pipe', + }) + + let stdout = '' + let stderr = '' + + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString() + }) + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString() + }) + + child.on('error', (error) => { + reject(buildCommandError({ formattedCommand, status: null, signal: null, stdout, stderr, error })) + }) + child.on('close', (status, signal) => { + if (status !== 0) { + reject(buildCommandError({ formattedCommand, status, signal, stdout, stderr })) + } else { + resolve(stdout) + } + }) + }) + + if (commandName === 'git') { + const result = gitMutex.then(run) + gitMutex = result.then( + () => { + // ignore result + }, + () => { + // ignore exception + } + ) + return result + } + + return run() + }, } } +function buildCommandError({ + formattedCommand, + status, + signal, + stdout, + stderr, + error, +}: { + formattedCommand: string + status: number | null + signal: NodeJS.Signals | null + stdout: string + stderr: string + error?: Error +}): Error { + const formattedStderr = stderr ? `\n---- stderr: ----\n${stderr}\n----` : '' + const formattedStdout = stdout ? `\n---- stdout: ----\n${stdout}\n----` : '' + const exitCause = signal !== null ? ` due to signal ${signal}` : status !== null ? ` with exit status ${status}` : '' + return new Error(`Command failed${exitCause}: ${formattedCommand}${formattedStderr}${formattedStdout}`, { + cause: error, + }) +} + /** * Parse template values passed to the `command` template tag, and return a list of arguments to run * the command.