Skip to content

Commit

Permalink
Restore assets before unit-tests are run (#32810)
Browse files Browse the repository at this point in the history
## Issue
Fixes #32805
Related PR: #32806

The original issue was that "restoring recordings when the first test is
triggered" takes time, leading to flakily failing unit-test pipelines.
There is a need to have the recordings pre-loaded before the tests are
run in the CI pipeline.

#32806 - Earlier attempts of pre-restoring recordings in CI for playback
tests with retries and increased timeouts to avoid race conditions did
not fully solve the problem. This PR is a follow up of that.

## Description
Employs a new approach to restore assets sequentially using the
test-proxy tool, which is already downloaded by the CI pipeline.
The assets restoration process is applied only to the unit-test pipeline
in playback mode.

Updates to the `rush-runner` tool:

1. Enhanced `rushRunAllWithDirection` to handle test-proxy restore for
packages in CI pipeline.
2. Introduced new helper function: `runTestProxyRestore`.
3. Added a new helper function `spawnNodeWithOutput` to spawn NodeJS
programs and return the output.
4. Introduced `--ci` flag to help with debugging in local; calls `--ci`
flag in the yml scripts for the unit-test commands.

## Example Output
[Pipeline:
https://dev.azure.com/azure-sdk/public/_build/results?buildId=4523710&view=logs&j=45d25fdd-5540-5f10-8daf-622e80647be7&t=a7f7761c-0312-5a29-1d0c-9736ead6db1f&l=60](https://dev.azure.com/azure-sdk/public/_build/results?buildId=4529992&view=results)
  • Loading branch information
HarshaNalluru authored Feb 3, 2025
1 parent 060ec66 commit 2d4563b
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 9 deletions.
4 changes: 2 additions & 2 deletions eng/pipelines/templates/steps/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ steps:
# Option "-p max" ensures parallelism is set to the number of cores on all platforms, which improves build times.
# The default on Windows is "cores - 1" (microsoft/rushstack#436).
- script: |
node eng/tools/rush-runner/index.js unit-test:node $(ChangedServices) -packages "$(ArtifactPackageNames)" --verbose -p max
node eng/tools/rush-runner/index.js unit-test:node $(ChangedServices) -packages "$(ArtifactPackageNames)" --ci --verbose -p max
displayName: "Test libraries"
condition: and(succeeded(),eq(variables['TestType'], 'node'))
# Option "-p max" ensures parallelism is set to the number of cores on all platforms, which improves build times.
# The default on Windows is "cores - 1" (microsoft/rushstack#436).
- script: |
node eng/tools/rush-runner/index.js unit-test:browser $(ChangedServices) -packages "$(ArtifactPackageNames)" --verbose -p max
node eng/tools/rush-runner/index.js unit-test:browser $(ChangedServices) -packages "$(ArtifactPackageNames)" --ci --verbose -p max
displayName: "Test libraries"
condition: and(succeeded(),eq(variables['TestType'], 'browser'))
Expand Down
4 changes: 2 additions & 2 deletions eng/tools/rush-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { executeActions } from "./src/actions.js";
import { parseArgs } from "./src/args.js";

function main() {
const { action, serviceDirs, rushParams, artifactNames } = parseArgs();
exit(executeActions(action, serviceDirs, rushParams, artifactNames));
const { action, serviceDirs, rushParams, artifactNames, ciFlag } = parseArgs();
exit(executeActions(action, serviceDirs, rushParams, artifactNames, ciFlag));
}

main();
3 changes: 2 additions & 1 deletion eng/tools/rush-runner/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
* @param {string} artifactNames - package names to filter to
* @returns
*/
export function executeActions(action, serviceDirs, rushParams, artifactNames) {
export function executeActions(action, serviceDirs, rushParams, artifactNames, ciFlag) {
const actionComponents = action.toLowerCase().split(":");

console.log(`Packages to build: ${artifactNames}`);
Expand All @@ -43,6 +43,7 @@ export function executeActions(action, serviceDirs, rushParams, artifactNames) {
action,
getDirectionMappedPackages(packageNames, action, serviceDirs),
rushParams,
ciFlag
);
break;

Expand Down
9 changes: 7 additions & 2 deletions eng/tools/rush-runner/src/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export function parseArgs() {
let inFlags = false;
let isPackageFilter = false;
let artifactNames = "";
let ciFlag = false;
const services = [],
flags = [];
const [_scriptPath, action, ...givenArgs] = process.argv.slice(1);
Expand All @@ -29,7 +30,11 @@ export function parseArgs() {
}

if (inFlags) {
flags.push(arg);
if (arg === "--ci") {
ciFlag = true;
} else {
flags.push(arg);
}
} else if (isPackageFilter) {
artifactNames = arg;
isPackageFilter = false;
Expand All @@ -41,5 +46,5 @@ export function parseArgs() {
}
}

return { action, serviceDirs: services, rushParams: flags, artifactNames };
return { action, serviceDirs: services, rushParams: flags, artifactNames, ciFlag };
}
59 changes: 57 additions & 2 deletions eng/tools/rush-runner/src/rush.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

// @ts-check

import { spawnNode } from "./spawn.js";
import { spawnNode, spawnNodeWithOutput } from "./spawn.js";
import { getBaseDir } from "./env.js";
import { join as pathJoin } from "node:path";
import { runTestProxyRestore } from "./testProxyRestore.js";

/**
* Helper to run a global rush command
Expand Down Expand Up @@ -41,7 +42,7 @@ export function rushRunAll(action, direction, packages, rushParams) {
* @param {string[][]} packagesWithDirection - Any array of strings containing ["direction packageName"...]
* @param {string[]} rushParams - what parameters to pass to rush
*/
export function rushRunAllWithDirection(action, packagesWithDirection, rushParams) {
export function rushRunAllWithDirection(action, packagesWithDirection, rushParams, ciFlag) {
const invocation = packagesWithDirection.flatMap(([direction, packageName]) => [
direction,
packageName,
Expand All @@ -52,6 +53,40 @@ export function rushRunAllWithDirection(action, packagesWithDirection, rushParam
invocation,
});

// Restore assets for packages that are being 'unit-test'-ed in the CI pipeline
if (
// 1. eng/tools/rush-runner/index.js is running in CI: "--ci" flag is set
// Example: node eng/tools/rush-runner/index.js unit-test:node servicebus template -packages "azure-service-bus,azure-template" --ci --verbose -p max
ciFlag
// 2. Ensure not in "live" or "record" mode (run only in playback mode)
&& (!["live", "record"].includes(process.env.TEST_MODE))
// 3. Ensure the action is either 'unit-test:node' or 'unit-test:browser' (unit tests)
&& (['unit-test:node', 'unit-test:browser'].includes(action))
) {
console.log(`Running rush list with ${invocation.join(" ")}`);

// Get the list of packages to run the action on
let listCommandOutput = "";
try {
listCommandOutput = spawnNodeWithOutput(
getBaseDir(),
"common/scripts/install-run-rush.js",
"list",
...invocation,
);
} catch (error) {
console.error("Error running rush list command:", error);
}

if (listCommandOutput) {
// Parse the output to get package names
const packages = parsePackageNames(listCommandOutput);

// Run test-proxy restore for the parsed packages
runTestProxyRestore(packages);
}
}

return spawnNode(
getBaseDir(),
"common/scripts/install-run-rush.js",
Expand Down Expand Up @@ -80,3 +115,23 @@ export function runRushInPackageDirs(action, packageDirs, onError) {
}
return exitCode;
}

/**
* Parses the output of the `rush list ...` command to extract package names.
*
* @param {string} rushListOutput - The output string from the rush list command.
* @returns {string[]} - An array of package names that start with '@azure'.
*/
function parsePackageNames(rushListOutput) {
const packageNames = [];
const lines = rushListOutput.split('\n'); // Split the output into lines

for (const line of lines) {
const trimmedLine = line.trim(); // Trim whitespace
if (trimmedLine.startsWith('@azure')) { // Assuming package names start with '@azure'
packageNames.push(trimmedLine);
}
}

return packageNames;
}
21 changes: 21 additions & 0 deletions eng/tools/rush-runner/src/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,24 @@ export function spawnNode(cwd, ...args) {

return proc.status ?? 1;
}

/**
* Helper function to spawn NodeJS programs and return the output
*
* @param {string} cwd - current working directory
* @param {string[]} args - rest of arguments
* @returns {string} - output of the command
*/
export function spawnNodeWithOutput(cwd, ...args) {
console.log(`Executing: "node ${args.join(" ")}" in ${cwd}\n\n`);
const proc = spawnSync("node", args, { cwd, stdio: "pipe" });

if (proc.error) {
throw new Error(`Error executing command: ${proc.error.message}`);
}

const output = proc.stdout.toString();
console.log(`\n\nNode process exited with code ${proc.status}`);

return output;
}
63 changes: 63 additions & 0 deletions eng/tools/rush-runner/src/testProxyRestore.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

// @ts-check
import { join as pathJoin } from "node:path";
import { execSync } from "node:child_process";
import { getBaseDir } from "./env.js";
import { existsSync, readFileSync } from "node:fs";
import { parse } from "../../../../common/lib/jju/parse.js";

/**
* Runs test-proxy restore for the given packages.
*
* @param {string[]} packages - An array of package names to restore.
*/
export function runTestProxyRestore(packages) {
// Get the path to the proxy executable from the environment variable
const proxyExe = process.env.PROXY_EXE; // Set in the pipeline before this script is run
if (!proxyExe) {
console.error('PROXY_EXE environment variable is not set');
return;
}

console.log('Starting test-proxy restore for packages:', packages);
const completedPackages = [];
for (const packageName of packages) {
const rushSpec = readFileJson(pathJoin(getBaseDir(), "rush.json"));

// Find the target package
const targetPackage = rushSpec.projects.find(
packageSpec => packageSpec.packageName == packageName
);

// Get the directory of the target package
const targetPackageDir = pathJoin(getBaseDir(), targetPackage.projectFolder);

// Path to the assets.json file in the target package directory
const assetsJsonPath = pathJoin(targetPackageDir, 'assets.json');

// Check if the assets.json file exists
if (existsSync(assetsJsonPath)) {
try {
console.log(`Executing test-proxy restore for ${packageName}`);
execSync(`${proxyExe} restore -a "assets.json"`, { cwd: targetPackageDir, stdio: 'inherit' });
completedPackages.push(packageName);
} catch (error) {
console.error(`Error executing test-proxy restore: ${error.message}`);
}
}
}
console.log('Completed test-proxy restore for the packages:', completedPackages);
}


function readFileJson(filename) {
try {
const fileContents = readFileSync(filename);
const jsonResult = parse(fileContents);
return jsonResult;
} catch (ex) {
console.error(ex);
}
}

0 comments on commit 2d4563b

Please sign in to comment.