diff --git a/common/changes/@microsoft/rush/fix-unreliable-tests_2026-01-06-04-45.json b/common/changes/@microsoft/rush/fix-unreliable-tests_2026-01-06-04-45.json new file mode 100644 index 0000000000..efcd84c45f --- /dev/null +++ b/common/changes/@microsoft/rush/fix-unreliable-tests_2026-01-06-04-45.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "comment": "", + "type": "none", + "packageName": "@microsoft/rush" + } + ], + "packageName": "@microsoft/rush", + "email": "iclanton@users.noreply.github.com" +} \ No newline at end of file diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts index 4b61e83c26..015c987187 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -20,8 +20,8 @@ jest.mock(`@rushstack/package-deps-hash`, () => { getGitHashForFiles(filePaths: Iterable): ReadonlyMap { return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath])); }, - hashFilesAsync(rootDirectory: string, filePaths: Iterable): ReadonlyMap { - return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath])); + hashFilesAsync(rootDirectory: string, filePaths: Iterable): Promise> { + return Promise.resolve(new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath]))); } }; }); @@ -33,8 +33,13 @@ import { FileSystem, JsonFile, Path } from '@rushstack/node-core-library'; import type { IDetailedRepoState } from '@rushstack/package-deps-hash'; import { Autoinstaller } from '../../logic/Autoinstaller'; import type { ITelemetryData } from '../../logic/Telemetry'; -import { getCommandLineParserInstanceAsync, type SpawnMockArgs, type SpawnMockCall } from './TestUtils'; -import { EnvironmentConfiguration } from '../../api/EnvironmentConfiguration'; +import { + getCommandLineParserInstanceAsync, + type SpawnMockArgs, + type SpawnMockCall, + isolateEnvironmentConfigurationForTests, + type IEnvironmentConfigIsolation +} from './TestUtils'; import { IS_WINDOWS } from '../../utilities/executionUtilities'; // Ordinals into the `mock.calls` array referencing each of the arguments to `spawn`. Note that @@ -73,13 +78,15 @@ function expectSpawnToMatchRegexp(spawnCall: SpawnMockCall, expectedRegexp: RegE describe('RushCommandLineParser', () => { describe('execute', () => { + let _envIsolation: IEnvironmentConfigIsolation; + + beforeEach(() => { + _envIsolation = isolateEnvironmentConfigurationForTests(); + }); + afterEach(() => { jest.clearAllMocks(); - EnvironmentConfiguration.reset(); - jest - .spyOn(EnvironmentConfiguration, 'buildCacheOverrideJsonFilePath', 'get') - .mockReturnValue(undefined); - jest.spyOn(EnvironmentConfiguration, 'buildCacheOverrideJson', 'get').mockReturnValue(undefined); + _envIsolation.restore(); }); describe('in basic repo', () => { diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts index e1f0d46ea6..e4501830cb 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts @@ -13,12 +13,15 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return { hasSubmodules: false, hasUncommittedChanges: false, - files: new Map(), + files: new Map([['common/config/rush/npm-shrinkwrap.json', 'hash']]), symlinks: new Map() }; }, getRepoChangesAsync(): ReadonlyMap { return new Map(); + }, + hashFilesAsync(rootDirectory: string, filePaths: Iterable): Promise> { + return Promise.resolve(new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath]))); } }; }); @@ -28,11 +31,19 @@ import type { IDetailedRepoState } from '@rushstack/package-deps-hash'; import { Autoinstaller } from '../../logic/Autoinstaller'; import type { ITelemetryData } from '../../logic/Telemetry'; import { getCommandLineParserInstanceAsync, setSpawnMock } from './TestUtils'; +import { isolateEnvironmentConfigurationForTests, type IEnvironmentConfigIsolation } from './TestUtils'; describe('RushCommandLineParserFailureCases', () => { describe('execute', () => { + let _envIsolation: IEnvironmentConfigIsolation; + + beforeEach(() => { + _envIsolation = isolateEnvironmentConfigurationForTests({ silenceStderrWrite: true }); + }); + afterEach(() => { - jest.clearAllMocks(); + _envIsolation.restore(); + jest.restoreAllMocks(); }); describe('in repo plugin custom flushTelemetry', () => { diff --git a/libraries/rush-lib/src/cli/test/TestUtils.ts b/libraries/rush-lib/src/cli/test/TestUtils.ts index 8a4dc6d422..c8191358c2 100644 --- a/libraries/rush-lib/src/cli/test/TestUtils.ts +++ b/libraries/rush-lib/src/cli/test/TestUtils.ts @@ -6,6 +6,7 @@ import { AlreadyExistsBehavior, FileSystem, PackageJsonLookup } from '@rushstack import type { RushCommandLineParser as RushCommandLineParserType } from '../RushCommandLineParser'; import { FlagFile } from '../../api/FlagFile'; import { RushConstants } from '../../logic/RushConstants'; +import { EnvironmentConfiguration } from '../../api/EnvironmentConfiguration'; export type SpawnMockArgs = Parameters; export type SpawnMock = jest.Mock, SpawnMockArgs>; @@ -37,6 +38,23 @@ export interface IChildProcessModuleMock { spawn: jest.Mock; } +const DEFAULT_RUSH_ENV_VARS_TO_CLEAR: ReadonlyArray = [ + 'RUSH_BUILD_CACHE_OVERRIDE_JSON', + 'RUSH_BUILD_CACHE_OVERRIDE_JSON_FILE_PATH', + 'RUSH_BUILD_CACHE_CREDENTIAL', + 'RUSH_BUILD_CACHE_ENABLED', + 'RUSH_BUILD_CACHE_WRITE_ALLOWED' +]; + +export interface IWithEnvironmentConfigIsolationOptions { + envVarNamesToClear?: ReadonlyArray; + silenceStderrWrite?: boolean; +} + +export interface IEnvironmentConfigIsolation { + restore(): void; +} + /** * Configure the `child_process` `spawn` mock for these tests. This relies on the mock implementation * in `mock_child_process`. @@ -102,3 +120,81 @@ export async function getCommandLineParserInstanceAsync( repoPath }; } + +/** + * Clears Rush-related environment variables and resets EnvironmentConfiguration for deterministic tests. + * + * Notes: + * - EnvironmentConfiguration caches some values, so we also stub the build-cache override getters. + * - Rush treats any stderr output during `rush test` as a warning, which fails the command; some + * tests intentionally simulate failures and may need stderr silenced. + */ +export function isolateEnvironmentConfigurationForTests( + options: IWithEnvironmentConfigIsolationOptions = {} +): IEnvironmentConfigIsolation { + const envVarNamesToClear: ReadonlyArray = + options.envVarNamesToClear ?? DEFAULT_RUSH_ENV_VARS_TO_CLEAR; + + const savedProcessEnv: Record = {}; + for (const envVarName of envVarNamesToClear) { + savedProcessEnv[envVarName] = process.env[envVarName]; + delete process.env[envVarName]; + } + + EnvironmentConfiguration.reset(); + + const restoreFns: Array<() => void> = []; + + restoreFns.push(() => { + for (const envVarName of envVarNamesToClear) { + const oldValue: string | undefined = savedProcessEnv[envVarName]; + if (oldValue === undefined) { + delete process.env[envVarName]; + } else { + process.env[envVarName] = oldValue; + } + } + }); + + if (options.silenceStderrWrite) { + type StderrWrite = typeof process.stderr.write; + const silentWrite: unknown = ( + chunk: string | Uint8Array, + encoding?: BufferEncoding | ((err?: Error | null) => void), + cb?: (err?: Error | null) => void + ): boolean => { + if (typeof encoding === 'function') { + encoding(null); + } else { + cb?.(null); + } + return true; + }; + + const writeSpy: jest.SpyInstance, Parameters> = jest + .spyOn(process.stderr, 'write') + .mockImplementation(silentWrite as StderrWrite); + + restoreFns.push(() => writeSpy.mockRestore()); + } + + // EnvironmentConfiguration.reset() does not clear cached values for these fields. + const overrideJsonFilePathSpy: jest.SpyInstance = jest + .spyOn(EnvironmentConfiguration, 'buildCacheOverrideJsonFilePath', 'get') + .mockReturnValue(undefined); + const overrideJsonSpy: jest.SpyInstance = jest + .spyOn(EnvironmentConfiguration, 'buildCacheOverrideJson', 'get') + .mockReturnValue(undefined); + + restoreFns.push(() => overrideJsonFilePathSpy.mockRestore()); + restoreFns.push(() => overrideJsonSpy.mockRestore()); + restoreFns.push(() => EnvironmentConfiguration.reset()); + + return { + restore: () => { + for (let i: number = restoreFns.length - 1; i >= 0; i--) { + restoreFns[i](); + } + } + }; +}