From 2152622efb26bc7e2fc126b29c063ed48c76ca74 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 2 Jan 2024 11:52:26 -0800 Subject: [PATCH 1/4] Require IOperationOptions.name --- apps/heft/src/cli/HeftActionRunner.ts | 2 ++ common/reviews/api/operation-graph.api.md | 6 +++--- libraries/operation-graph/src/Operation.ts | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/apps/heft/src/cli/HeftActionRunner.ts b/apps/heft/src/cli/HeftActionRunner.ts index d0cb2e6076..b5fc587c95 100644 --- a/apps/heft/src/cli/HeftActionRunner.ts +++ b/apps/heft/src/cli/HeftActionRunner.ts @@ -447,6 +447,7 @@ function _getOrCreatePhaseOperation( // Only create the operation. Dependencies are hooked up separately operation = new Operation({ groupName: phase.phaseName, + name: `${phase.phaseName} phase`, runner: new PhaseOperationRunner({ phase, internalHeftSession }) }); operations.set(key, operation); @@ -466,6 +467,7 @@ function _getOrCreateTaskOperation( if (!operation) { operation = new Operation({ groupName: task.parentPhase.phaseName, + name: `${task.taskName} task`, runner: new TaskOperationRunner({ internalHeftSession, task diff --git a/common/reviews/api/operation-graph.api.md b/common/reviews/api/operation-graph.api.md index e85f4bad9e..e03e5bffa2 100644 --- a/common/reviews/api/operation-graph.api.md +++ b/common/reviews/api/operation-graph.api.md @@ -58,7 +58,7 @@ export interface IOperationExecutionOptions { // @beta export interface IOperationOptions { groupName?: string | undefined; - name?: string | undefined; + name: string; runner?: IOperationRunner | undefined; weight?: number | undefined; } @@ -140,7 +140,7 @@ export interface IWatchLoopState { // @beta export class Operation implements IOperationStates { - constructor(options?: IOperationOptions); + constructor(options: IOperationOptions); // (undocumented) addDependency(dependency: Operation): void; readonly consumers: Set; @@ -152,7 +152,7 @@ export class Operation implements IOperationStates { _executeAsync(context: IExecuteOperationContext): Promise; readonly groupName: string | undefined; lastState: IOperationState | undefined; - readonly name: string | undefined; + readonly name: string; // (undocumented) reset(): void; runner: IOperationRunner | undefined; diff --git a/libraries/operation-graph/src/Operation.ts b/libraries/operation-graph/src/Operation.ts index 167c7abcaf..db9bc9a1a9 100644 --- a/libraries/operation-graph/src/Operation.ts +++ b/libraries/operation-graph/src/Operation.ts @@ -22,7 +22,7 @@ export interface IOperationOptions { /** * The name of this operation, for logging. */ - name?: string | undefined; + name: string; /** * The group that this operation belongs to. Will be used for logging and duration tracking. @@ -101,7 +101,7 @@ export class Operation implements IOperationStates { /** * The name of this operation, for logging. */ - public readonly name: string | undefined; + public readonly name: string; /** * When the scheduler is ready to process this `Operation`, the `runner` implements the actual work of @@ -174,11 +174,11 @@ export class Operation implements IOperationStates { */ private _runPending: boolean = true; - public constructor(options?: IOperationOptions) { + public constructor(options: IOperationOptions) { this.groupName = options?.groupName; this.runner = options?.runner; this.weight = options?.weight || 1; - this.name = options?.name; + this.name = options.name; } public addDependency(dependency: Operation): void { From e3647d354dc906a7448fc0859d8ee46ddca394fa Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Mon, 8 Jan 2024 16:30:13 -0800 Subject: [PATCH 2/4] - Rename "name" to "operationName" to make it easier to trace how operation names percolate through the call graphs - Fix spelling of "requestor" and make it non-optional --- apps/heft/src/cli/HeftActionRunner.ts | 10 ++-- .../runners/PhaseOperationRunner.ts | 2 +- .../operations/runners/TaskOperationRunner.ts | 2 +- common/reviews/api/operation-graph.api.md | 18 +++--- .../operation-graph/src/IOperationRunner.ts | 2 +- libraries/operation-graph/src/Operation.ts | 12 ++-- .../src/OperationExecutionManager.ts | 6 +- .../src/OperationGroupRecord.ts | 2 +- libraries/operation-graph/src/WatchLoop.ts | 20 +++---- .../src/calculateCriticalPath.ts | 8 ++- .../operation-graph/src/protocol.types.ts | 2 +- .../test/OperationExecutionManager.test.ts | 56 +++++++++---------- .../src/test/calculateCriticalPath.test.ts | 12 ++-- 13 files changed, 78 insertions(+), 74 deletions(-) diff --git a/apps/heft/src/cli/HeftActionRunner.ts b/apps/heft/src/cli/HeftActionRunner.ts index b5fc587c95..7fd311338b 100644 --- a/apps/heft/src/cli/HeftActionRunner.ts +++ b/apps/heft/src/cli/HeftActionRunner.ts @@ -333,8 +333,8 @@ export class HeftActionRunner { executeAsync: (state: IWatchLoopState): Promise => { return this._executeOnceAsync(executionManager, state.abortSignal, state.requestRun); }, - onRequestRun: (requestor?: string) => { - terminal.writeLine(Colorize.bold(`New run requested by ${requestor || 'unknown task'}`)); + onRequestRun: (requester: string) => { + terminal.writeLine(Colorize.bold(`New run requested by ${requester}`)); }, onAbort: () => { terminal.writeLine(Colorize.bold(`Cancelling incremental build...`)); @@ -346,7 +346,7 @@ export class HeftActionRunner { private async _executeOnceAsync( executionManager: OperationExecutionManager, abortSignal: AbortSignal, - requestRun?: (requestor?: string) => void + requestRun?: (requester: string) => void ): Promise { // Record this as the start of task execution. this._metricsCollector.setStartTime(); @@ -447,7 +447,7 @@ function _getOrCreatePhaseOperation( // Only create the operation. Dependencies are hooked up separately operation = new Operation({ groupName: phase.phaseName, - name: `${phase.phaseName} phase`, + operationName: `${phase.phaseName} phase`, runner: new PhaseOperationRunner({ phase, internalHeftSession }) }); operations.set(key, operation); @@ -467,7 +467,7 @@ function _getOrCreateTaskOperation( if (!operation) { operation = new Operation({ groupName: task.parentPhase.phaseName, - name: `${task.taskName} task`, + operationName: `${task.taskName} task`, runner: new TaskOperationRunner({ internalHeftSession, task diff --git a/apps/heft/src/operations/runners/PhaseOperationRunner.ts b/apps/heft/src/operations/runners/PhaseOperationRunner.ts index f6f1ba3ed0..7f760624fd 100644 --- a/apps/heft/src/operations/runners/PhaseOperationRunner.ts +++ b/apps/heft/src/operations/runners/PhaseOperationRunner.ts @@ -23,7 +23,7 @@ export class PhaseOperationRunner implements IOperationRunner { private readonly _options: IPhaseOperationRunnerOptions; private _isClean: boolean = false; - public get name(): string { + public get operationName(): string { return `Phase ${JSON.stringify(this._options.phase.phaseName)}`; } diff --git a/apps/heft/src/operations/runners/TaskOperationRunner.ts b/apps/heft/src/operations/runners/TaskOperationRunner.ts index 66274a696a..18c0143725 100644 --- a/apps/heft/src/operations/runners/TaskOperationRunner.ts +++ b/apps/heft/src/operations/runners/TaskOperationRunner.ts @@ -55,7 +55,7 @@ export class TaskOperationRunner implements IOperationRunner { public readonly silent: boolean = false; - public get name(): string { + public get operationName(): string { const { taskName, parentPhase } = this._options.task; return `Task ${JSON.stringify(taskName)} of phase ${JSON.stringify(parentPhase.phaseName)}`; } diff --git a/common/reviews/api/operation-graph.api.md b/common/reviews/api/operation-graph.api.md index e03e5bffa2..427caa001f 100644 --- a/common/reviews/api/operation-graph.api.md +++ b/common/reviews/api/operation-graph.api.md @@ -33,7 +33,7 @@ export interface IExecuteOperationContext extends Omit Promise, priority: number): Promise; - requestRun?: (requestor?: string) => void; + requestRun?: (requester: string) => void; terminal: ITerminal; } @@ -50,7 +50,7 @@ export interface IOperationExecutionOptions { // (undocumented) parallelism: number; // (undocumented) - requestRun?: (requestor?: string) => void; + requestRun?: (requester: string) => void; // (undocumented) terminal: ITerminal; } @@ -58,7 +58,7 @@ export interface IOperationExecutionOptions { // @beta export interface IOperationOptions { groupName?: string | undefined; - name: string; + operationName: string; runner?: IOperationRunner | undefined; weight?: number | undefined; } @@ -66,7 +66,7 @@ export interface IOperationOptions { // @beta export interface IOperationRunner { executeAsync(context: IOperationRunnerContext): Promise; - readonly name: string; + readonly operationName: string; silent: boolean; } @@ -99,7 +99,7 @@ export interface IRequestRunEventMessage { // (undocumented) event: 'requestRun'; // (undocumented) - requestor?: string; + requester: string; } // @beta @@ -127,7 +127,7 @@ export interface IWatchLoopOptions { executeAsync: (state: IWatchLoopState) => Promise; onAbort: () => void; onBeforeExecute: () => void; - onRequestRun: (requestor?: string) => void; + onRequestRun: (requester: string) => void; } // @beta @@ -135,7 +135,7 @@ export interface IWatchLoopState { // (undocumented) get abortSignal(): AbortSignal; // (undocumented) - requestRun: (requestor?: string) => void; + requestRun: (requester: string) => void; } // @beta @@ -152,7 +152,7 @@ export class Operation implements IOperationStates { _executeAsync(context: IExecuteOperationContext): Promise; readonly groupName: string | undefined; lastState: IOperationState | undefined; - readonly name: string; + readonly operationName: string; // (undocumented) reset(): void; runner: IOperationRunner | undefined; @@ -231,7 +231,7 @@ export class Stopwatch { export class WatchLoop implements IWatchLoopState { constructor(options: IWatchLoopOptions); get abortSignal(): AbortSignal; - requestRun: (requestor?: string) => void; + requestRun: (requester: string) => void; runIPCAsync(host?: IPCHost): Promise; runUntilAbortedAsync(abortSignal: AbortSignal, onWaiting: () => void): Promise; runUntilStableAsync(abortSignal: AbortSignal): Promise; diff --git a/libraries/operation-graph/src/IOperationRunner.ts b/libraries/operation-graph/src/IOperationRunner.ts index 543257273c..2590a4dafa 100644 --- a/libraries/operation-graph/src/IOperationRunner.ts +++ b/libraries/operation-graph/src/IOperationRunner.ts @@ -81,7 +81,7 @@ export interface IOperationRunner { /** * Name of the operation, for logging. */ - readonly name: string; + readonly operationName: string; /** * Indicates that this runner is architectural and should not be reported on. diff --git a/libraries/operation-graph/src/Operation.ts b/libraries/operation-graph/src/Operation.ts index db9bc9a1a9..b85442098d 100644 --- a/libraries/operation-graph/src/Operation.ts +++ b/libraries/operation-graph/src/Operation.ts @@ -22,7 +22,7 @@ export interface IOperationOptions { /** * The name of this operation, for logging. */ - name: string; + operationName: string; /** * The group that this operation belongs to. Will be used for logging and duration tracking. @@ -68,7 +68,7 @@ export interface IExecuteOperationContext extends Omit void; + requestRun?: (requester: string) => void; /** * Terminal to write output to. @@ -101,7 +101,7 @@ export class Operation implements IOperationStates { /** * The name of this operation, for logging. */ - public readonly name: string; + public readonly operationName: string; /** * When the scheduler is ready to process this `Operation`, the `runner` implements the actual work of @@ -178,7 +178,7 @@ export class Operation implements IOperationStates { this.groupName = options?.groupName; this.runner = options?.runner; this.weight = options?.weight || 1; - this.name = options.name; + this.operationName = options.operationName; } public addDependency(dependency: Operation): void { @@ -280,7 +280,9 @@ export class Operation implements IOperationStates { // The requestRun callback is assumed to remain constant // throughout the lifetime of the process, so it is safe // to capture here. - return requestRun(this.name); + return requestRun(this.operationName); + case undefined: + throw new InternalError(`The operation state is undefined`); default: // This line is here to enforce exhaustiveness const currentStatus: undefined = this.state?.status; diff --git a/libraries/operation-graph/src/OperationExecutionManager.ts b/libraries/operation-graph/src/OperationExecutionManager.ts index f780802cd6..ba8aa9de41 100644 --- a/libraries/operation-graph/src/OperationExecutionManager.ts +++ b/libraries/operation-graph/src/OperationExecutionManager.ts @@ -21,7 +21,7 @@ export interface IOperationExecutionOptions { parallelism: number; terminal: ITerminal; - requestRun?: (requestor?: string) => void; + requestRun?: (requester: string) => void; } /** @@ -77,8 +77,8 @@ export class OperationExecutionManager { for (const dependency of consumer.dependencies) { if (!operations.has(dependency)) { throw new Error( - `Operation ${JSON.stringify(consumer.name)} declares a dependency on operation ` + - `${JSON.stringify(dependency.name)} that is not in the set of operations to execute.` + `Operation ${JSON.stringify(consumer.operationName)} declares a dependency on operation ` + + `${JSON.stringify(dependency.operationName)} that is not in the set of operations to execute.` ); } } diff --git a/libraries/operation-graph/src/OperationGroupRecord.ts b/libraries/operation-graph/src/OperationGroupRecord.ts index bb99ec38ec..a22eb85ceb 100644 --- a/libraries/operation-graph/src/OperationGroupRecord.ts +++ b/libraries/operation-graph/src/OperationGroupRecord.ts @@ -54,7 +54,7 @@ export class OperationGroupRecord { public setOperationAsComplete(operation: Operation, state: IOperationState): void { if (!this._remainingOperations.has(operation)) { - throw new InternalError(`Operation ${operation.name} is not in the group ${this.name}`); + throw new InternalError(`Operation ${operation.operationName} is not in the group ${this.name}`); } if (state.status === OperationStatus.Aborted) { diff --git a/libraries/operation-graph/src/WatchLoop.ts b/libraries/operation-graph/src/WatchLoop.ts index ca911c32bf..971187d9bf 100644 --- a/libraries/operation-graph/src/WatchLoop.ts +++ b/libraries/operation-graph/src/WatchLoop.ts @@ -31,7 +31,7 @@ export interface IWatchLoopOptions { /** * Logging callback when a run is requested (and hasn't already been). */ - onRequestRun: (requestor?: string) => void; + onRequestRun: (requester: string) => void; /** * Logging callback when a run is aborted. */ @@ -45,7 +45,7 @@ export interface IWatchLoopOptions { */ export interface IWatchLoopState { get abortSignal(): AbortSignal; - requestRun: (requestor?: string) => void; + requestRun: (requester: string) => void; } /** @@ -60,7 +60,7 @@ export class WatchLoop implements IWatchLoopState { private _isRunning: boolean; private _runRequested: boolean; private _requestRunPromise: Promise; - private _resolveRequestRun!: (requestor?: string) => void; + private _resolveRequestRun!: (requester: string) => void; public constructor(options: IWatchLoopOptions) { this._options = options; @@ -147,7 +147,7 @@ export class WatchLoop implements IWatchLoopState { } } - function requestRunFromHost(requestor?: string): void { + function requestRunFromHost(requester: string): void { if (runRequestedFromHost) { return; } @@ -192,9 +192,9 @@ export class WatchLoop implements IWatchLoopState { try { status = await this.runUntilStableAsync(abortController.signal); - // ESLINT: "Promises must be awaited, end with a call to .catch, end with a call to .then ..." - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this._requestRunPromise.finally(requestRunFromHost); + this._requestRunPromise.finally(() => { + requestRunFromHost('runIPCAsync'); + }); } catch (err) { status = OperationStatus.Failure; return reject(err); @@ -225,16 +225,16 @@ export class WatchLoop implements IWatchLoopState { /** * Requests that a new run occur. */ - public requestRun: (requestor?: string) => void = (requestor?: string) => { + public requestRun: (requester: string) => void = (requester: string) => { if (!this._runRequested) { - this._options.onRequestRun(requestor); + this._options.onRequestRun(requester); this._runRequested = true; if (this._isRunning) { this._options.onAbort(); this._abortCurrent(); } } - this._resolveRequestRun(requestor); + this._resolveRequestRun(requester); }; /** diff --git a/libraries/operation-graph/src/calculateCriticalPath.ts b/libraries/operation-graph/src/calculateCriticalPath.ts index d00ba11345..c81872346f 100644 --- a/libraries/operation-graph/src/calculateCriticalPath.ts +++ b/libraries/operation-graph/src/calculateCriticalPath.ts @@ -2,7 +2,7 @@ // See LICENSE in the project root for license information. export interface ISortableOperation> { - name: string | undefined; + operationName: string | undefined; criticalPathLength?: number | undefined; weight: number; consumers: Set; @@ -54,7 +54,9 @@ export function calculateShortestPath>( } if (!finalParent) { - throw new Error(`Could not find a path from "${startOperation.name}" to "${endOperation.name}"`); + throw new Error( + `Could not find a path from "${startOperation.operationName}" to "${endOperation.operationName}"` + ); } // Walk back up the path from the end operation to the start operation @@ -81,7 +83,7 @@ export function calculateCriticalPathLength>( throw new Error( 'A cyclic dependency was encountered:\n ' + - shortestPath.map((visitedTask) => visitedTask.name).join('\n -> ') + shortestPath.map((visitedTask) => visitedTask.operationName).join('\n -> ') ); } diff --git a/libraries/operation-graph/src/protocol.types.ts b/libraries/operation-graph/src/protocol.types.ts index 1c8b6293cb..7e297c1445 100644 --- a/libraries/operation-graph/src/protocol.types.ts +++ b/libraries/operation-graph/src/protocol.types.ts @@ -10,7 +10,7 @@ import type { OperationStatus } from './OperationStatus'; */ export interface IRequestRunEventMessage { event: 'requestRun'; - requestor?: string; + requester: string; } /** diff --git a/libraries/operation-graph/src/test/OperationExecutionManager.test.ts b/libraries/operation-graph/src/test/OperationExecutionManager.test.ts index e9230100a1..700e99d8c1 100644 --- a/libraries/operation-graph/src/test/OperationExecutionManager.test.ts +++ b/libraries/operation-graph/src/test/OperationExecutionManager.test.ts @@ -23,10 +23,10 @@ describe(OperationExecutionManager.name, () => { it('throws if a dependency is not in the set', () => { const alpha: Operation = new Operation({ - name: 'alpha' + operationName: 'alpha' }); const beta: Operation = new Operation({ - name: 'beta' + operationName: 'beta' }); alpha.addDependency(beta); @@ -38,10 +38,10 @@ describe(OperationExecutionManager.name, () => { it('sets critical path lengths', () => { const alpha: Operation = new Operation({ - name: 'alpha' + operationName: 'alpha' }); const beta: Operation = new Operation({ - name: 'beta' + operationName: 'beta' }); alpha.addDependency(beta); @@ -73,7 +73,7 @@ describe(OperationExecutionManager.name, () => { it('handles trivial input', async () => { const operation: Operation = new Operation({ - name: 'alpha' + operationName: 'alpha' }); const manager: OperationExecutionManager = new OperationExecutionManager(new Set([operation])); @@ -97,17 +97,17 @@ describe(OperationExecutionManager.name, () => { const runBeta: ExecuteAsyncMock = jest.fn(); const alpha: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync: runAlpha, silent: false } }); const beta: Operation = new Operation({ - name: 'beta', + operationName: 'beta', runner: { - name: 'beta', + operationName: 'beta', executeAsync: runBeta, silent: false } @@ -149,17 +149,17 @@ describe(OperationExecutionManager.name, () => { const runBeta: ExecuteAsyncMock = jest.fn(); const alpha: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync: runAlpha, silent: false } }); const beta: Operation = new Operation({ - name: 'beta', + operationName: 'beta', runner: { - name: 'beta', + operationName: 'beta', executeAsync: runBeta, silent: false } @@ -197,9 +197,9 @@ describe(OperationExecutionManager.name, () => { it('does not track noops', async () => { const operation: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync(): Promise { return Promise.resolve(OperationStatus.NoOp); }, @@ -226,17 +226,17 @@ describe(OperationExecutionManager.name, () => { const runBeta: ExecuteAsyncMock = jest.fn(); const alpha: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync: runAlpha, silent: false } }); const beta: Operation = new Operation({ - name: 'beta', + operationName: 'beta', runner: { - name: 'beta', + operationName: 'beta', executeAsync: runBeta, silent: false } @@ -297,17 +297,17 @@ describe(OperationExecutionManager.name, () => { ); const alpha: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync: run, silent: false } }); const beta: Operation = new Operation({ - name: 'beta', + operationName: 'beta', runner: { - name: 'beta', + operationName: 'beta', executeAsync: run, silent: false } @@ -343,17 +343,17 @@ describe(OperationExecutionManager.name, () => { const requestRun: jest.Mock = jest.fn(); const alpha: Operation = new Operation({ - name: 'alpha', + operationName: 'alpha', runner: { - name: 'alpha', + operationName: 'alpha', executeAsync: runAlpha, silent: false } }); const beta: Operation = new Operation({ - name: 'beta', + operationName: 'beta', runner: { - name: 'beta', + operationName: 'beta', executeAsync: runBeta, silent: false } @@ -402,7 +402,7 @@ describe(OperationExecutionManager.name, () => { betaRequestRun!(); expect(requestRun).toHaveBeenCalledTimes(1); - expect(requestRun).toHaveBeenLastCalledWith(beta.name); + expect(requestRun).toHaveBeenLastCalledWith(beta.operationName); const terminalProvider2: StringBufferTerminalProvider = new StringBufferTerminalProvider(false); const terminal2: ITerminal = new Terminal(terminalProvider2); diff --git a/libraries/operation-graph/src/test/calculateCriticalPath.test.ts b/libraries/operation-graph/src/test/calculateCriticalPath.test.ts index bc45ffb6ce..5212fc8650 100644 --- a/libraries/operation-graph/src/test/calculateCriticalPath.test.ts +++ b/libraries/operation-graph/src/test/calculateCriticalPath.test.ts @@ -20,7 +20,7 @@ function createGraph( if (weights) { for (const [name, weight] of weights) { nodes.set(name, { - name, + operationName: name, weight, consumers: new Set() }); @@ -31,7 +31,7 @@ function createGraph( let node: ITestOperation | undefined = nodes.get(name); if (!node) { node = { - name, + operationName: name, weight: 1, consumers: new Set() }; @@ -60,22 +60,22 @@ describe(calculateShortestPath.name, () => { ]); const result1: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!); - expect(result1.map((x) => x.name)).toMatchSnapshot('long'); + expect(result1.map((x) => x.operationName)).toMatchSnapshot('long'); graph.get('c')!.consumers.add(graph.get('a')!); const result2: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!); - expect(result2.map((x) => x.name)).toMatchSnapshot('with shortcut'); + expect(result2.map((x) => x.operationName)).toMatchSnapshot('with shortcut'); graph.get('f')!.consumers.add(graph.get('c')!); const result3: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!); - expect(result3.map((x) => x.name)).toMatchSnapshot('with multiple shortcuts'); + expect(result3.map((x) => x.operationName)).toMatchSnapshot('with multiple shortcuts'); graph.get('a')!.consumers.add(graph.get('f')!); const result4: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('a')!); - expect(result4.map((x) => x.name)).toMatchSnapshot('with multiple shortcuts (circular)'); + expect(result4.map((x) => x.operationName)).toMatchSnapshot('with multiple shortcuts (circular)'); }); }); From a5cce4c3a990fb55801aa0f7a70b1776f0242168 Mon Sep 17 00:00:00 2001 From: Bart van den Ende Date: Tue, 1 Oct 2024 10:11:42 +0200 Subject: [PATCH 3/4] chore: resolve build issues and add rush change files --- .../octogonz-heft-issue-4467_2024-10-01-08-08.json | 10 ++++++++++ .../octogonz-heft-issue-4467_2024-10-01-08-08.json | 10 ++++++++++ .../octogonz-heft-issue-4467_2024-10-01-08-08.json | 10 ++++++++++ libraries/operation-graph/src/WatchLoop.ts | 11 +++++++---- .../src/logic/operations/IPCOperationRunner.ts | 2 +- 5 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json create mode 100644 common/changes/@rushstack/heft/octogonz-heft-issue-4467_2024-10-01-08-08.json create mode 100644 common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json diff --git a/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json b/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json new file mode 100644 index 0000000000..bd7ff97cb3 --- /dev/null +++ b/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/common/changes/@rushstack/heft/octogonz-heft-issue-4467_2024-10-01-08-08.json b/common/changes/@rushstack/heft/octogonz-heft-issue-4467_2024-10-01-08-08.json new file mode 100644 index 0000000000..62c3633861 --- /dev/null +++ b/common/changes/@rushstack/heft/octogonz-heft-issue-4467_2024-10-01-08-08.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft", + "comment": "Require a logging name for every operation.", + "type": "patch" + } + ], + "packageName": "@rushstack/heft" +} \ No newline at end of file diff --git a/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json b/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json new file mode 100644 index 0000000000..9445a0a96a --- /dev/null +++ b/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/operation-graph", + "comment": "Require a logging name for every operation.", + "type": "patch" + } + ], + "packageName": "@rushstack/operation-graph" +} \ No newline at end of file diff --git a/libraries/operation-graph/src/WatchLoop.ts b/libraries/operation-graph/src/WatchLoop.ts index 971187d9bf..49b16bbb29 100644 --- a/libraries/operation-graph/src/WatchLoop.ts +++ b/libraries/operation-graph/src/WatchLoop.ts @@ -156,7 +156,7 @@ export class WatchLoop implements IWatchLoopState { const requestRunMessage: IRequestRunEventMessage = { event: 'requestRun', - requestor + requester }; tryMessageHost(requestRunMessage); @@ -192,9 +192,12 @@ export class WatchLoop implements IWatchLoopState { try { status = await this.runUntilStableAsync(abortController.signal); - this._requestRunPromise.finally(() => { - requestRunFromHost('runIPCAsync'); - }); + this._requestRunPromise + // the reject callback in the promise is discarded so we ignore errors + .catch(() => {}) + .finally(() => { + requestRunFromHost('runIPCAsync'); + }); } catch (err) { status = OperationStatus.Failure; return reject(err); diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index cf18a24f44..04648fa1ec 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -103,7 +103,7 @@ export class IPCOperationRunner implements IOperationRunner { this._ipcProcess.on('message', (message: unknown) => { if (isRequestRunEventMessage(message)) { - this._requestRun(message.requestor); + this._requestRun(message.requester); } else if (isSyncEventMessage(message)) { resolveReadyPromise(); } From ae8683edc046a326b22e201fca18ba331d59bab8 Mon Sep 17 00:00:00 2001 From: Bart van den Ende Date: Tue, 1 Oct 2024 21:04:10 +0200 Subject: [PATCH 4/4] chore: PR feedback on requester and changelog --- apps/heft/src/cli/HeftActionRunner.ts | 6 +++--- ...ctogonz-heft-issue-4467_2024-10-01-08-08.json | 10 ---------- ...ctogonz-heft-issue-4467_2024-10-01-08-08.json | 4 ++-- common/reviews/api/operation-graph.api.md | 12 ++++++------ libraries/operation-graph/src/Operation.ts | 2 +- .../src/OperationExecutionManager.ts | 2 +- libraries/operation-graph/src/WatchLoop.ts | 16 ++++++++-------- libraries/operation-graph/src/protocol.types.ts | 2 +- .../src/logic/operations/IPCOperationRunner.ts | 2 +- 9 files changed, 23 insertions(+), 33 deletions(-) delete mode 100644 common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json diff --git a/apps/heft/src/cli/HeftActionRunner.ts b/apps/heft/src/cli/HeftActionRunner.ts index 7fd311338b..540b3a754b 100644 --- a/apps/heft/src/cli/HeftActionRunner.ts +++ b/apps/heft/src/cli/HeftActionRunner.ts @@ -333,8 +333,8 @@ export class HeftActionRunner { executeAsync: (state: IWatchLoopState): Promise => { return this._executeOnceAsync(executionManager, state.abortSignal, state.requestRun); }, - onRequestRun: (requester: string) => { - terminal.writeLine(Colorize.bold(`New run requested by ${requester}`)); + onRequestRun: (requestor: string) => { + terminal.writeLine(Colorize.bold(`New run requested by ${requestor}`)); }, onAbort: () => { terminal.writeLine(Colorize.bold(`Cancelling incremental build...`)); @@ -346,7 +346,7 @@ export class HeftActionRunner { private async _executeOnceAsync( executionManager: OperationExecutionManager, abortSignal: AbortSignal, - requestRun?: (requester: string) => void + requestRun?: (requestor: string) => void ): Promise { // Record this as the start of task execution. this._metricsCollector.setStartTime(); diff --git a/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json b/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json deleted file mode 100644 index bd7ff97cb3..0000000000 --- a/common/changes/@microsoft/rush/octogonz-heft-issue-4467_2024-10-01-08-08.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "changes": [ - { - "packageName": "@microsoft/rush", - "comment": "", - "type": "none" - } - ], - "packageName": "@microsoft/rush" -} \ No newline at end of file diff --git a/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json b/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json index 9445a0a96a..ddaadb5ac0 100644 --- a/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json +++ b/common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json @@ -2,8 +2,8 @@ "changes": [ { "packageName": "@rushstack/operation-graph", - "comment": "Require a logging name for every operation.", - "type": "patch" + "comment": "(BREAKING API CHANGE) Require an operationName for every operation to improve logging.", + "type": "minor" } ], "packageName": "@rushstack/operation-graph" diff --git a/common/reviews/api/operation-graph.api.md b/common/reviews/api/operation-graph.api.md index 427caa001f..4e835a1ade 100644 --- a/common/reviews/api/operation-graph.api.md +++ b/common/reviews/api/operation-graph.api.md @@ -33,7 +33,7 @@ export interface IExecuteOperationContext extends Omit Promise, priority: number): Promise; - requestRun?: (requester: string) => void; + requestRun?: (requestor: string) => void; terminal: ITerminal; } @@ -50,7 +50,7 @@ export interface IOperationExecutionOptions { // (undocumented) parallelism: number; // (undocumented) - requestRun?: (requester: string) => void; + requestRun?: (requestor: string) => void; // (undocumented) terminal: ITerminal; } @@ -99,7 +99,7 @@ export interface IRequestRunEventMessage { // (undocumented) event: 'requestRun'; // (undocumented) - requester: string; + requestor: string; } // @beta @@ -127,7 +127,7 @@ export interface IWatchLoopOptions { executeAsync: (state: IWatchLoopState) => Promise; onAbort: () => void; onBeforeExecute: () => void; - onRequestRun: (requester: string) => void; + onRequestRun: (requestor: string) => void; } // @beta @@ -135,7 +135,7 @@ export interface IWatchLoopState { // (undocumented) get abortSignal(): AbortSignal; // (undocumented) - requestRun: (requester: string) => void; + requestRun: (requestor: string) => void; } // @beta @@ -231,7 +231,7 @@ export class Stopwatch { export class WatchLoop implements IWatchLoopState { constructor(options: IWatchLoopOptions); get abortSignal(): AbortSignal; - requestRun: (requester: string) => void; + requestRun: (requestor: string) => void; runIPCAsync(host?: IPCHost): Promise; runUntilAbortedAsync(abortSignal: AbortSignal, onWaiting: () => void): Promise; runUntilStableAsync(abortSignal: AbortSignal): Promise; diff --git a/libraries/operation-graph/src/Operation.ts b/libraries/operation-graph/src/Operation.ts index b85442098d..c1cd78638c 100644 --- a/libraries/operation-graph/src/Operation.ts +++ b/libraries/operation-graph/src/Operation.ts @@ -68,7 +68,7 @@ export interface IExecuteOperationContext extends Omit void; + requestRun?: (requestor: string) => void; /** * Terminal to write output to. diff --git a/libraries/operation-graph/src/OperationExecutionManager.ts b/libraries/operation-graph/src/OperationExecutionManager.ts index ba8aa9de41..5788f8561f 100644 --- a/libraries/operation-graph/src/OperationExecutionManager.ts +++ b/libraries/operation-graph/src/OperationExecutionManager.ts @@ -21,7 +21,7 @@ export interface IOperationExecutionOptions { parallelism: number; terminal: ITerminal; - requestRun?: (requester: string) => void; + requestRun?: (requestor: string) => void; } /** diff --git a/libraries/operation-graph/src/WatchLoop.ts b/libraries/operation-graph/src/WatchLoop.ts index 49b16bbb29..fb59f16a18 100644 --- a/libraries/operation-graph/src/WatchLoop.ts +++ b/libraries/operation-graph/src/WatchLoop.ts @@ -31,7 +31,7 @@ export interface IWatchLoopOptions { /** * Logging callback when a run is requested (and hasn't already been). */ - onRequestRun: (requester: string) => void; + onRequestRun: (requestor: string) => void; /** * Logging callback when a run is aborted. */ @@ -45,7 +45,7 @@ export interface IWatchLoopOptions { */ export interface IWatchLoopState { get abortSignal(): AbortSignal; - requestRun: (requester: string) => void; + requestRun: (requestor: string) => void; } /** @@ -60,7 +60,7 @@ export class WatchLoop implements IWatchLoopState { private _isRunning: boolean; private _runRequested: boolean; private _requestRunPromise: Promise; - private _resolveRequestRun!: (requester: string) => void; + private _resolveRequestRun!: (requestor: string) => void; public constructor(options: IWatchLoopOptions) { this._options = options; @@ -147,7 +147,7 @@ export class WatchLoop implements IWatchLoopState { } } - function requestRunFromHost(requester: string): void { + function requestRunFromHost(requestor: string): void { if (runRequestedFromHost) { return; } @@ -156,7 +156,7 @@ export class WatchLoop implements IWatchLoopState { const requestRunMessage: IRequestRunEventMessage = { event: 'requestRun', - requester + requestor }; tryMessageHost(requestRunMessage); @@ -228,16 +228,16 @@ export class WatchLoop implements IWatchLoopState { /** * Requests that a new run occur. */ - public requestRun: (requester: string) => void = (requester: string) => { + public requestRun: (requestor: string) => void = (requestor: string) => { if (!this._runRequested) { - this._options.onRequestRun(requester); + this._options.onRequestRun(requestor); this._runRequested = true; if (this._isRunning) { this._options.onAbort(); this._abortCurrent(); } } - this._resolveRequestRun(requester); + this._resolveRequestRun(requestor); }; /** diff --git a/libraries/operation-graph/src/protocol.types.ts b/libraries/operation-graph/src/protocol.types.ts index 7e297c1445..60100afca4 100644 --- a/libraries/operation-graph/src/protocol.types.ts +++ b/libraries/operation-graph/src/protocol.types.ts @@ -10,7 +10,7 @@ import type { OperationStatus } from './OperationStatus'; */ export interface IRequestRunEventMessage { event: 'requestRun'; - requester: string; + requestor: string; } /** diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 04648fa1ec..cf18a24f44 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -103,7 +103,7 @@ export class IPCOperationRunner implements IOperationRunner { this._ipcProcess.on('message', (message: unknown) => { if (isRequestRunEventMessage(message)) { - this._requestRun(message.requester); + this._requestRun(message.requestor); } else if (isSyncEventMessage(message)) { resolveReadyPromise(); }