diff --git a/eng/tools/rush-runner/helpers.js b/eng/tools/rush-runner/helpers.js new file mode 100644 index 000000000000..9b07f31ed6d4 --- /dev/null +++ b/eng/tools/rush-runner/helpers.js @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// @ts-check + +import * as fs from "node:fs"; +import * as path from "node:path"; + +/** @type {Record<"core"|"test-utils"|"identity", string[]>} */ +export const reducedDependencyTestMatrix = { + core: [ + "@azure-rest/synapse-access-control", + "@azure/arm-resources", + "@azure/identity", + "@azure/service-bus", + "@azure/template", + ], + "test-utils": [ + "@azure-tests/perf-storage-blob", + "@azure/arm-eventgrid", + "@azure/ai-text-analytics", + "@azure/identity", + "@azure/template", + ], + identity: [ + "@azure-tests/perf-storage-blob", + "@azure/ai-text-analytics", + "@azure/arm-resources", + "@azure/identity-cache-persistence", + "@azure/identity-vscode", + "@azure/storage-blob", + "@azure/template", + ], +}; + +/** @type {string[]} */ +const restrictedToPackages = [ + "@azure/abort-controller", + "@azure/core-amqp", + "@azure/core-auth", + "@azure/core-client", + "@azure/core-http-compat", + "@azure/core-lro", + "@azure/core-paging", + "@azure/core-rest-pipeline", + "@azure/core-sse", + "@azure/core-tracing", + "@azure/core-util", + "@azure/core-xml", + "@azure/logger", + "@azure-rest/core-client", + "@typespec/ts-http-runtime", + "@azure/identity", + "@azure/arm-resources", + "@azure-tools/test-perf", + "@azure-tools/test-recorder", + "@azure-tools/test-credential", + "@azure-tools/test-utils", + "@azure-tools/test-utils-vitest", +]; + +/** + * Helper function that determines the rush command flag to use based on each individual package name for the 'build' check. + * + * If the targeted package is one of the restricted packages with a ton of dependents, we only want to run that package + * and not all of its dependents. + * @param {string[]} packageNames - An array of strings containing the packages names to run the action on. + * @param {string[]} actionComponents - An array of strings containing the packages names to run the action on. + */ +export const getDirectionMappedPackages = (packageNames, actionComponents) => { + const mappedPackages = []; + + for (const packageName of packageNames) { + // Build command without any additional option should build the project and downstream + // If service is configured to run only a set of downstream projects then build all projects leading to them to support testing + // If the package is a core package, azure-identity or arm-resources then build only the package, + // otherwise build the package and all its dependents + var rushCommandFlag = "--impacted-by"; + + if (restrictedToPackages.includes(packageName)) { + // if this is one of our restricted packages with a ton of deps, make it targeted + // as including all dependents will be too much + rushCommandFlag = "--to"; + } else if (actionComponents.length == 1) { + // else we are building the project and its dependents + rushCommandFlag = "--from"; + } + + mappedPackages.push([rushCommandFlag, packageName]); + } + + return mappedPackages; +}; + +/** + * Returns an array of full paths to package.json files under a directory + * + * @param {string} searchDir - directory to search + */ +const getPackageJSONs = (searchDir) => { + // This gets all the directories with package.json at the `sdk//` level excluding "arm-" packages + const sdkDirectories = fs + .readdirSync(searchDir) + .map((f) => path.join(searchDir, f, "package.json")); // turn potential directory names into package.json paths + + // This gets all the directories with package.json at the `sdk///perf-tests` level excluding "-track-1" perf test packages + let perfTestDirectories = []; + const searchPerfTestDir = path.join(searchDir, "perf-tests"); + if (fs.existsSync(searchPerfTestDir)) { + perfTestDirectories = fs + .readdirSync(searchPerfTestDir) + .filter((f) => !f.endsWith("-track-1")) // exclude libraries ending with "-track-1" (perf test projects) + .map((f) => path.join(searchPerfTestDir, f, "package.json")); // turn potential directory names into package.json paths + } + + return sdkDirectories.concat(perfTestDirectories).filter((f) => fs.existsSync(f)); // only keep paths for files that actually exist +}; + +/** + * Returns package names and package dirs arrays + * + * @param {string} baseDir - + * @param {string[]} serviceDirs - + * @param {string} artifactNames - + */ +export const getServicePackages = (baseDir, serviceDirs, artifactNames) => { + const packageNames = []; + const packageDirs = []; + let validSdkTypes = ["client", "mgmt", "perf-test", "utility"]; // valid "sdk-type"s that we are looking for, to be able to apply rush-runner jobs on + const artifacts = artifactNames.split(","); + for (const serviceDir of serviceDirs) { + const searchDir = path.resolve(path.join(baseDir, "sdk", serviceDir)); + const packageJSONs = getPackageJSONs(searchDir); + for (const filePath of packageJSONs) { + const contents = JSON.parse(fs.readFileSync(filePath, "utf8")); + const artifactName = contents.name.replace("@", "").replace("/", "-"); + if ( + validSdkTypes.includes(contents["sdk-type"]) && + (artifactNames.length === 0 || artifacts.includes(artifactName)) + ) { + packageNames.push(contents.name); + packageDirs.push(path.dirname(filePath)); + } + } + } + return {packageNames, packageDirs}; +}; diff --git a/eng/tools/rush-runner/index.js b/eng/tools/rush-runner/index.js index cb76c449818e..b394e044ecab 100644 --- a/eng/tools/rush-runner/index.js +++ b/eng/tools/rush-runner/index.js @@ -3,37 +3,14 @@ // @ts-check -import * as fs from "node:fs"; import * as path from "node:path"; import * as process from "node:process"; import { spawnSync } from "node:child_process"; - -/** @type {Record<"core"|"test-utils"|"identity", string[]>} */ -const reducedDependencyTestMatrix = { - core: [ - "@azure-rest/synapse-access-control", - "@azure/arm-resources", - "@azure/identity", - "@azure/service-bus", - "@azure/template", - ], - "test-utils": [ - "@azure-tests/perf-storage-blob", - "@azure/arm-eventgrid", - "@azure/ai-text-analytics", - "@azure/identity", - "@azure/template", - ], - identity: [ - "@azure-tests/perf-storage-blob", - "@azure/ai-text-analytics", - "@azure/arm-resources", - "@azure/identity-cache-persistence", - "@azure/identity-vscode", - "@azure/storage-blob", - "@azure/template", - ], -}; +import { + getDirectionMappedPackages, + getServicePackages, + reducedDependencyTestMatrix, +} from "./helpers.js"; const parseArgs = () => { if ( @@ -69,160 +46,81 @@ const parseArgs = () => { } else { if (arg && arg !== "*") { // exclude empty value and special value "*" meaning all libraries - arg.split(" ").forEach((serviceDirectory) => services.push(serviceDirectory)); + services.push(...arg.split(" ")); } } } - return [baseDir, action, services, flags, artifactNames]; -}; - -const getPackageJSONs = (searchDir) => { - // This gets all the directories with package.json at the `sdk//` level excluding "arm-" packages - const sdkDirectories = fs - .readdirSync(searchDir) - .map((f) => path.join(searchDir, f, "package.json")); // turn potential directory names into package.json paths - - // This gets all the directories with package.json at the `sdk///perf-tests` level excluding "-track-1" perf test packages - let perfTestDirectories = []; - const searchPerfTestDir = path.join(searchDir, "perf-tests"); - if (fs.existsSync(searchPerfTestDir)) { - perfTestDirectories = fs - .readdirSync(searchPerfTestDir) - .filter((f) => !f.endsWith("-track-1")) // exclude libraries ending with "-track-1" (perf test projects) - .map((f) => path.join(searchPerfTestDir, f, "package.json")); // turn potential directory names into package.json paths - } - return sdkDirectories.concat(perfTestDirectories).filter((f) => fs.existsSync(f)); // only keep paths for files that actually exist + return { baseDir, action, services, flags, artifactNames }; }; -/** @type {string[]} */ -const restrictedToPackages = [ - "@azure/abort-controller", - "@azure/core-amqp", - "@azure/core-auth", - "@azure/core-client", - "@azure/core-http-compat", - "@azure/core-lro", - "@azure/core-paging", - "@azure/core-rest-pipeline", - "@azure/core-sse", - "@azure/core-tracing", - "@azure/core-util", - "@azure/core-xml", - "@azure/logger", - "@azure-rest/core-client", - "@typespec/ts-http-runtime", - "@azure/identity", - "@azure/arm-resources", - "@azure-tools/test-perf", - "@azure-tools/test-recorder", - "@azure-tools/test-credential", - "@azure-tools/test-utils", - "@azure-tools/test-utils-vitest" -]; - /** - * Helper function that determines the rush command flag to use based on each individual package name for the 'build' check. + * Helper function to spawn NodeJS programs * - * If the targeted package is one of the restricted packages with a ton of dependents, we only want to run that package - * and not all of its dependents. - * @param packageNames string[] An array of strings containing the packages names to run the action on. + * @param {string} cwd - current working directory + * @param {string[]} args - rest of arguments */ -const getDirectionMappedPackages = (packageNames) => { - const mappedPackages = []; - - for (const packageName of packageNames) { - // Build command without any additional option should build the project and downstream - // If service is configured to run only a set of downstream projects then build all projects leading to them to support testing - // If the package is a core package, azure-identity or arm-resources then build only the package, - // otherwise build the package and all its dependents - var rushCommandFlag = "--impacted-by"; - - if (restrictedToPackages.includes(packageName)) { - // if this is one of our restricted packages with a ton of deps, make it targeted - // as including all dependents will be too much - rushCommandFlag = "--to"; - } else if (actionComponents.length == 1) { - // else we are building the project and its dependents - rushCommandFlag = "--from"; - } - - mappedPackages.push([rushCommandFlag, packageName]); - } - - return mappedPackages; -}; - -const getServicePackages = (baseDir, serviceDirs, artifactNames) => { - const packageNames = []; - const packageDirs = []; - let validSdkTypes = ["client", "mgmt", "perf-test", "utility"]; // valid "sdk-type"s that we are looking for, to be able to apply rush-runner jobs on - console.log(`Packages to build: ${artifactNames}`); - const artifacts = artifactNames.split(","); - for (const serviceDir of serviceDirs) { - const searchDir = path.resolve(path.join(baseDir, "sdk", serviceDir)); - const packageJSONs = getPackageJSONs(searchDir); - for (const filePath of packageJSONs) { - const contents = JSON.parse(fs.readFileSync(filePath, "utf8")); - const artifactName = contents.name.replace("@", "").replace("/", "-"); - if ( - validSdkTypes.includes(contents["sdk-type"]) && - (artifactNames.length === 0 || artifacts.includes(artifactName)) - ) { - packageNames.push(contents.name); - packageDirs.push(path.dirname(filePath)); - } - } - } - console.log(`Packages eligible to run rush task: ${packageNames}`); - return [packageNames, packageDirs]; -}; - const spawnNode = (cwd, ...args) => { console.log(`Executing: "node ${args.join(" ")}" in ${cwd}\n\n`); const proc = spawnSync("node", args, { cwd, stdio: "inherit" }); console.log(`\n\nNode process exited with code ${proc.status} `); - if (proc.status !== 0) { - // proc.status will be null if the subprocess terminated due to a signal, which I don't think - // should ever happen, but if it does it's safer to fail. - process.exitCode = proc.status || 1; - } - return proc.status; + return proc.status ?? 1; }; +/** + * flatMap + * + * @param {string[]} arr - string array + * @param {(a: string) => string[]} f - function + */ const flatMap = (arr, f) => { const result = arr.map(f); return [].concat(...result); }; -const [baseDir, action, serviceDirs, rushParams, artifactNames] = parseArgs(); +const { baseDir, action, services: serviceDirs, flags: rushParams, artifactNames } = parseArgs(); const actionComponents = action.toLowerCase().split(":"); -const [packageNames, packageDirs] = getServicePackages(baseDir, serviceDirs, artifactNames); +console.log(`Packages to build: ${artifactNames}`); +const { packageNames, packageDirs } = getServicePackages(baseDir, serviceDirs, artifactNames); +console.log(`Packages eligible to run rush task: ${packageNames}`); /** * Helper function to provide the rush logic that is used frequently below * - * @param direction string which kind of rush tree selector to run (either "--from" or "--to") - * @param packages string[] the names of the packages to run the action on + * @param {string} direction - which kind of rush tree selector to run (either "--from" or "--to") + * @param {string[]} packages - the names of the packages to run the action on */ function rushRunAll(direction, packages) { const params = flatMap(packages, (p) => [direction, p]); - spawnNode(baseDir, "common/scripts/install-run-rush.js", action, ...params, ...rushParams); + return spawnNode(baseDir, "common/scripts/install-run-rush.js", action, ...params, ...rushParams); } /** * Helper function to invoke the rush logic split up by direction. * - * @param packagesWithDirection string[] Any array of strings containing ["direction packageName"...] + * @param {string[][]} packagesWithDirection - Any array of strings containing ["direction packageName"...] */ function rushRunAllWithDirection(packagesWithDirection) { const invocation = packagesWithDirection.flatMap(([direction, packageName]) => [ direction, packageName, ]); + console.dir({ + l: `rushRunAllWithDirection - 1`, + packagesWithDirection, + invocation, + }); spawnNode(baseDir, "common/scripts/install-run-rush.js", action, ...invocation, ...rushParams); + + return spawnNode( + baseDir, + "common/scripts/install-run-rush.js", + action, + ...invocation, + ...rushParams, + ); } /** @@ -240,16 +138,25 @@ function tryGetPkgRelativePath(absolutePath) { : absolutePath.substring(sdkDirectoryPathStartIndex); } -const isReducedTestScopeEnabled = reducedDependencyTestMatrix[serviceDirs]; -if (isReducedTestScopeEnabled) { - // If a service is configured to have reduced test matrix then run rush for those reduced projects - console.log(`Found reduced test matrix configured for ${serviceDirs}.`); - packageNames.push(...reducedDependencyTestMatrix[serviceDirs]); +let isReducedTestScopeEnabled = false; + +for (const dir of serviceDirs) { + if (reducedDependencyTestMatrix[dir]) { + isReducedTestScopeEnabled = true; + // If a service is configured to have reduced test matrix then run rush for those reduced projects + console.log(`Found reduced test matrix configured for ${serviceDirs}.`); + packageNames.push(...reducedDependencyTestMatrix[dir]); + } } -const packagesWithDirection = getDirectionMappedPackages(packageNames); + +const packagesWithDirection = getDirectionMappedPackages(packageNames, actionComponents); const rushx_runner_path = path.join(baseDir, "common/scripts/install-run-rushx.js"); +let exitCode = 0; if (serviceDirs.length === 0) { - spawnNode(baseDir, "common/scripts/install-run-rush.js", action, ...rushParams); + exitCode = spawnNode(baseDir, "common/scripts/install-run-rush.js", action, ...rushParams); + if (exitCode) { + process.exit(exitCode); + } } else { switch (actionComponents[0]) { case "build": @@ -271,15 +178,20 @@ if (serviceDirs.length === 0) { case "lint": for (const packageDir of packageDirs) { - spawnNode(packageDir, rushx_runner_path, action); + const code = spawnNode(packageDir, rushx_runner_path, action); + if (code) { + exitCode = code; + } } break; case "check-format": for (const packageDir of packageDirs) { - if (spawnNode(packageDir, rushx_runner_path, action) !== 0) { + const code = spawnNode(packageDir, rushx_runner_path, action); + if (code !== 0) { console.log( `\nInvoke "rushx format" inside ${tryGetPkgRelativePath(packageDir)} to fix formatting\n`, ); + exitCode = code; } } break; @@ -289,3 +201,5 @@ if (serviceDirs.length === 0) { break; } } + +process.exit(exitCode); diff --git a/eng/tools/rush-runner/package.json b/eng/tools/rush-runner/package.json index fb1819ed0aae..213f849c16dd 100644 --- a/eng/tools/rush-runner/package.json +++ b/eng/tools/rush-runner/package.json @@ -3,6 +3,7 @@ "type": "module", "prettier": "../../../common/tools/eslint-plugin-azure-sdk/prettier.json", "scripts": { + "build": "npm run format && npm run lint && npm run typecheck && npm run test", "format": "prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore index.js \"test/**/*.js\"", "typecheck": "tsc", "lint": "eslint index.js test", diff --git a/eng/tools/rush-runner/test/helper.spec.js b/eng/tools/rush-runner/test/helper.spec.js deleted file mode 100644 index 7b4995559f82..000000000000 --- a/eng/tools/rush-runner/test/helper.spec.js +++ /dev/null @@ -1,7 +0,0 @@ -import { assert, describe, it } from "vitest"; - -describe("helper", () => { - it("should pass", () => { - assert(true); - }); -}); diff --git a/eng/tools/rush-runner/test/helpers.spec.js b/eng/tools/rush-runner/test/helpers.spec.js new file mode 100644 index 000000000000..911222050953 --- /dev/null +++ b/eng/tools/rush-runner/test/helpers.spec.js @@ -0,0 +1,41 @@ +import { assert, describe, it } from "vitest"; +import { getDirectionMappedPackages, reducedDependencyTestMatrix } from "../helpers.js"; + +describe("getDirectionMappedPackages", () => { + it("should use --to reduced core scope for changed core package", () => { + const changed = ["@azure/core-client"]; + const mapped = getDirectionMappedPackages(changed, ["unit-test"]); + + assert.deepStrictEqual( + mapped, + reducedDependencyTestMatrix["core"].map((p) => ["--to", p]), + ); + }); + + it("should use --to reduced core scope for changed test-util package", () => { + const changed = ["@azure-tool/test-utils-vitest"]; + const mapped = getDirectionMappedPackages(changed, ["unit-test"]); + + assert.deepStrictEqual( + mapped, + reducedDependencyTestMatrix["core"].map((p) => ["--to", p]), + ); + }); + + it("should use --from for changed non-core packages", () => { + const changed = ["@azure/app-configuration"]; + const mapped = getDirectionMappedPackages(changed, ["unit-test"]); + + const expected = [["--from", "@azure/app-configuration"]]; + assert.deepStrictEqual(mapped, expected); + }); + + it("should use --to and --from for mixed packages", () => { + const changed = ["@azure/core-rest-pipeline", "@azure/app-configuration"]; + const mapped = getDirectionMappedPackages(changed, ["lint"]); + + const expected = reducedDependencyTestMatrix["core"].map((p) => ["--to", p]); + expected.push(["--from", "@azure/app-configuration"]); + assert.deepStrictEqual(mapped, expected); + }); +});