From a107e9edc66d02249fa99fc9e053e6760e1da246 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 10 Jan 2020 09:54:58 +0000 Subject: [PATCH] feat(ngcc): lock ngcc when processing (#34722) Previously, it was possible for multiple instance of ngcc to be running at the same time, but this is not supported and can cause confusing and flakey errors at build time. Now, only one instance of ngcc can run at a time. If a second instance tries to execute it fails with an appropriate error message. See https://github.com/angular/angular/issues/32431#issuecomment-571825781 PR Close #34722 --- .../ngcc/src/execution/cluster/executor.ts | 16 +- .../ngcc/src/execution/lock_file.ts | 119 ++++++++++ .../src/execution/single_process_executor.ts | 35 +-- packages/compiler-cli/ngcc/src/main.ts | 13 +- .../test/execution/cluster/executor_spec.ts | 72 +++++- .../ngcc/test/execution/lock_file_spec.ts | 206 ++++++++++++++++++ .../single_processor_executor_spec.ts | 97 +++++++++ .../ngcc/test/helpers/mock_lock_file.ts | 26 +++ .../ngcc/test/integration/ngcc_spec.ts | 2 + 9 files changed, 553 insertions(+), 33 deletions(-) create mode 100644 packages/compiler-cli/ngcc/src/execution/lock_file.ts create mode 100644 packages/compiler-cli/ngcc/test/execution/lock_file_spec.ts create mode 100644 packages/compiler-cli/ngcc/test/execution/single_processor_executor_spec.ts create mode 100644 packages/compiler-cli/ngcc/test/helpers/mock_lock_file.ts diff --git a/packages/compiler-cli/ngcc/src/execution/cluster/executor.ts b/packages/compiler-cli/ngcc/src/execution/cluster/executor.ts index b0af89d6d0..8479c82625 100644 --- a/packages/compiler-cli/ngcc/src/execution/cluster/executor.ts +++ b/packages/compiler-cli/ngcc/src/execution/cluster/executor.ts @@ -13,6 +13,7 @@ import * as cluster from 'cluster'; import {Logger} from '../../logging/logger'; import {PackageJsonUpdater} from '../../writing/package_json_updater'; import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from '../api'; +import {LockFile} from '../lock_file'; import {ClusterMaster} from './master'; import {ClusterWorker} from './worker'; @@ -25,18 +26,19 @@ import {ClusterWorker} from './worker'; export class ClusterExecutor implements Executor { constructor( private workerCount: number, private logger: Logger, - private pkgJsonUpdater: PackageJsonUpdater) {} + private pkgJsonUpdater: PackageJsonUpdater, private lockFile: LockFile) {} async execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): Promise { if (cluster.isMaster) { - this.logger.debug( - `Running ngcc on ${this.constructor.name} (using ${this.workerCount} worker processes).`); - // This process is the cluster master. - const master = - new ClusterMaster(this.workerCount, this.logger, this.pkgJsonUpdater, analyzeEntryPoints); - return master.run(); + return this.lockFile.lock(() => { + this.logger.debug( + `Running ngcc on ${this.constructor.name} (using ${this.workerCount} worker processes).`); + const master = new ClusterMaster( + this.workerCount, this.logger, this.pkgJsonUpdater, analyzeEntryPoints); + return master.run(); + }); } else { // This process is a cluster worker. const worker = new ClusterWorker(this.logger, createCompileFn); diff --git a/packages/compiler-cli/ngcc/src/execution/lock_file.ts b/packages/compiler-cli/ngcc/src/execution/lock_file.ts new file mode 100644 index 0000000000..1bb0ce7273 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/execution/lock_file.ts @@ -0,0 +1,119 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import * as process from 'process'; +import {FileSystem} from '../../../src/ngtsc/file_system'; + +/** + * The LockFile is used to prevent more than one instance of ngcc executing at the same time. + * + * When ngcc starts executing, it creates a file in the `compiler-cli/ngcc` folder. If it finds one + * is already there then it fails with a suitable error message. + * When ngcc completes executing, it removes the file so that future ngcc executions can start. + */ +export class LockFile { + lockFilePath = + this.fs.resolve(require.resolve('@angular/compiler-cli/ngcc'), '../__ngcc_lock_file__'); + + constructor(private fs: FileSystem) {} + + /** + * Run a function guarded by the lock file. + * + * Note that T can be a Promise. If so, we run the `remove()` call in the promise's `finally` + * handler. Otherwise we run the `remove()` call in the `try...finally` block. + * + * @param fn The function to run. + */ + lock(fn: () => T): T { + let isAsync = false; + this.create(); + try { + const result = fn(); + if (result instanceof Promise) { + isAsync = true; + // The cast is necessary because TS cannot deduce that T is now a promise here. + return result.finally(() => this.remove()) as unknown as T; + } else { + return result; + } + } finally { + if (!isAsync) { + this.remove(); + } + } + } + + /** + * Write a lock file to disk, or error if there is already one there. + */ + protected create() { + try { + this.addSignalHandlers(); + // To avoid race conditions, we check for existence of the lockfile + // by actually trying to create it exclusively + this.fs.writeFile(this.lockFilePath, process.pid.toString(), /* exclusive */ true); + } catch (e) { + this.removeSignalHandlers(); + if (e.code !== 'EEXIST') { + throw e; + } + + // The lockfile already exists so raise a helpful error. + // It is feasible that the lockfile was removed between the previous check for existence + // and this file-read. If so then we still error but as gracefully as possible. + let pid: string; + try { + pid = this.fs.readFile(this.lockFilePath); + } catch { + pid = '{unknown}'; + } + + throw new Error( + `ngcc is already running at process with id ${pid}.\n` + + `(If you are sure no ngcc process is running then you should delete the lockfile at ${this.lockFilePath}.)`); + } + } + + /** + * Remove the lock file from disk. + */ + protected remove() { + this.removeSignalHandlers(); + if (this.fs.exists(this.lockFilePath)) { + this.fs.removeFile(this.lockFilePath); + } + } + + protected addSignalHandlers() { + process.once('SIGINT', this.signalHandler); + process.once('SIGHUP', this.signalHandler); + } + + protected removeSignalHandlers() { + process.removeListener('SIGINT', this.signalHandler); + process.removeListener('SIGHUP', this.signalHandler); + } + + /** + * This handle needs to be defined as a property rather than a method + * so that it can be passed around as a bound function. + */ + protected signalHandler = + () => { + this.remove(); + this.exit(1); + } + + /** + * This function wraps `process.exit()` which makes it easier to manage in unit tests, + * since it is not possible to mock out `process.exit()` when it is called from signal handlers. + */ + protected exit(code: number): void { + process.exit(code); + } +} diff --git a/packages/compiler-cli/ngcc/src/execution/single_process_executor.ts b/packages/compiler-cli/ngcc/src/execution/single_process_executor.ts index 205a8228ac..d06dceab3c 100644 --- a/packages/compiler-cli/ngcc/src/execution/single_process_executor.ts +++ b/packages/compiler-cli/ngcc/src/execution/single_process_executor.ts @@ -10,6 +10,7 @@ import {Logger} from '../logging/logger'; import {PackageJsonUpdater} from '../writing/package_json_updater'; import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './api'; +import {LockFile} from './lock_file'; import {onTaskCompleted} from './utils'; @@ -17,27 +18,31 @@ import {onTaskCompleted} from './utils'; * An `Executor` that processes all tasks serially and completes synchronously. */ export class SingleProcessExecutor implements Executor { - constructor(private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater) {} + constructor( + private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater, + private lockFile: LockFile) {} execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void { - this.logger.debug(`Running ngcc on ${this.constructor.name}.`); + this.lockFile.lock(() => { + this.logger.debug(`Running ngcc on ${this.constructor.name}.`); - const taskQueue = analyzeEntryPoints(); - const compile = - createCompileFn((task, outcome) => onTaskCompleted(this.pkgJsonUpdater, task, outcome)); + const taskQueue = analyzeEntryPoints(); + const compile = + createCompileFn((task, outcome) => onTaskCompleted(this.pkgJsonUpdater, task, outcome)); - // Process all tasks. - this.logger.debug('Processing tasks...'); - const startTime = Date.now(); + // Process all tasks. + this.logger.debug('Processing tasks...'); + const startTime = Date.now(); - while (!taskQueue.allTasksCompleted) { - const task = taskQueue.getNextTask() !; - compile(task); - taskQueue.markTaskCompleted(task); - } + while (!taskQueue.allTasksCompleted) { + const task = taskQueue.getNextTask() !; + compile(task); + taskQueue.markTaskCompleted(task); + } - const duration = Math.round((Date.now() - startTime) / 1000); - this.logger.debug(`Processed tasks in ${duration}s.`); + const duration = Math.round((Date.now() - startTime) / 1000); + this.logger.debug(`Processed tasks in ${duration}s.`); + }); } } diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 5cb77c0527..5bed3fd4ea 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -26,6 +26,7 @@ import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_poin import {AnalyzeEntryPointsFn, CreateCompileFn, Executor, PartiallyOrderedTasks, Task, TaskProcessingOutcome, TaskQueue} from './execution/api'; import {ClusterExecutor} from './execution/cluster/executor'; import {ClusterPackageJsonUpdater} from './execution/cluster/package_json_updater'; +import {LockFile} from './execution/lock_file'; import {AsyncSingleProcessExecutor, SingleProcessExecutor} from './execution/single_process_executor'; import {ParallelTaskQueue} from './execution/task_selection/parallel_task_queue'; import {SerialTaskQueue} from './execution/task_selection/serial_task_queue'; @@ -285,7 +286,7 @@ export function mainNgcc( }; // The executor for actually planning and getting the work done. - const executor = getExecutor(async, inParallel, logger, pkgJsonUpdater); + const executor = getExecutor(async, inParallel, logger, pkgJsonUpdater, new LockFile(fileSystem)); return executor.execute(analyzeEntryPoints, createCompileFn); } @@ -330,17 +331,17 @@ function getTaskQueue( } function getExecutor( - async: boolean, inParallel: boolean, logger: Logger, - pkgJsonUpdater: PackageJsonUpdater): Executor { + async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater, + lockFile: LockFile): Executor { if (inParallel) { // Execute in parallel (which implies async). // Use up to 8 CPU cores for workers, always reserving one for master. const workerCount = Math.min(8, os.cpus().length - 1); - return new ClusterExecutor(workerCount, logger, pkgJsonUpdater); + return new ClusterExecutor(workerCount, logger, pkgJsonUpdater, lockFile); } else { // Execute serially, on a single thread (either sync or async). - return async ? new AsyncSingleProcessExecutor(logger, pkgJsonUpdater) : - new SingleProcessExecutor(logger, pkgJsonUpdater); + return async ? new AsyncSingleProcessExecutor(logger, pkgJsonUpdater, lockFile) : + new SingleProcessExecutor(logger, pkgJsonUpdater, lockFile); } } diff --git a/packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts b/packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts index 41b8a6ce0b..6c6609f313 100644 --- a/packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts +++ b/packages/compiler-cli/ngcc/test/execution/cluster/executor_spec.ts @@ -14,6 +14,7 @@ import {ClusterExecutor} from '../../../src/execution/cluster/executor'; import {ClusterMaster} from '../../../src/execution/cluster/master'; import {ClusterWorker} from '../../../src/execution/cluster/worker'; import {PackageJsonUpdater} from '../../../src/writing/package_json_updater'; +import {MockLockFile} from '../../helpers/mock_lock_file'; import {MockLogger} from '../../helpers/mock_logger'; import {mockProperty} from '../../helpers/spy_utils'; @@ -23,14 +24,19 @@ describe('ClusterExecutor', () => { let masterRunSpy: jasmine.Spy; let workerRunSpy: jasmine.Spy; let mockLogger: MockLogger; + let mockLockFile: MockLockFile; let executor: ClusterExecutor; beforeEach(() => { - masterRunSpy = spyOn(ClusterMaster.prototype, 'run'); - workerRunSpy = spyOn(ClusterWorker.prototype, 'run'); + masterRunSpy = spyOn(ClusterMaster.prototype, 'run') + .and.returnValue(Promise.resolve('CusterMaster#run()')); + workerRunSpy = spyOn(ClusterWorker.prototype, 'run') + .and.returnValue(Promise.resolve('CusterWorker#run()')); mockLogger = new MockLogger(); - executor = new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater); + mockLockFile = new MockLockFile(); + executor = + new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, mockLockFile); }); describe('execute()', () => { @@ -47,7 +53,6 @@ describe('ClusterExecutor', () => { }); it('should delegate to `ClusterMaster#run()`', async() => { - masterRunSpy.and.returnValue('CusterMaster#run()'); const analyzeEntryPointsSpy = jasmine.createSpy('analyzeEntryPoints'); const createCompilerFnSpy = jasmine.createSpy('createCompilerFn'); @@ -60,6 +65,58 @@ describe('ClusterExecutor', () => { expect(analyzeEntryPointsSpy).toHaveBeenCalledWith(); expect(createCompilerFnSpy).not.toHaveBeenCalled(); }); + + it('should call LockFile.create() and LockFile.remove() if master runner completes successfully', + async() => { + const anyFn: () => any = () => undefined; + await executor.execute(anyFn, anyFn); + expect(mockLockFile.log).toEqual(['create()', 'remove()']); + }); + + it('should call LockFile.create() and LockFile.remove() if master runner fails', async() => { + const anyFn: () => any = () => undefined; + masterRunSpy.and.returnValue(Promise.reject(new Error('master runner error'))); + let error = ''; + try { + await executor.execute(anyFn, anyFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('master runner error'); + expect(mockLockFile.log).toEqual(['create()', 'remove()']); + }); + + it('should not call master runner if Lockfile.create() fails', async() => { + const anyFn: () => any = () => undefined; + const lockFile = new MockLockFile({throwOnCreate: true}); + executor = + new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, lockFile); + let error = ''; + try { + await executor.execute(anyFn, anyFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('LockFile.create() error'); + expect(lockFile.log).toEqual(['create()']); + expect(masterRunSpy).not.toHaveBeenCalled(); + }); + + it('should fail if Lockfile.remove() fails', async() => { + const anyFn: () => any = () => undefined; + const lockFile = new MockLockFile({throwOnRemove: true}); + executor = + new ClusterExecutor(42, mockLogger, null as unknown as PackageJsonUpdater, lockFile); + let error = ''; + try { + await executor.execute(anyFn, anyFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('LockFile.remove() error'); + expect(lockFile.log).toEqual(['create()', 'remove()']); + expect(masterRunSpy).toHaveBeenCalled(); + }); }); describe('(on cluster worker)', () => { @@ -73,7 +130,6 @@ describe('ClusterExecutor', () => { }); it('should delegate to `ClusterWorker#run()`', async() => { - workerRunSpy.and.returnValue('CusterWorker#run()'); const analyzeEntryPointsSpy = jasmine.createSpy('analyzeEntryPoints'); const createCompilerFnSpy = jasmine.createSpy('createCompilerFn'); @@ -86,6 +142,12 @@ describe('ClusterExecutor', () => { expect(analyzeEntryPointsSpy).not.toHaveBeenCalled(); expect(createCompilerFnSpy).toHaveBeenCalledWith(jasmine.any(Function)); }); + + it('should not call LockFile.create() or LockFile.remove()', async() => { + const anyFn: () => any = () => undefined; + await executor.execute(anyFn, anyFn); + expect(mockLockFile.log).toEqual([]); + }); }); }); }); diff --git a/packages/compiler-cli/ngcc/test/execution/lock_file_spec.ts b/packages/compiler-cli/ngcc/test/execution/lock_file_spec.ts new file mode 100644 index 0000000000..3d09e68082 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/execution/lock_file_spec.ts @@ -0,0 +1,206 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import * as process from 'process'; +import {FileSystem, getFileSystem} from '../../../src/ngtsc/file_system'; +import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; +import {LockFile} from '../../src/execution/lock_file'; + +/** + * This class allows us to test the protected methods of LockFile directly, + * which are normally hidden as "protected". + * + * We also add logging in here to track what is being called and in what order. + * + * Finally this class stubs out the `exit()` method to prevent unit tests from exiting the process. + */ +class LockFileUnderTest extends LockFile { + log: string[] = []; + constructor(fs: FileSystem, private handleSignals = false) { + super(fs); + fs.ensureDir(fs.dirname(this.lockFilePath)); + } + create() { + this.log.push('create()'); + super.create(); + } + remove() { + this.log.push('remove()'); + super.remove(); + } + addSignalHandlers() { + if (this.handleSignals) { + super.addSignalHandlers(); + } + } + removeSignalHandlers() { super.removeSignalHandlers(); } + exit(code: number) { this.log.push(`exit(${code})`); } +} + +runInEachFileSystem(() => { + describe('LockFile', () => { + describe('lock() - synchronous', () => { + it('should guard the `fn()` with calls to `create()` and `remove()`', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + + lockFile.lock(() => lockFile.log.push('fn()')); + expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']); + }); + + it('should guard the `fn()` with calls to `create()` and `remove()`, even if it throws', + () => { + let error: string = ''; + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + + try { + lockFile.lock(() => { + lockFile.log.push('fn()'); + throw new Error('ERROR'); + }); + } catch (e) { + error = e.message; + } + expect(error).toEqual('ERROR'); + expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']); + }); + + it('should remove the lockfile if CTRL-C is triggered', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true); + + lockFile.lock(() => { + lockFile.log.push('SIGINT'); + process.emit('SIGINT', 'SIGINT'); + }); + // Since the test does not actually exit process, the `remove()` is called one more time. + expect(lockFile.log).toEqual(['create()', 'SIGINT', 'remove()', 'exit(1)', 'remove()']); + // Clean up the signal handlers. In practice this is not needed since the process would have + // been terminated already. + lockFile.removeSignalHandlers(); + }); + + it('should remove the lockfile if terminal is closed', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true); + + lockFile.lock(() => { + lockFile.log.push('SIGHUP'); + process.emit('SIGHUP', 'SIGHUP'); + }); + // Since this does not actually exit process, the `remove()` is called one more time. + expect(lockFile.log).toEqual(['create()', 'SIGHUP', 'remove()', 'exit(1)', 'remove()']); + // Clean up the signal handlers. In practice this is not needed since the process would have + // been terminated already. + lockFile.removeSignalHandlers(); + }); + }); + + describe('lock() - asynchronous', () => { + it('should guard the `fn()` with calls to `create()` and `remove()`', async() => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + + await lockFile.lock(async() => { + lockFile.log.push('fn() - before'); + // This promise forces node to do a tick in this function, ensuring that we are truly + // testing an async scenario. + await Promise.resolve(); + lockFile.log.push('fn() - after'); + }); + expect(lockFile.log).toEqual(['create()', 'fn() - before', 'fn() - after', 'remove()']); + }); + + it('should guard the `fn()` with calls to `create()` and `remove()`, even if it throws', + async() => { + let error: string = ''; + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + lockFile.create = () => lockFile.log.push('create()'); + lockFile.remove = () => lockFile.log.push('remove()'); + + try { + await lockFile.lock(async() => { + lockFile.log.push('fn()'); + throw new Error('ERROR'); + }); + } catch (e) { + error = e.message; + } + expect(error).toEqual('ERROR'); + expect(lockFile.log).toEqual(['create()', 'fn()', 'remove()']); + }); + + it('should remove the lockfile if CTRL-C is triggered', async() => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true); + + await lockFile.lock(async() => { + lockFile.log.push('SIGINT'); + process.emit('SIGINT', 'SIGINT'); + }); + // Since the test does not actually exit process, the `remove()` is called one more time. + expect(lockFile.log).toEqual(['create()', 'SIGINT', 'remove()', 'exit(1)', 'remove()']); + // Clean up the signal handlers. In practice this is not needed since the process would have + // been terminated already. + lockFile.removeSignalHandlers(); + }); + + it('should remove the lockfile if terminal is closed', async() => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs, /* handleSignals */ true); + + await lockFile.lock(async() => { + lockFile.log.push('SIGHUP'); + process.emit('SIGHUP', 'SIGHUP'); + }); + // Since this does not actually exit process, the `remove()` is called one more time. + expect(lockFile.log).toEqual(['create()', 'SIGHUP', 'remove()', 'exit(1)', 'remove()']); + // Clean up the signal handlers. In practice this is not needed since the process would have + // been terminated already. + lockFile.removeSignalHandlers(); + }); + }); + + describe('create()', () => { + it('should write a lock file to the file-system', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + expect(fs.exists(lockFile.lockFilePath)).toBe(false); + lockFile.create(); + expect(fs.exists(lockFile.lockFilePath)).toBe(true); + }); + + it('should error if a lock file already exists', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + fs.writeFile(lockFile.lockFilePath, '188'); + expect(() => lockFile.create()) + .toThrowError( + `ngcc is already running at process with id 188.\n` + + `(If you are sure no ngcc process is running then you should delete the lockfile at ${lockFile.lockFilePath}.)`); + }); + }); + + describe('remove()', () => { + it('should remove the lock file from the file-system', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + fs.writeFile(lockFile.lockFilePath, '188'); + lockFile.remove(); + expect(fs.exists(lockFile.lockFilePath)).toBe(false); + }); + + it('should not error if the lock file does not exist', () => { + const fs = getFileSystem(); + const lockFile = new LockFileUnderTest(fs); + expect(() => lockFile.remove()).not.toThrow(); + }); + }); + }); +}); diff --git a/packages/compiler-cli/ngcc/test/execution/single_processor_executor_spec.ts b/packages/compiler-cli/ngcc/test/execution/single_processor_executor_spec.ts new file mode 100644 index 0000000000..e55b26b531 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/execution/single_processor_executor_spec.ts @@ -0,0 +1,97 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/// + +import {SingleProcessExecutor} from '../../src/execution/single_process_executor'; +import {SerialTaskQueue} from '../../src/execution/task_selection/serial_task_queue'; +import {PackageJsonUpdater} from '../../src/writing/package_json_updater'; +import {MockLockFile} from '../helpers/mock_lock_file'; +import {MockLogger} from '../helpers/mock_logger'; + + +describe('SingleProcessExecutor', () => { + let mockLogger: MockLogger; + let mockLockFile: MockLockFile; + let executor: SingleProcessExecutor; + + beforeEach(() => { + mockLogger = new MockLogger(); + mockLockFile = new MockLockFile(); + executor = + new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, mockLockFile); + }); + + describe('execute()', () => { + it('should call LockFile.create() and LockFile.remove() if processing completes successfully', + () => { + const noTasks = () => new SerialTaskQueue([] as any); + const createCompileFn: () => any = () => undefined; + executor.execute(noTasks, createCompileFn); + expect(mockLockFile.log).toEqual(['create()', 'remove()']); + }); + + it('should call LockFile.create() and LockFile.remove() if `analyzeEntryPoints` fails', () => { + const errorFn: () => never = () => { throw new Error('analyze error'); }; + const createCompileFn: () => any = () => undefined; + let error: string = ''; + try { + executor.execute(errorFn, createCompileFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('analyze error'); + expect(mockLockFile.log).toEqual(['create()', 'remove()']); + }); + + it('should call LockFile.create() and LockFile.remove() if `createCompileFn` fails', () => { + const oneTask = () => new SerialTaskQueue([{}] as any); + const createErrorCompileFn: () => any = () => { throw new Error('compile error'); }; + let error: string = ''; + try { + executor.execute(oneTask, createErrorCompileFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('compile error'); + expect(mockLockFile.log).toEqual(['create()', 'remove()']); + }); + + it('should not call `analyzeEntryPoints` if Lockfile.create() fails', () => { + const lockFile = new MockLockFile({throwOnCreate: true}); + const analyzeFn: () => any = () => { lockFile.log.push('analyzeFn'); }; + const anyFn: () => any = () => undefined; + executor = + new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, lockFile); + let error = ''; + try { + executor.execute(analyzeFn, anyFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('LockFile.create() error'); + expect(lockFile.log).toEqual(['create()']); + }); + + it('should fail if Lockfile.remove() fails', () => { + const noTasks = () => new SerialTaskQueue([] as any); + const anyFn: () => any = () => undefined; + const lockFile = new MockLockFile({throwOnRemove: true}); + executor = + new SingleProcessExecutor(mockLogger, null as unknown as PackageJsonUpdater, lockFile); + let error = ''; + try { + executor.execute(noTasks, anyFn); + } catch (e) { + error = e.message; + } + expect(error).toEqual('LockFile.remove() error'); + expect(lockFile.log).toEqual(['create()', 'remove()']); + }); + }); +}); diff --git a/packages/compiler-cli/ngcc/test/helpers/mock_lock_file.ts b/packages/compiler-cli/ngcc/test/helpers/mock_lock_file.ts new file mode 100644 index 0000000000..77b2040fb5 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/helpers/mock_lock_file.ts @@ -0,0 +1,26 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {MockFileSystemNative} from '../../../src/ngtsc/file_system/testing'; +import {LockFile} from '../../src/execution/lock_file'; + +export class MockLockFile extends LockFile { + log: string[] = []; + constructor(private options: {throwOnCreate?: boolean, throwOnRemove?: boolean} = {}) { + // This `MockLockFile` is not used in tests that are run via `runInEachFileSystem()` + // So we cannot use `getFileSystem()` but instead just instantiate a mock file-system. + super(new MockFileSystemNative()); + } + create() { + this.log.push('create()'); + if (this.options.throwOnCreate) throw new Error('LockFile.create() error'); + } + remove() { + this.log.push('remove()'); + if (this.options.throwOnRemove) throw new Error('LockFile.remove() error'); + } +} diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index ec676e814f..dcfb6e3e25 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -13,6 +13,7 @@ import * as os from 'os'; import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem, join} from '../../../src/ngtsc/file_system'; import {Folder, MockFileSystem, TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadStandardTestFiles, loadTestFiles} from '../../../test/helpers'; +import {LockFile} from '../../src/execution/lock_file'; import {mainNgcc} from '../../src/main'; import {markAsProcessed} from '../../src/packages/build_marker'; import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point'; @@ -1319,6 +1320,7 @@ runInEachFileSystem(() => { function initMockFileSystem(fs: FileSystem, testFiles: Folder) { if (fs instanceof MockFileSystem) { fs.init(testFiles); + fs.ensureDir(fs.dirname(new LockFile(fs).lockFilePath)); } // a random test package that no metadata.json file so not compiled by Angular.