Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e3647d3

Browse files
octogonzbartvandenende-wm
octogonz
authored and
bartvandenende-wm
committedOct 1, 2024
- 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
1 parent 2152622 commit e3647d3

13 files changed

+78
-74
lines changed
 

‎apps/heft/src/cli/HeftActionRunner.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,8 @@ export class HeftActionRunner {
333333
executeAsync: (state: IWatchLoopState): Promise<OperationStatus> => {
334334
return this._executeOnceAsync(executionManager, state.abortSignal, state.requestRun);
335335
},
336-
onRequestRun: (requestor?: string) => {
337-
terminal.writeLine(Colorize.bold(`New run requested by ${requestor || 'unknown task'}`));
336+
onRequestRun: (requester: string) => {
337+
terminal.writeLine(Colorize.bold(`New run requested by ${requester}`));
338338
},
339339
onAbort: () => {
340340
terminal.writeLine(Colorize.bold(`Cancelling incremental build...`));
@@ -346,7 +346,7 @@ export class HeftActionRunner {
346346
private async _executeOnceAsync(
347347
executionManager: OperationExecutionManager,
348348
abortSignal: AbortSignal,
349-
requestRun?: (requestor?: string) => void
349+
requestRun?: (requester: string) => void
350350
): Promise<OperationStatus> {
351351
// Record this as the start of task execution.
352352
this._metricsCollector.setStartTime();
@@ -447,7 +447,7 @@ function _getOrCreatePhaseOperation(
447447
// Only create the operation. Dependencies are hooked up separately
448448
operation = new Operation({
449449
groupName: phase.phaseName,
450-
name: `${phase.phaseName} phase`,
450+
operationName: `${phase.phaseName} phase`,
451451
runner: new PhaseOperationRunner({ phase, internalHeftSession })
452452
});
453453
operations.set(key, operation);
@@ -467,7 +467,7 @@ function _getOrCreateTaskOperation(
467467
if (!operation) {
468468
operation = new Operation({
469469
groupName: task.parentPhase.phaseName,
470-
name: `${task.taskName} task`,
470+
operationName: `${task.taskName} task`,
471471
runner: new TaskOperationRunner({
472472
internalHeftSession,
473473
task

‎apps/heft/src/operations/runners/PhaseOperationRunner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export class PhaseOperationRunner implements IOperationRunner {
2323
private readonly _options: IPhaseOperationRunnerOptions;
2424
private _isClean: boolean = false;
2525

26-
public get name(): string {
26+
public get operationName(): string {
2727
return `Phase ${JSON.stringify(this._options.phase.phaseName)}`;
2828
}
2929

‎apps/heft/src/operations/runners/TaskOperationRunner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class TaskOperationRunner implements IOperationRunner {
5555

5656
public readonly silent: boolean = false;
5757

58-
public get name(): string {
58+
public get operationName(): string {
5959
const { taskName, parentPhase } = this._options.task;
6060
return `Task ${JSON.stringify(taskName)} of phase ${JSON.stringify(parentPhase.phaseName)}`;
6161
}

‎common/reviews/api/operation-graph.api.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export interface IExecuteOperationContext extends Omit<IOperationRunnerContext,
3333
afterExecute(operation: Operation, state: IOperationState): void;
3434
beforeExecute(operation: Operation, state: IOperationState): void;
3535
queueWork(workFn: () => Promise<OperationStatus>, priority: number): Promise<OperationStatus>;
36-
requestRun?: (requestor?: string) => void;
36+
requestRun?: (requester: string) => void;
3737
terminal: ITerminal;
3838
}
3939

@@ -50,23 +50,23 @@ export interface IOperationExecutionOptions {
5050
// (undocumented)
5151
parallelism: number;
5252
// (undocumented)
53-
requestRun?: (requestor?: string) => void;
53+
requestRun?: (requester: string) => void;
5454
// (undocumented)
5555
terminal: ITerminal;
5656
}
5757

5858
// @beta
5959
export interface IOperationOptions {
6060
groupName?: string | undefined;
61-
name: string;
61+
operationName: string;
6262
runner?: IOperationRunner | undefined;
6363
weight?: number | undefined;
6464
}
6565

6666
// @beta
6767
export interface IOperationRunner {
6868
executeAsync(context: IOperationRunnerContext): Promise<OperationStatus>;
69-
readonly name: string;
69+
readonly operationName: string;
7070
silent: boolean;
7171
}
7272

@@ -99,7 +99,7 @@ export interface IRequestRunEventMessage {
9999
// (undocumented)
100100
event: 'requestRun';
101101
// (undocumented)
102-
requestor?: string;
102+
requester: string;
103103
}
104104

105105
// @beta
@@ -127,15 +127,15 @@ export interface IWatchLoopOptions {
127127
executeAsync: (state: IWatchLoopState) => Promise<OperationStatus>;
128128
onAbort: () => void;
129129
onBeforeExecute: () => void;
130-
onRequestRun: (requestor?: string) => void;
130+
onRequestRun: (requester: string) => void;
131131
}
132132

133133
// @beta
134134
export interface IWatchLoopState {
135135
// (undocumented)
136136
get abortSignal(): AbortSignal;
137137
// (undocumented)
138-
requestRun: (requestor?: string) => void;
138+
requestRun: (requester: string) => void;
139139
}
140140

141141
// @beta
@@ -152,7 +152,7 @@ export class Operation implements IOperationStates {
152152
_executeAsync(context: IExecuteOperationContext): Promise<OperationStatus>;
153153
readonly groupName: string | undefined;
154154
lastState: IOperationState | undefined;
155-
readonly name: string;
155+
readonly operationName: string;
156156
// (undocumented)
157157
reset(): void;
158158
runner: IOperationRunner | undefined;
@@ -231,7 +231,7 @@ export class Stopwatch {
231231
export class WatchLoop implements IWatchLoopState {
232232
constructor(options: IWatchLoopOptions);
233233
get abortSignal(): AbortSignal;
234-
requestRun: (requestor?: string) => void;
234+
requestRun: (requester: string) => void;
235235
runIPCAsync(host?: IPCHost): Promise<void>;
236236
runUntilAbortedAsync(abortSignal: AbortSignal, onWaiting: () => void): Promise<void>;
237237
runUntilStableAsync(abortSignal: AbortSignal): Promise<OperationStatus>;

‎libraries/operation-graph/src/IOperationRunner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export interface IOperationRunner {
8181
/**
8282
* Name of the operation, for logging.
8383
*/
84-
readonly name: string;
84+
readonly operationName: string;
8585

8686
/**
8787
* Indicates that this runner is architectural and should not be reported on.

‎libraries/operation-graph/src/Operation.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface IOperationOptions {
2222
/**
2323
* The name of this operation, for logging.
2424
*/
25-
name: string;
25+
operationName: string;
2626

2727
/**
2828
* The group that this operation belongs to. Will be used for logging and duration tracking.
@@ -68,7 +68,7 @@ export interface IExecuteOperationContext extends Omit<IOperationRunnerContext,
6868
* A callback to the overarching orchestrator to request that the operation be invoked again.
6969
* Used in watch mode to signal that inputs have changed.
7070
*/
71-
requestRun?: (requestor?: string) => void;
71+
requestRun?: (requester: string) => void;
7272

7373
/**
7474
* Terminal to write output to.
@@ -101,7 +101,7 @@ export class Operation implements IOperationStates {
101101
/**
102102
* The name of this operation, for logging.
103103
*/
104-
public readonly name: string;
104+
public readonly operationName: string;
105105

106106
/**
107107
* 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 {
178178
this.groupName = options?.groupName;
179179
this.runner = options?.runner;
180180
this.weight = options?.weight || 1;
181-
this.name = options.name;
181+
this.operationName = options.operationName;
182182
}
183183

184184
public addDependency(dependency: Operation): void {
@@ -280,7 +280,9 @@ export class Operation implements IOperationStates {
280280
// The requestRun callback is assumed to remain constant
281281
// throughout the lifetime of the process, so it is safe
282282
// to capture here.
283-
return requestRun(this.name);
283+
return requestRun(this.operationName);
284+
case undefined:
285+
throw new InternalError(`The operation state is undefined`);
284286
default:
285287
// This line is here to enforce exhaustiveness
286288
const currentStatus: undefined = this.state?.status;

‎libraries/operation-graph/src/OperationExecutionManager.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export interface IOperationExecutionOptions {
2121
parallelism: number;
2222
terminal: ITerminal;
2323

24-
requestRun?: (requestor?: string) => void;
24+
requestRun?: (requester: string) => void;
2525
}
2626

2727
/**
@@ -77,8 +77,8 @@ export class OperationExecutionManager {
7777
for (const dependency of consumer.dependencies) {
7878
if (!operations.has(dependency)) {
7979
throw new Error(
80-
`Operation ${JSON.stringify(consumer.name)} declares a dependency on operation ` +
81-
`${JSON.stringify(dependency.name)} that is not in the set of operations to execute.`
80+
`Operation ${JSON.stringify(consumer.operationName)} declares a dependency on operation ` +
81+
`${JSON.stringify(dependency.operationName)} that is not in the set of operations to execute.`
8282
);
8383
}
8484
}

‎libraries/operation-graph/src/OperationGroupRecord.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class OperationGroupRecord {
5454

5555
public setOperationAsComplete(operation: Operation, state: IOperationState): void {
5656
if (!this._remainingOperations.has(operation)) {
57-
throw new InternalError(`Operation ${operation.name} is not in the group ${this.name}`);
57+
throw new InternalError(`Operation ${operation.operationName} is not in the group ${this.name}`);
5858
}
5959

6060
if (state.status === OperationStatus.Aborted) {

‎libraries/operation-graph/src/WatchLoop.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export interface IWatchLoopOptions {
3131
/**
3232
* Logging callback when a run is requested (and hasn't already been).
3333
*/
34-
onRequestRun: (requestor?: string) => void;
34+
onRequestRun: (requester: string) => void;
3535
/**
3636
* Logging callback when a run is aborted.
3737
*/
@@ -45,7 +45,7 @@ export interface IWatchLoopOptions {
4545
*/
4646
export interface IWatchLoopState {
4747
get abortSignal(): AbortSignal;
48-
requestRun: (requestor?: string) => void;
48+
requestRun: (requester: string) => void;
4949
}
5050

5151
/**
@@ -60,7 +60,7 @@ export class WatchLoop implements IWatchLoopState {
6060
private _isRunning: boolean;
6161
private _runRequested: boolean;
6262
private _requestRunPromise: Promise<string | undefined>;
63-
private _resolveRequestRun!: (requestor?: string) => void;
63+
private _resolveRequestRun!: (requester: string) => void;
6464

6565
public constructor(options: IWatchLoopOptions) {
6666
this._options = options;
@@ -147,7 +147,7 @@ export class WatchLoop implements IWatchLoopState {
147147
}
148148
}
149149

150-
function requestRunFromHost(requestor?: string): void {
150+
function requestRunFromHost(requester: string): void {
151151
if (runRequestedFromHost) {
152152
return;
153153
}
@@ -192,9 +192,9 @@ export class WatchLoop implements IWatchLoopState {
192192

193193
try {
194194
status = await this.runUntilStableAsync(abortController.signal);
195-
// ESLINT: "Promises must be awaited, end with a call to .catch, end with a call to .then ..."
196-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
197-
this._requestRunPromise.finally(requestRunFromHost);
195+
this._requestRunPromise.finally(() => {
196+
requestRunFromHost('runIPCAsync');
197+
});
198198
} catch (err) {
199199
status = OperationStatus.Failure;
200200
return reject(err);
@@ -225,16 +225,16 @@ export class WatchLoop implements IWatchLoopState {
225225
/**
226226
* Requests that a new run occur.
227227
*/
228-
public requestRun: (requestor?: string) => void = (requestor?: string) => {
228+
public requestRun: (requester: string) => void = (requester: string) => {
229229
if (!this._runRequested) {
230-
this._options.onRequestRun(requestor);
230+
this._options.onRequestRun(requester);
231231
this._runRequested = true;
232232
if (this._isRunning) {
233233
this._options.onAbort();
234234
this._abortCurrent();
235235
}
236236
}
237-
this._resolveRequestRun(requestor);
237+
this._resolveRequestRun(requester);
238238
};
239239

240240
/**

‎libraries/operation-graph/src/calculateCriticalPath.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// See LICENSE in the project root for license information.
33

44
export interface ISortableOperation<T extends ISortableOperation<T>> {
5-
name: string | undefined;
5+
operationName: string | undefined;
66
criticalPathLength?: number | undefined;
77
weight: number;
88
consumers: Set<T>;
@@ -54,7 +54,9 @@ export function calculateShortestPath<T extends ISortableOperation<T>>(
5454
}
5555

5656
if (!finalParent) {
57-
throw new Error(`Could not find a path from "${startOperation.name}" to "${endOperation.name}"`);
57+
throw new Error(
58+
`Could not find a path from "${startOperation.operationName}" to "${endOperation.operationName}"`
59+
);
5860
}
5961

6062
// Walk back up the path from the end operation to the start operation
@@ -81,7 +83,7 @@ export function calculateCriticalPathLength<T extends ISortableOperation<T>>(
8183

8284
throw new Error(
8385
'A cyclic dependency was encountered:\n ' +
84-
shortestPath.map((visitedTask) => visitedTask.name).join('\n -> ')
86+
shortestPath.map((visitedTask) => visitedTask.operationName).join('\n -> ')
8587
);
8688
}
8789

‎libraries/operation-graph/src/protocol.types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { OperationStatus } from './OperationStatus';
1010
*/
1111
export interface IRequestRunEventMessage {
1212
event: 'requestRun';
13-
requestor?: string;
13+
requester: string;
1414
}
1515

1616
/**

‎libraries/operation-graph/src/test/OperationExecutionManager.test.ts

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ describe(OperationExecutionManager.name, () => {
2323

2424
it('throws if a dependency is not in the set', () => {
2525
const alpha: Operation = new Operation({
26-
name: 'alpha'
26+
operationName: 'alpha'
2727
});
2828
const beta: Operation = new Operation({
29-
name: 'beta'
29+
operationName: 'beta'
3030
});
3131

3232
alpha.addDependency(beta);
@@ -38,10 +38,10 @@ describe(OperationExecutionManager.name, () => {
3838

3939
it('sets critical path lengths', () => {
4040
const alpha: Operation = new Operation({
41-
name: 'alpha'
41+
operationName: 'alpha'
4242
});
4343
const beta: Operation = new Operation({
44-
name: 'beta'
44+
operationName: 'beta'
4545
});
4646

4747
alpha.addDependency(beta);
@@ -73,7 +73,7 @@ describe(OperationExecutionManager.name, () => {
7373

7474
it('handles trivial input', async () => {
7575
const operation: Operation = new Operation({
76-
name: 'alpha'
76+
operationName: 'alpha'
7777
});
7878
const manager: OperationExecutionManager = new OperationExecutionManager(new Set([operation]));
7979

@@ -97,17 +97,17 @@ describe(OperationExecutionManager.name, () => {
9797
const runBeta: ExecuteAsyncMock = jest.fn();
9898

9999
const alpha: Operation = new Operation({
100-
name: 'alpha',
100+
operationName: 'alpha',
101101
runner: {
102-
name: 'alpha',
102+
operationName: 'alpha',
103103
executeAsync: runAlpha,
104104
silent: false
105105
}
106106
});
107107
const beta: Operation = new Operation({
108-
name: 'beta',
108+
operationName: 'beta',
109109
runner: {
110-
name: 'beta',
110+
operationName: 'beta',
111111
executeAsync: runBeta,
112112
silent: false
113113
}
@@ -149,17 +149,17 @@ describe(OperationExecutionManager.name, () => {
149149
const runBeta: ExecuteAsyncMock = jest.fn();
150150

151151
const alpha: Operation = new Operation({
152-
name: 'alpha',
152+
operationName: 'alpha',
153153
runner: {
154-
name: 'alpha',
154+
operationName: 'alpha',
155155
executeAsync: runAlpha,
156156
silent: false
157157
}
158158
});
159159
const beta: Operation = new Operation({
160-
name: 'beta',
160+
operationName: 'beta',
161161
runner: {
162-
name: 'beta',
162+
operationName: 'beta',
163163
executeAsync: runBeta,
164164
silent: false
165165
}
@@ -197,9 +197,9 @@ describe(OperationExecutionManager.name, () => {
197197

198198
it('does not track noops', async () => {
199199
const operation: Operation = new Operation({
200-
name: 'alpha',
200+
operationName: 'alpha',
201201
runner: {
202-
name: 'alpha',
202+
operationName: 'alpha',
203203
executeAsync(): Promise<OperationStatus> {
204204
return Promise.resolve(OperationStatus.NoOp);
205205
},
@@ -226,17 +226,17 @@ describe(OperationExecutionManager.name, () => {
226226
const runBeta: ExecuteAsyncMock = jest.fn();
227227

228228
const alpha: Operation = new Operation({
229-
name: 'alpha',
229+
operationName: 'alpha',
230230
runner: {
231-
name: 'alpha',
231+
operationName: 'alpha',
232232
executeAsync: runAlpha,
233233
silent: false
234234
}
235235
});
236236
const beta: Operation = new Operation({
237-
name: 'beta',
237+
operationName: 'beta',
238238
runner: {
239-
name: 'beta',
239+
operationName: 'beta',
240240
executeAsync: runBeta,
241241
silent: false
242242
}
@@ -297,17 +297,17 @@ describe(OperationExecutionManager.name, () => {
297297
);
298298

299299
const alpha: Operation = new Operation({
300-
name: 'alpha',
300+
operationName: 'alpha',
301301
runner: {
302-
name: 'alpha',
302+
operationName: 'alpha',
303303
executeAsync: run,
304304
silent: false
305305
}
306306
});
307307
const beta: Operation = new Operation({
308-
name: 'beta',
308+
operationName: 'beta',
309309
runner: {
310-
name: 'beta',
310+
operationName: 'beta',
311311
executeAsync: run,
312312
silent: false
313313
}
@@ -343,17 +343,17 @@ describe(OperationExecutionManager.name, () => {
343343
const requestRun: jest.Mock = jest.fn();
344344

345345
const alpha: Operation = new Operation({
346-
name: 'alpha',
346+
operationName: 'alpha',
347347
runner: {
348-
name: 'alpha',
348+
operationName: 'alpha',
349349
executeAsync: runAlpha,
350350
silent: false
351351
}
352352
});
353353
const beta: Operation = new Operation({
354-
name: 'beta',
354+
operationName: 'beta',
355355
runner: {
356-
name: 'beta',
356+
operationName: 'beta',
357357
executeAsync: runBeta,
358358
silent: false
359359
}
@@ -402,7 +402,7 @@ describe(OperationExecutionManager.name, () => {
402402
betaRequestRun!();
403403

404404
expect(requestRun).toHaveBeenCalledTimes(1);
405-
expect(requestRun).toHaveBeenLastCalledWith(beta.name);
405+
expect(requestRun).toHaveBeenLastCalledWith(beta.operationName);
406406

407407
const terminalProvider2: StringBufferTerminalProvider = new StringBufferTerminalProvider(false);
408408
const terminal2: ITerminal = new Terminal(terminalProvider2);

‎libraries/operation-graph/src/test/calculateCriticalPath.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function createGraph(
2020
if (weights) {
2121
for (const [name, weight] of weights) {
2222
nodes.set(name, {
23-
name,
23+
operationName: name,
2424
weight,
2525
consumers: new Set()
2626
});
@@ -31,7 +31,7 @@ function createGraph(
3131
let node: ITestOperation | undefined = nodes.get(name);
3232
if (!node) {
3333
node = {
34-
name,
34+
operationName: name,
3535
weight: 1,
3636
consumers: new Set()
3737
};
@@ -60,22 +60,22 @@ describe(calculateShortestPath.name, () => {
6060
]);
6161

6262
const result1: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!);
63-
expect(result1.map((x) => x.name)).toMatchSnapshot('long');
63+
expect(result1.map((x) => x.operationName)).toMatchSnapshot('long');
6464

6565
graph.get('c')!.consumers.add(graph.get('a')!);
6666

6767
const result2: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!);
68-
expect(result2.map((x) => x.name)).toMatchSnapshot('with shortcut');
68+
expect(result2.map((x) => x.operationName)).toMatchSnapshot('with shortcut');
6969

7070
graph.get('f')!.consumers.add(graph.get('c')!);
7171

7272
const result3: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('f')!);
73-
expect(result3.map((x) => x.name)).toMatchSnapshot('with multiple shortcuts');
73+
expect(result3.map((x) => x.operationName)).toMatchSnapshot('with multiple shortcuts');
7474

7575
graph.get('a')!.consumers.add(graph.get('f')!);
7676

7777
const result4: ITestOperation[] = calculateShortestPath(graph.get('a')!, graph.get('a')!);
78-
expect(result4.map((x) => x.name)).toMatchSnapshot('with multiple shortcuts (circular)');
78+
expect(result4.map((x) => x.operationName)).toMatchSnapshot('with multiple shortcuts (circular)');
7979
});
8080
});
8181

0 commit comments

Comments
 (0)
Please sign in to comment.