fix(upgrade-verify): isolate builds from env and context changes

ziacik · Jul 23, 2023 · 09bc255 · 09bc255
1 parent 6b1d255
commit 09bc255
Show file tree

Hide file tree

Showing 13 changed files with 79 additions and 7 deletions.
diff --git a/packages/upgrade-verify/CHANGELOG.md b/packages/upgrade-verify/CHANGELOG.md
@@ -7,13 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.0.3] - 2023-07-23
+
+### Fixed
+
+- Build runs are now isolated from ExecutorContext and process.env changes so that they do not interfere with each other.
+
 ## [0.0.2] - 2023-07-07
 
 ### Changed
 
 - READMEs updated and props added to package.json.
 - A _Package subpath './package.json' is not defined by "exports"_ error hopefully fixed.
 
-[unreleased]: https://github.com/ziacik/nx-tools/compare/upgrade-verify-0.0.2...HEAD
+[unreleased]: https://github.com/ziacik/nx-tools/compare/upgrade-verify-0.0.3...HEAD
+[0.0.3]: https://github.com/ziacik/nx-tools/compare/upgrade-verify-0.0.2...upgrade-verify-0.0.3
 [0.0.2]: https://github.com/ziacik/nx-tools/compare/upgrade-verify-0.0.1...upgrade-verify-0.0.2
 [0.0.1]: https://github.com/ziacik/nx-tools/releases/tag/upgrade-verify-0.0.1
diff --git a/packages/upgrade-verify/package.json b/packages/upgrade-verify/package.json
@@ -1,6 +1,6 @@
 {
 	"name": "@ziacik/upgrade-verify",
-	"version": "0.0.2",
+	"version": "0.0.3",
 	"type": "commonjs",
 	"executors": "./executors.json",
 	"keywords": [

diff --git a/...pgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/3rdpartylicenses.txt b/...pgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/3rdpartylicenses.txt
@@ -0,0 +1,2 @@
+@ziacik/source
+MIT
diff --git a/...ges/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/assets/.gitkeep b/...ges/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/assets/.gitkeep
diff --git a/packages/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/favicon.ico b/packages/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/favicon.ico
diff --git a/packages/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/index.html b/packages/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app/index.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<title>TestApp</title>
+		<base href="/">
+		<meta name="viewport" content="width=device-width, initial-scale=1" />
+		<link rel="icon" type="image/x-icon" href="favicon.ico" />
+	<link rel="stylesheet" href="styles.ef46db3751d8e999.css"><link rel="stylesheet" href="main.0274b0362ab850a9.css"></head>
+	<body>
+		<app-root></app-root>
+	<script src="runtime.dd97139be5136261.js" type="module"></script><script src="styles.91b7890a3e13c303.js" type="module"></script><script src="main.3d931a8f6ad8b28f.js" type="module"></script></body>
+</html>
diff --git a/...e-verify/src/executors/verify-build/__fixtures__/_dist/test-app/main.0274b0362ab850a9.css b/...e-verify/src/executors/verify-build/__fixtures__/_dist/test-app/main.0274b0362ab850a9.css
diff --git a/...de-verify/src/executors/verify-build/__fixtures__/_dist/test-app/main.3d931a8f6ad8b28f.js b/...de-verify/src/executors/verify-build/__fixtures__/_dist/test-app/main.3d931a8f6ad8b28f.js
diff --git a/...verify/src/executors/verify-build/__fixtures__/_dist/test-app/runtime.dd97139be5136261.js b/...verify/src/executors/verify-build/__fixtures__/_dist/test-app/runtime.dd97139be5136261.js
diff --git a/...-verify/src/executors/verify-build/__fixtures__/_dist/test-app/styles.91b7890a3e13c303.js b/...-verify/src/executors/verify-build/__fixtures__/_dist/test-app/styles.91b7890a3e13c303.js
diff --git a/...verify/src/executors/verify-build/__fixtures__/_dist/test-app/styles.ef46db3751d8e999.css b/...verify/src/executors/verify-build/__fixtures__/_dist/test-app/styles.ef46db3751d8e999.css
diff --git a/packages/upgrade-verify/src/executors/verify-build/executor.spec.ts b/packages/upgrade-verify/src/executors/verify-build/executor.spec.ts
@@ -23,7 +23,7 @@ describe('VerifyBuild Executor', () => {
 				yield {
 					success: true,
 					options: {
-						outputPath: resolve(__dirname, '__fixtures__/dist/test-app'),
+						outputPath: resolve(__dirname, '__fixtures__/_dist/test-app'),
 					},
 				};
 			})()
@@ -119,6 +119,40 @@ describe('VerifyBuild Executor', () => {
 		expect(readFile).not.toHaveBeenCalled();
 		expect(writeFile).not.toHaveBeenCalled();
 	});
+
+	it('isolates build runs from executor context modifications', async () => {
+		jest.spyOn(devkit, 'runExecutor').mockImplementation(async (targetDescription, overrides, context) => {
+			if (context.target?.command === 'should-not-retain-this') {
+				throw new Error('Context is modified from previous run.');
+			}
+			context.target ??= {};
+			context.target.command = 'should-not-retain-this';
+			return (async function* () {
+				yield { success: true };
+			})();
+		});
+		context.target = { command: 'whatever' };
+		const { success } = await executor(options, context);
+		expect(success).toBe(true);
+	});
+
+	it('isolates build runs from process env modifications (but retains the originals)', async () => {
+		jest.spyOn(devkit, 'runExecutor').mockImplementation(async (targetDescription, overrides, context) => {
+			if (process.env['something'] === 'should-not-retain-this') {
+				throw new Error('Env is modified from previous run.');
+			}
+			if (process.env['andThis'] !== 'should-be-retained') {
+				throw new Error('Env var has not been retained between runs.');
+			}
+			process.env['something'] = 'should-not-retain-this';
+			return (async function* () {
+				yield { success: true };
+			})();
+		});
+		process.env['andThis'] = 'should-be-retained';
+		const { success } = await executor(options, context);
+		expect(success).toBe(true);
+	});
 });
 
 function createContext(): ExecutorContext {
@@ -135,7 +169,7 @@ function createContext(): ExecutorContext {
 					targets: {
 						build: {
 							options: {
-								outputPath: 'packages/upgrade-verify/src/executors/verify-build/__fixtures__/dist/test-app',
+								outputPath: 'packages/upgrade-verify/src/executors/verify-build/__fixtures__/_dist/test-app',
 							},
 							configurations: {
 								production: {},

diff --git a/packages/upgrade-verify/src/executors/verify-build/executor.ts b/packages/upgrade-verify/src/executors/verify-build/executor.ts
@@ -1,6 +1,7 @@
 import { ExecutorContext, logger, runExecutor } from '@nx/devkit';
 import { mkdir, writeFile } from 'fs/promises';
 import { join } from 'path';
+import { env } from 'process';
 import { compareStats } from './dist-stat-comparer';
 import { calculateDistStats, loadExistingDistStats } from './dist-stats';
 import { VerifyBuildExecutorSchema } from './schema';
@@ -29,16 +30,20 @@ export default async function verifyBuild(options: VerifyBuildExecutorSchema, co
 	}
 
 	let success = true;
+	const envBackup = { ...env };
 
 	for (const configurationName of Object.keys(projectConfig.targets['build'].configurations)) {
+		retainEnv(envBackup);
+		const runContext: ExecutorContext = JSON.parse(JSON.stringify(context));
+
 		const result = await runExecutor(
 			{
-				project: context.projectName,
+				project: runContext.projectName ?? '',
 				target: 'build',
 				configuration: configurationName,
 			},
 			{},
-			context
+			runContext
 		);
 
 		for await (const x of result) {
@@ -56,7 +61,7 @@ export default async function verifyBuild(options: VerifyBuildExecutorSchema, co
 		if (existingStats != null) {
 			const comparison = compareStats(existingStats, newStats);
 			logger.info(
-				`Stats for ${context.projectName}/${configurationName}: ${comparison.totalSizeDifferencePercentage}% total size difference, ${comparison.fileCountDifferencePercentage}% file count difference, ${comparison.newFilesPercentage}% new files, ${comparison.deletedFilesPercentage}% deleted files`
+				`Stats for ${runContext.projectName}/${configurationName}: ${comparison.totalSizeDifferencePercentage}% total size difference, ${comparison.fileCountDifferencePercentage}% file count difference, ${comparison.newFilesPercentage}% new files, ${comparison.deletedFilesPercentage}% deleted files`
 			);
 
 			if (
@@ -80,3 +85,10 @@ async function tryMkdir(statsDir: string) {
 		// ignore
 	}
 }
+
+function retainEnv(envBackup: Record<string, unknown>): void {
+	for (const key of Object.keys(env)) {
+		delete env[key];
+	}
+	Object.assign(env, envBackup);
+}