Skip to content

Commit 2c8a7d4

Browse files
authored
Merge pull request microsoft#1070 from hogmoru/taskrunner-tests
[rush] Rework TaskRunner to use Terminal API, add tests
2 parents fd35a0a + 3d60203 commit 2c8a7d4

10 files changed

Lines changed: 232 additions & 37 deletions

File tree

apps/rush-lib/src/logic/taskRunner/TaskRunner.ts

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
import * as colors from 'colors';
55
import * as os from 'os';
66
import { Interleaver } from '@microsoft/stream-collator';
7+
import {
8+
Terminal, ConsoleTerminalProvider, ITerminalProvider
9+
} from '@microsoft/node-core-library';
710

811
import { Stopwatch } from '../../utilities/Stopwatch';
912
import { ITask, ITaskDefinition } from './ITask';
@@ -15,9 +18,7 @@ import { TaskError } from './TaskError';
1518
* Any class of task definition may be registered, and dependencies between tasks are
1619
* easily specified. Initially, and at the end of each task execution, all unblocked tasks
1720
* are added to a ready queue which is then executed. This is done continually until all
18-
* tasks are complete, or prematurely fails if any of the tasks fail. Note that all task
19-
* definitions must
20-
* @todo #168352: add unit tests
21+
* tasks are complete, or prematurely fails if any of the tasks fail.
2122
*/
2223
export class TaskRunner {
2324
private _tasks: Map<string, ITask>;
@@ -29,15 +30,20 @@ export class TaskRunner {
2930
private _currentActiveTasks: number;
3031
private _totalTasks: number;
3132
private _completedTasks: number;
32-
33-
constructor(quietMode: boolean,
34-
parallelism: string |undefined,
35-
changedProjectsOnly: boolean) {
33+
private _terminal: Terminal;
34+
35+
constructor(
36+
quietMode: boolean,
37+
parallelism: string | undefined,
38+
changedProjectsOnly: boolean,
39+
customTerminal?: ITerminalProvider
40+
) {
3641
this._tasks = new Map<string, ITask>();
3742
this._buildQueue = [];
3843
this._quietMode = quietMode;
3944
this._hasAnyFailures = false;
4045
this._changedProjectsOnly = changedProjectsOnly;
46+
this._terminal = new Terminal(customTerminal || new ConsoleTerminalProvider());
4147

4248
const numberOfCores: number = os.cpus().length;
4349

@@ -53,8 +59,7 @@ export class TaskRunner {
5359

5460
this._parallelism = parallelismInt;
5561
}
56-
57-
} else {
62+
} else {
5863
// If an explicit parallelism number wasn't provided, then choose a sensible
5964
// default.
6065
if (os.platform() === 'win32') {
@@ -86,7 +91,7 @@ export class TaskRunner {
8691
this._tasks.set(task.name, task);
8792

8893
if (!this._quietMode) {
89-
console.log(`Registered ${task.name}`);
94+
this._terminal.writeLine(`Registered ${task.name}`);
9095
}
9196
}
9297

@@ -131,7 +136,7 @@ export class TaskRunner {
131136
this._currentActiveTasks = 0;
132137
this._completedTasks = 0;
133138
this._totalTasks = this._tasks.size;
134-
console.log(`Executing a maximum of ${this._parallelism} simultaneous processes...${os.EOL}`);
139+
this._terminal.writeLine(`Executing a maximum of ${this._parallelism} simultaneous processes...${os.EOL}`);
135140

136141
this._checkForCyclicDependencies(this._tasks.values(), []);
137142

@@ -194,7 +199,7 @@ export class TaskRunner {
194199
this._currentActiveTasks++;
195200
const task: ITask = ctask;
196201
task.status = TaskStatus.Executing;
197-
console.log(colors.white(`[${task.name}] started`));
202+
this._terminal.writeLine(colors.white(`[${task.name}] started`));
198203

199204
task.stopwatch = Stopwatch.start();
200205
task.writer = Interleaver.registerTask(task.name, this._quietMode);
@@ -230,7 +235,7 @@ export class TaskRunner {
230235
task.error = error;
231236
this._markTaskAsFailed(task);
232237
}
233-
).then(() => this._startAvailableTasks()));
238+
).then(() => this._startAvailableTasks()));
234239
}
235240

236241
return Promise.all(taskPromises).then(() => { /* collapse void[] to void */ });
@@ -240,7 +245,7 @@ export class TaskRunner {
240245
* Marks a task as having failed and marks each of its dependents as blocked
241246
*/
242247
private _markTaskAsFailed(task: ITask): void {
243-
console.log(colors.red(`${os.EOL}${this._getCurrentCompletedTaskString()}[${task.name}] failed to build!`));
248+
this._terminal.writeErrorLine(`${os.EOL}${this._getCurrentCompletedTaskString()}[${task.name}] failed to build!`);
244249
task.status = TaskStatus.Failure;
245250
task.dependents.forEach((dependent: ITask) => {
246251
this._markTaskAsBlocked(dependent, task);
@@ -253,8 +258,8 @@ export class TaskRunner {
253258
private _markTaskAsBlocked(task: ITask, failedTask: ITask): void {
254259
if (task.status === TaskStatus.Ready) {
255260
this._completedTasks++;
256-
console.log(colors.red(`${this._getCurrentCompletedTaskString()}`
257-
+ `[${task.name}] blocked by [${failedTask.name}]!`));
261+
this._terminal.writeErrorLine(`${this._getCurrentCompletedTaskString()}`
262+
+ `[${task.name}] blocked by [${failedTask.name}]!`);
258263
task.status = TaskStatus.Blocked;
259264
task.dependents.forEach((dependent: ITask) => {
260265
this._markTaskAsBlocked(dependent, failedTask);
@@ -266,7 +271,7 @@ export class TaskRunner {
266271
* Marks a task as being completed, and removes it from the dependencies list of all its dependents
267272
*/
268273
private _markTaskAsSuccess(task: ITask): void {
269-
console.log(colors.green(`${this._getCurrentCompletedTaskString()}`
274+
this._terminal.writeLine(colors.green(`${this._getCurrentCompletedTaskString()}`
270275
+ `[${task.name}] completed successfully in ${task.stopwatch.toString()}`));
271276
task.status = TaskStatus.Success;
272277

@@ -283,8 +288,8 @@ export class TaskRunner {
283288
* list of all its dependents
284289
*/
285290
private _markTaskAsSuccessWithWarning(task: ITask): void {
286-
console.log(colors.yellow(`${this._getCurrentCompletedTaskString()}`
287-
+ `[${task.name}] completed with warnings in ${task.stopwatch.toString()}`));
291+
this._terminal.writeWarningLine(`${this._getCurrentCompletedTaskString()}`
292+
+ `[${task.name}] completed with warnings in ${task.stopwatch.toString()}`);
288293
task.status = TaskStatus.SuccessWithWarning;
289294
task.dependents.forEach((dependent: ITask) => {
290295
if (!this._changedProjectsOnly) {
@@ -298,7 +303,7 @@ export class TaskRunner {
298303
* Marks a task as skipped.
299304
*/
300305
private _markTaskAsSkipped(task: ITask): void {
301-
console.log(colors.green(`${this._getCurrentCompletedTaskString()}[${task.name}] skipped`));
306+
this._terminal.writeLine(colors.green(`${this._getCurrentCompletedTaskString()}[${task.name}] skipped`));
302307
task.status = TaskStatus.Skipped;
303308
task.dependents.forEach((dependent: ITask) => {
304309
dependent.dependencies.delete(task);
@@ -330,7 +335,6 @@ export class TaskRunner {
330335
* the furthest away "root" node
331336
*/
332337
private _calculateCriticalPaths(task: ITask): number {
333-
334338
// Return the memoized value
335339
if (task.criticalPathLength !== undefined) {
336340
return task.criticalPathLength;
@@ -360,7 +364,7 @@ export class TaskRunner {
360364
}
361365
});
362366

363-
console.log('');
367+
this._terminal.writeLine('');
364368

365369
this._printStatus(TaskStatus.Executing, tasksByStatus, colors.yellow);
366370
this._printStatus(TaskStatus.Ready, tasksByStatus, colors.white);
@@ -374,12 +378,12 @@ export class TaskRunner {
374378
if (tasksWithErrors) {
375379
tasksWithErrors.forEach((task: ITask) => {
376380
if (task.error) {
377-
console.log(colors.red(`[${task.name}] ${task.error.message}`));
381+
this._terminal.writeErrorLine(`[${task.name}] ${task.error.message}`);
378382
}
379383
});
380384
}
381385

382-
console.log('');
386+
this._terminal.writeLine('');
383387
}
384388

385389
private _printStatus(
@@ -390,16 +394,16 @@ export class TaskRunner {
390394
const tasks: ITask[] = tasksByStatus[status];
391395

392396
if (tasks && tasks.length) {
393-
console.log(color(`${status} (${tasks.length})`));
394-
console.log(color('================================'));
397+
this._terminal.writeLine(color(`${status} (${tasks.length})`));
398+
this._terminal.writeLine(color('================================'));
395399
for (let i: number = 0; i < tasks.length; i++) {
396400
const task: ITask = tasks[i];
397401

398402
switch (status) {
399403
case TaskStatus.Executing:
400404
case TaskStatus.Ready:
401405
case TaskStatus.Skipped:
402-
console.log(color(task.name));
406+
this._terminal.writeLine(color(task.name));
403407
break;
404408

405409
case TaskStatus.Success:
@@ -408,9 +412,9 @@ export class TaskRunner {
408412
case TaskStatus.Failure:
409413
if (task.stopwatch) {
410414
const time: string = task.stopwatch.toString();
411-
console.log(color(`${task.name} (${time})`));
415+
this._terminal.writeLine(color(`${task.name} (${time})`));
412416
} else {
413-
console.log(color(`${task.name}`));
417+
this._terminal.writeLine(color(`${task.name}`));
414418
}
415419
break;
416420
}
@@ -422,13 +426,13 @@ export class TaskRunner {
422426
.map(text => text.trim())
423427
.filter(text => text)
424428
.join(os.EOL);
425-
426-
console.log(stderr + (i !== tasks.length - 1 ? os.EOL : ''));
429+
this._terminal.writeLine(stderr + (i !== tasks.length - 1 ? os.EOL : ''));
427430
}
428431
}
429432
}
430433

431-
console.log(color('================================' + os.EOL));
434+
this._terminal.writeLine(color('================================' + os.EOL));
432435
}
433436
}
437+
434438
}
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { EOL } from 'os';
2+
import { TaskRunner } from '../TaskRunner';
3+
import { ITaskWriter } from '@microsoft/stream-collator';
4+
import { TaskStatus } from '../TaskStatus';
5+
import { ITaskDefinition } from '../ITask';
6+
import { StringBufferTerminalProvider } from '@microsoft/node-core-library';
7+
8+
function createDummyTask(name: string, action?: () => void): ITaskDefinition {
9+
return {
10+
name,
11+
isIncrementalBuildAllowed: false,
12+
execute: (writer: ITaskWriter) => {
13+
if (action) {
14+
action();
15+
}
16+
return Promise.resolve(TaskStatus.Success);
17+
}
18+
};
19+
}
20+
21+
function checkConsoleOutput(logger: StringBufferTerminalProvider): void {
22+
expect(logger.getOutput()).toMatchSnapshot();
23+
expect(logger.getVerbose()).toMatchSnapshot();
24+
expect(logger.getWarningOutput()).toMatchSnapshot();
25+
expect(logger.getErrorOutput()).toMatchSnapshot();
26+
}
27+
28+
describe('TaskRunner', () => {
29+
let logger: StringBufferTerminalProvider;
30+
let taskRunner: TaskRunner;
31+
32+
describe('Constructor', () => {
33+
it('throwsErrorOnInvalidParallelism', () => {
34+
logger = new StringBufferTerminalProvider(true);
35+
expect(() => new TaskRunner(false, 'tequila', false, logger))
36+
.toThrowErrorMatchingSnapshot();
37+
});
38+
});
39+
40+
describe('Dependencies', () => {
41+
beforeEach(() => {
42+
logger = new StringBufferTerminalProvider(true);
43+
taskRunner = new TaskRunner(false, '1', false, logger);
44+
});
45+
46+
it('throwsErrorOnNonExistentTask', () => {
47+
expect(() => taskRunner.addDependencies('foo', []))
48+
.toThrowErrorMatchingSnapshot();
49+
});
50+
51+
it('throwsErrorOnNonExistentDependency', () => {
52+
taskRunner.addTask(createDummyTask('foo'));
53+
expect(() => taskRunner.addDependencies('foo', ['bar']))
54+
.toThrowErrorMatchingSnapshot();
55+
});
56+
57+
it('detectsDependencyCycle', () => {
58+
taskRunner.addTask(createDummyTask('foo'));
59+
taskRunner.addTask(createDummyTask('bar'));
60+
taskRunner.addDependencies('foo', ['bar']);
61+
taskRunner.addDependencies('bar', ['foo']);
62+
expect(() => taskRunner.execute()).toThrowErrorMatchingSnapshot();
63+
});
64+
65+
it('respectsDependencyOrder', () => {
66+
const result: Array<string> = [];
67+
taskRunner.addTask(createDummyTask('two', () => result.push('2')));
68+
taskRunner.addTask(createDummyTask('one', () => result.push('1')));
69+
taskRunner.addDependencies('two', ['one']);
70+
return taskRunner
71+
.execute()
72+
.then(() => {
73+
expect(result.join(',')).toEqual('1,2');
74+
checkConsoleOutput(logger);
75+
})
76+
.catch(error => fail(error));
77+
});
78+
});
79+
80+
describe('Error logging', () => {
81+
beforeEach(() => {
82+
logger = new StringBufferTerminalProvider(true);
83+
taskRunner = new TaskRunner(false, '1', false, logger);
84+
});
85+
86+
const EXPECTED_FAIL: string = 'Promise returned by execute() resolved but was expected to fail';
87+
88+
it('printedStderrAfterError', () => {
89+
taskRunner.addTask({
90+
name: 'stdout+stderr',
91+
isIncrementalBuildAllowed: false,
92+
execute: (writer: ITaskWriter) => {
93+
writer.write('Hold my beer...' + EOL);
94+
writer.writeError('Woops' + EOL);
95+
return Promise.resolve(TaskStatus.Failure);
96+
}
97+
});
98+
return taskRunner
99+
.execute()
100+
.then(() => fail(EXPECTED_FAIL))
101+
.catch(err => {
102+
expect(err.message).toMatchSnapshot();
103+
const allMessages: string = logger.getOutput();
104+
expect(allMessages).not.toContain('Hold my beer...');
105+
expect(allMessages).toContain('Woops');
106+
checkConsoleOutput(logger);
107+
});
108+
});
109+
});
110+
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`TaskRunner Constructor throwsErrorOnInvalidParallelism 1`] = `"Invalid parallelism value of 'tequila', expected a number or 'max'"`;
4+
5+
exports[`TaskRunner Dependencies detectsDependencyCycle 1`] = `
6+
"A cyclic dependency was encountered:
7+
foo
8+
-> bar
9+
-> foo
10+
Consider using the cyclicDependencyProjects option for rush.json."
11+
`;
12+
13+
exports[`TaskRunner Dependencies respectsDependencyOrder 1`] = `"Registered two[n]Registered one[n]Executing a maximum of 1 simultaneous processes...[-n-][n][x][37m[one] started[x][39m[n][x][32m1 of 2: [one] completed successfully in 0.00 seconds[x][39m[n][x][37m[two] started[x][39m[n][x][32m2 of 2: [two] completed successfully in 0.00 seconds[x][39m[n][n][x][32mSUCCESS (2)[x][39m[n][x][32m================================[x][39m[n][x][32mtwo (0.00 seconds)[x][39m[n][x][32mone (0.00 seconds)[x][39m[n][x][32m================================[x][39m[-n-][x][32m[x][39m[n][n]"`;
14+
15+
exports[`TaskRunner Dependencies respectsDependencyOrder 2`] = `""`;
16+
17+
exports[`TaskRunner Dependencies respectsDependencyOrder 3`] = `""`;
18+
19+
exports[`TaskRunner Dependencies respectsDependencyOrder 4`] = `""`;
20+
21+
exports[`TaskRunner Dependencies throwsErrorOnNonExistentDependency 1`] = `"The project 'bar' has not been registered."`;
22+
23+
exports[`TaskRunner Dependencies throwsErrorOnNonExistentTask 1`] = `"The task 'foo' has not been registered"`;
24+
25+
exports[`TaskRunner Error logging printedStderrAfterError 1`] = `"Project(s) failed to build"`;
26+
27+
exports[`TaskRunner Error logging printedStderrAfterError 2`] = `"Registered stdout+stderr[n]Executing a maximum of 1 simultaneous processes...[-n-][n][x][37m[stdout+stderr] started[x][39m[n][n][x][31mFAILURE (1)[x][39m[n][x][31m================================[x][39m[n][x][31mstdout+stderr (0.00 seconds)[x][39m[n]Woops[n][x][31m================================[x][39m[-n-][x][31m[x][39m[n][n]"`;
28+
29+
exports[`TaskRunner Error logging printedStderrAfterError 3`] = `""`;
30+
31+
exports[`TaskRunner Error logging printedStderrAfterError 4`] = `""`;
32+
33+
exports[`TaskRunner Error logging printedStderrAfterError 5`] = `"[x][31m[-n-]1 of 1: [stdout+stderr] failed to build![x][39m[n]"`;
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/node-core-library",
5+
"comment": "Exposing utility class StringBufferTerminalProvider, useful to clients of Terminal API for their own unit tests",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/node-core-library",
10+
"email": "hogmoru@users.noreply.github.com"
11+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "Improve TaskRunner flexibility & testability by using Terminal API, add basic unit tests",
5+
"packageName": "@microsoft/rush",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "hogmoru@users.noreply.github.com"
11+
}

0 commit comments

Comments
 (0)