Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[rush] Add support for new --print-log-file-paths parameter #4999

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[rush] Add support for new --print-log-file-paths parameter #4999

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
713e069
feature: add new --print-log-file-paths parameter
witcher112 Nov 9, 2024
cb6ea32
chore: rush change
witcher112 Nov 9, 2024
14cf2af
chore: rush test --update-snapshots
witcher112 Nov 9, 2024
e8fd1eb
feature: print only main log file path as it contains both stdout and…
witcher112 Nov 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions ...on/changes/@microsoft/rush/summarizer-improvements-vscode1-proposal_2024-11-09-00-30.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Add new --print-log-file-paths parameter for rush commands that prints log file paths after failed action summaries",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
24 changes: 24 additions & 0 deletions libraries/rush-lib/src/api/test/__snapshots__/RushCommandLine.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,14 @@ Object {
"required": false,
"shortName": undefined,
},
Object {
"description": "Display log file paths for projects that failed to build",
"environmentVariable": undefined,
"kind": "Flag",
"longName": "--print-log-file-paths",
"required": false,
"shortName": undefined,
},
Object {
"description": "Display the logs during the build, rather than just displaying the build status summary",
"environmentVariable": undefined,
Expand Down Expand Up @@ -1358,6 +1366,14 @@ Object {
"required": false,
"shortName": undefined,
},
Object {
"description": "Display log file paths for projects that failed to build",
"environmentVariable": undefined,
"kind": "Flag",
"longName": "--print-log-file-paths",
"required": false,
"shortName": undefined,
},
Object {
"description": "Display the logs during the build, rather than just displaying the build status summary",
"environmentVariable": undefined,
Expand Down Expand Up @@ -1491,6 +1507,14 @@ Object {
"required": false,
"shortName": undefined,
},
Object {
"description": "Display log file paths for projects that failed to build",
"environmentVariable": undefined,
"kind": "Flag",
"longName": "--print-log-file-paths",
"required": false,
"shortName": undefined,
},
Object {
"description": "Display the logs during the build, rather than just displaying the build status summary",
"environmentVariable": undefined,
Expand Down
8 changes: 7 additions & 1 deletion libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {

private readonly _changedProjectsOnly: CommandLineFlagParameter | undefined;
private readonly _selectionParameters: SelectionParameterSet;
private readonly _printLogFilePathsParameter: CommandLineFlagParameter;
private readonly _verboseParameter: CommandLineFlagParameter;
private readonly _parallelismParameter: CommandLineStringParameter | undefined;
private readonly _ignoreHooksParameter: CommandLineFlagParameter;
Expand Down Expand Up @@ -220,6 +221,11 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
includeSubspaceSelector: false
});

this._printLogFilePathsParameter = this.defineFlagParameter({
parameterLongName: '--print-log-file-paths',
description: 'Display log file paths for projects that failed to build'
});

this._verboseParameter = this.defineFlagParameter({
parameterLongName: '--verbose',
parameterShortName: '-v',
Expand Down Expand Up @@ -367,7 +373,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
}

// Enable the standard summary
new OperationResultSummarizerPlugin(terminal).apply(this.hooks);
new OperationResultSummarizerPlugin(terminal, this._printLogFilePathsParameter.value).apply(this.hooks);

const { hooks: sessionHooks } = this.rushSession;
if (sessionHooks.runAnyPhasedCommand.isUsed()) {
Expand Down
22 changes: 16 additions & 6 deletions libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ exports[`CommandLineHelp prints the help for each action: build 1`] = `
[-t PROJECT] [-T PROJECT] [-f PROJECT] [-o PROJECT]
[-i PROJECT] [-I PROJECT]
[--to-version-policy VERSION_POLICY_NAME]
[--from-version-policy VERSION_POLICY_NAME] [-v] [-c]
[--ignore-hooks] [-s] [-m]
[--from-version-policy VERSION_POLICY_NAME]
[--print-log-file-paths] [-v] [-c] [--ignore-hooks] [-s]
[-m]


This command is similar to \\"rush rebuild\\", except that \\"rush build\\" performs
Expand Down Expand Up @@ -267,6 +268,9 @@ Optional arguments:
each of the projects belonging to VERSION_POLICY_NAME.
For details, refer to the website article \\"Selecting
subsets of projects\\".
--print-log-file-paths
Display log file paths for projects that failed to
build
-v, --verbose Display the logs during the build, rather than just
displaying the build status summary
-c, --changed-projects-only
Expand Down Expand Up @@ -431,8 +435,8 @@ exports[`CommandLineHelp prints the help for each action: import-strings 1`] = `
[-t PROJECT] [-T PROJECT] [-f PROJECT] [-o PROJECT]
[-i PROJECT] [-I PROJECT]
[--to-version-policy VERSION_POLICY_NAME]
[--from-version-policy VERSION_POLICY_NAME] [-v]
[--ignore-hooks]
[--from-version-policy VERSION_POLICY_NAME]
[--print-log-file-paths] [-v] [--ignore-hooks]
[--locale {en-us,fr-fr,es-es,zh-cn}]


Expand Down Expand Up @@ -536,6 +540,9 @@ Optional arguments:
each of the projects belonging to VERSION_POLICY_NAME.
For details, refer to the website article \\"Selecting
subsets of projects\\".
--print-log-file-paths
Display log file paths for projects that failed to
build
-v, --verbose Display the logs during the build, rather than just
displaying the build status summary
--ignore-hooks Skips execution of the \\"eventHooks\\" scripts defined
Expand Down Expand Up @@ -1029,8 +1036,8 @@ exports[`CommandLineHelp prints the help for each action: rebuild 1`] = `
[-t PROJECT] [-T PROJECT] [-f PROJECT] [-o PROJECT]
[-i PROJECT] [-I PROJECT]
[--to-version-policy VERSION_POLICY_NAME]
[--from-version-policy VERSION_POLICY_NAME] [-v]
[--ignore-hooks] [-s] [-m]
[--from-version-policy VERSION_POLICY_NAME]
[--print-log-file-paths] [-v] [--ignore-hooks] [-s] [-m]


This command assumes that the package.json file for each project contains a
Expand Down Expand Up @@ -1139,6 +1146,9 @@ Optional arguments:
each of the projects belonging to VERSION_POLICY_NAME.
For details, refer to the website article \\"Selecting
subsets of projects\\".
--print-log-file-paths
Display log file paths for projects that failed to
build
-v, --verbose Display the logs during the build, rather than just
displaying the build status summary
--ignore-hooks Skips execution of the \\"eventHooks\\" scripts defined
Expand Down
20 changes: 16 additions & 4 deletions libraries/rush-lib/src/logic/operations/OperationResultSummarizerPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,18 @@ type IOperationsByStatus = Map<OperationStatus, IOperationAndResult[]>;
*/
export class OperationResultSummarizerPlugin implements IPhasedCommandPlugin {
private readonly _terminal: ITerminal;
private readonly _shouldPrintLogFilePaths: boolean;

public constructor(terminal: ITerminal) {
public constructor(terminal: ITerminal, shouldPrintLogFilePaths: boolean) {
this._terminal = terminal;
this._shouldPrintLogFilePaths = shouldPrintLogFilePaths;
}

public apply(hooks: PhasedCommandHooks): void {
hooks.afterExecuteOperations.tap(
PLUGIN_NAME,
(result: IExecutionResult, context: ICreateOperationsContext): void => {
_printOperationStatus(this._terminal, result);
_printOperationStatus(this._terminal, this._shouldPrintLogFilePaths, result);
}
);
}
Expand All @@ -47,7 +49,7 @@ export class OperationResultSummarizerPlugin implements IPhasedCommandPlugin {
* Prints out a report of the status of each project
* @internal
*/
export function _printOperationStatus(terminal: ITerminal, result: IExecutionResult): void {
export function _printOperationStatus(terminal: ITerminal, shouldPrintLogFilePaths: boolean, result: IExecutionResult): void {
const { operationResults } = result;

const operationsByStatus: IOperationsByStatus = new Map();
Expand Down Expand Up @@ -119,6 +121,7 @@ export function _printOperationStatus(terminal: ITerminal, result: IExecutionRes

writeDetailedSummary(
terminal,
shouldPrintLogFilePaths,
OperationStatus.SuccessWithWarning,
operationsByStatus,
Colorize.yellow,
Expand All @@ -133,7 +136,7 @@ export function _printOperationStatus(terminal: ITerminal, result: IExecutionRes
'These operations were blocked by dependencies that failed:'
);

writeDetailedSummary(terminal, OperationStatus.Failure, operationsByStatus, Colorize.red);
writeDetailedSummary(terminal, shouldPrintLogFilePaths, OperationStatus.Failure, operationsByStatus, Colorize.red);

terminal.writeLine('');

Expand Down Expand Up @@ -196,6 +199,7 @@ function writeCondensedSummary(

function writeDetailedSummary(
terminal: ITerminal,
shouldPrintLogFilePaths: boolean,
status: OperationStatus,
operationsByStatus: IOperationsByStatus,
headingColor: (text: string) => string,
Expand Down Expand Up @@ -253,6 +257,14 @@ function writeDetailedSummary(
terminal.write(details);
}

if (shouldPrintLogFilePaths) {
if (operationResult.status === OperationStatus.Failure && operationResult.logFilePaths !== undefined) {
terminal.writeLine('');
terminal.writeLine(Colorize.gray(`stdout: ${operationResult.logFilePaths.text}`));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this log contains both stdout and stderr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't know that.

In such case, I guess single line log file: xyz should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Done

terminal.writeLine(Colorize.gray(`stderr: ${operationResult.logFilePaths.error}`));
}
}

terminal.writeLine('');
}
}
Expand Down
10 changes: 5 additions & 5 deletions libraries/rush-lib/src/logic/operations/test/OperationExecutionManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe(OperationExecutionManager.name, () => {
);

const result: IExecutionResult = await executionManager.executeAsync();
_printOperationStatus(mockTerminal, result);
_printOperationStatus(mockTerminal, false, result);
expect(result.status).toEqual(OperationStatus.Failure);
expect(result.operationResults.size).toEqual(1);
const firstResult: IOperationExecutionResult = result.operationResults.values().next().value;
Expand All @@ -110,7 +110,7 @@ describe(OperationExecutionManager.name, () => {
);

const result: IExecutionResult = await executionManager.executeAsync();
_printOperationStatus(mockTerminal, result);
_printOperationStatus(mockTerminal, false, result);
expect(result.status).toEqual(OperationStatus.Failure);
expect(result.operationResults.size).toEqual(1);
const firstResult: IOperationExecutionResult = result.operationResults.values().next().value;
Expand Down Expand Up @@ -184,7 +184,7 @@ describe(OperationExecutionManager.name, () => {
);

const result: IExecutionResult = await executionManager.executeAsync();
_printOperationStatus(mockTerminal, result);
_printOperationStatus(mockTerminal, false, result);
expect(result.status).toEqual(OperationStatus.SuccessWithWarning);
expect(result.operationResults.size).toEqual(1);
const firstResult: IOperationExecutionResult = result.operationResults.values().next().value;
Expand Down Expand Up @@ -223,7 +223,7 @@ describe(OperationExecutionManager.name, () => {
);

const result: IExecutionResult = await executionManager.executeAsync();
_printOperationStatus(mockTerminal, result);
_printOperationStatus(mockTerminal, false, result);
expect(result.status).toEqual(OperationStatus.Success);
expect(result.operationResults.size).toEqual(1);
const firstResult: IOperationExecutionResult = result.operationResults.values().next().value;
Expand All @@ -250,7 +250,7 @@ describe(OperationExecutionManager.name, () => {

const result: IExecutionResult = await executionManager.executeAsync();
_printTimeline({ terminal: mockTerminal, result, cobuildConfiguration: undefined });
_printOperationStatus(mockTerminal, result);
_printOperationStatus(mockTerminal, false, result);
const allMessages: string = mockWritable.getAllOutput();
expect(allMessages).toContain('Build step 1');
expect(allMessages).toContain('Warning: step 1 succeeded with warnings');
Expand Down
Loading