👷 parallelize test app builds (#4567)

Author: BenoitZugmeyer

.gitlab-ci.yml

@@ -231,6 +231,7 @@ e2e:
   extends:
     - .base-configuration
     - .test-allowed-branches
+    - .resource-allocation-4-cpus
   interruptible: true
   artifacts:
     when: always

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) {

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<string>()
-  for (const app of appsToBuild) {
-    for (const dep of app.deps ?? []) {
-      if (!built.has(dep)) {
-        buildApp(dep)
-        built.add(dep)
-      }
+  const buildPromises = new Map<string, Promise<void>>()
+
+  function ensureBuild(app: AppConfig): Promise<void> {
+    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<void> {

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<unknown>(undefined)
 
 interface CommandOptions {
   cwd?: string
@@ -12,6 +15,7 @@ interface CommandBuilder {
   withCurrentWorkingDirectory(newCurrentWorkingDirectory: string): CommandBuilder
   withLogs(): CommandBuilder
   run(): string
+  runAsync(): Promise<string>
 }
 
 /**
@@ -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<string, string> | 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<string> {
+      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<string>((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.