From 92bcbe7d5b22927fb89d70bbb8aa1a48beb5eea9 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Wed, 15 Nov 2023 16:55:06 +0100 Subject: [PATCH 1/5] add dispaying error messages not assosiated with tracked projects --- src/MSBuild/TerminalLogger/TerminalLogger.cs | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/MSBuild/TerminalLogger/TerminalLogger.cs b/src/MSBuild/TerminalLogger/TerminalLogger.cs index 9743b489917..398583aa7df 100644 --- a/src/MSBuild/TerminalLogger/TerminalLogger.cs +++ b/src/MSBuild/TerminalLogger/TerminalLogger.cs @@ -635,16 +635,18 @@ private bool IsImmediateMessage(string message) /// private void ErrorRaised(object sender, BuildErrorEventArgs e) { - var buildEventContext = e.BuildEventContext; - if (buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out Project? project)) - { - string message = EventArgsFormatting.FormatEventMessage( + BuildEventContext? buildEventContext = e.BuildEventContext; + Project? project = null; + bool isTrackedProject = buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out project); + string message = EventArgsFormatting.FormatEventMessage( category: AnsiCodes.Colorize("error", TerminalColor.Red), subcategory: e.Subcategory, message: e.Message, code: AnsiCodes.Colorize(e.Code, TerminalColor.Red), file: HighlightFileName(e.File), - projectFile: null, + + // for the tracked projects the project file name is included in the final output result. + projectFile: isTrackedProject ? null : e.ProjectFile ?? string.Empty, lineNumber: e.LineNumber, endLineNumber: e.EndLineNumber, columnNumber: e.ColumnNumber, @@ -652,11 +654,19 @@ private void ErrorRaised(object sender, BuildErrorEventArgs e) threadId: e.ThreadId, logOutputProperties: null); - project.AddBuildMessage(MessageSeverity.Error, message); + if (isTrackedProject) + { + project!.AddBuildMessage(MessageSeverity.Error, message); + } + + // It is necessary to display error messages reported by MSBuild, even if it's not tracked in _projects collection. + else + { + RenderImmediateMessage(message); } } -#endregion + #endregion #region Refresher thread implementation From 75eb26971ac8b93fec1a8b0364ad6cc747fb65e2 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 27 Nov 2023 10:30:13 +0100 Subject: [PATCH 2/5] add tests coverage + fix review comments --- ...crosoft.Build.CommandLine.UnitTests.csproj | 18 +++++++++ ...sts.PrintRestore_Failed.Linux.verified.txt | 4 ++ ...Tests.PrintRestore_Failed.OSX.verified.txt | 4 ++ ...s.PrintRestore_Failed.Windows.verified.txt | 4 ++ ...ore_SuccessWithWarnings.Linux.verified.txt | 4 ++ ...store_SuccessWithWarnings.OSX.verified.txt | 4 ++ ...e_SuccessWithWarnings.Windows.verified.txt | 4 ++ src/MSBuild.UnitTests/TerminalLogger_Tests.cs | 24 ++++++++++++ src/MSBuild/TerminalLogger/TerminalLogger.cs | 37 +++++++++++-------- 9 files changed, 87 insertions(+), 16 deletions(-) create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Windows.verified.txt create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt create mode 100644 src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Windows.verified.txt diff --git a/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj b/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj index abb5f36de05..2e1407ed4d2 100644 --- a/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj +++ b/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj @@ -75,4 +75,22 @@ + + + $([System.String]::Copy('%(FileName)').Split('.')[0]) + %(ParentFile).cs + + + $([System.String]::Copy('%(FileName)').Split('.')[0]) + %(ParentFile).cs + + + $([System.String]::Copy('%(FileName)').Split('.')[0]) + %(ParentFile).cs + + + $([System.String]::Copy('%(FileName)').Split('.')[0]) + + + diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt new file mode 100644 index 00000000000..5712a82c49e --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): error AA0000: Restore Failed + +Build failed with errors in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt new file mode 100644 index 00000000000..5712a82c49e --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): error AA0000: Restore Failed + +Build failed with errors in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Windows.verified.txt new file mode 100644 index 00000000000..1df7926cf3f --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Windows.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): error AA0000: Restore Failed + +Build failed with errors in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt new file mode 100644 index 00000000000..a7ebb4b009f --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): warning AA0000: Restore with Warning + +Build succeeded with warnings in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt new file mode 100644 index 00000000000..a7ebb4b009f --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): warning AA0000: Restore with Warning + +Build succeeded with warnings in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Windows.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Windows.verified.txt new file mode 100644 index 00000000000..bc7006945a4 --- /dev/null +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Windows.verified.txt @@ -0,0 +1,4 @@ +directory/file(1,2,3,4): warning AA0000: Restore with Warning + +Build succeeded with warnings in 0.0s +]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/TerminalLogger_Tests.cs b/src/MSBuild.UnitTests/TerminalLogger_Tests.cs index 451698e01ab..9d764455e9a 100644 --- a/src/MSBuild.UnitTests/TerminalLogger_Tests.cs +++ b/src/MSBuild.UnitTests/TerminalLogger_Tests.cs @@ -278,6 +278,30 @@ public Task PrintImmediateMessage_Skipped() return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform(); } + [Fact] + public Task PrintRestore_Failed() + { + bool succeeded = false; + ErrorRaised?.Invoke(_eventSender, MakeErrorEventArgs("Restore Failed")); + + ProjectFinished?.Invoke(_eventSender, MakeProjectFinishedEventArgs(_projectFile, succeeded)); + BuildFinished?.Invoke(_eventSender, MakeBuildFinishedEventArgs(succeeded)); + + return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform(); + } + + [Fact] + public Task PrintRestore_SuccessWithWarnings() + { + bool succeeded = true; + WarningRaised?.Invoke(_eventSender, MakeWarningEventArgs("Restore with Warning")); + + ProjectFinished?.Invoke(_eventSender, MakeProjectFinishedEventArgs(_projectFile, succeeded)); + BuildFinished?.Invoke(_eventSender, MakeBuildFinishedEventArgs(succeeded)); + + return Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform(); + } + [Fact] public Task PrintBuildSummary_Failed() { diff --git a/src/MSBuild/TerminalLogger/TerminalLogger.cs b/src/MSBuild/TerminalLogger/TerminalLogger.cs index 398583aa7df..f9a8129b127 100644 --- a/src/MSBuild/TerminalLogger/TerminalLogger.cs +++ b/src/MSBuild/TerminalLogger/TerminalLogger.cs @@ -591,16 +591,16 @@ private void MessageRaised(object sender, BuildMessageEventArgs e) /// private void WarningRaised(object sender, BuildWarningEventArgs e) { - var buildEventContext = e.BuildEventContext; - if (buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out Project? project)) - { - string message = EventArgsFormatting.FormatEventMessage( + BuildEventContext? buildEventContext = e.BuildEventContext; + Project? project = null; + bool isTrackedProject = buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out project); + string message = EventArgsFormatting.FormatEventMessage( category: AnsiCodes.Colorize("warning", TerminalColor.Yellow), subcategory: e.Subcategory, message: e.Message, code: AnsiCodes.Colorize(e.Code, TerminalColor.Yellow), file: HighlightFileName(e.File), - projectFile: null, + projectFile: e.ProjectFile ?? null, lineNumber: e.LineNumber, endLineNumber: e.EndLineNumber, columnNumber: e.ColumnNumber, @@ -608,11 +608,20 @@ private void WarningRaised(object sender, BuildWarningEventArgs e) threadId: e.ThreadId, logOutputProperties: null); + if (isTrackedProject) + { if (IsImmediateMessage(message)) { RenderImmediateMessage(message); } - project.AddBuildMessage(MessageSeverity.Warning, message); + + project!.AddBuildMessage(MessageSeverity.Warning, message); + } + else + { + // It is necessary to display warning messages reported by MSBuild, even if it's not tracked in _projects collection. + RenderImmediateMessage(message); + _buildHasWarnings = true; } } @@ -621,14 +630,12 @@ private void WarningRaised(object sender, BuildWarningEventArgs e) /// /// Raised event. /// true if marker is detected. - private bool IsImmediateMessage(string message) - { + private bool IsImmediateMessage(string message) => #if NET7_0_OR_GREATER - return ImmediateMessageRegex().IsMatch(message); + ImmediateMessageRegex().IsMatch(message); #else - return _immediateMessageKeywords.Any(imk => message.IndexOf(imk, StringComparison.OrdinalIgnoreCase) >= 0); + _immediateMessageKeywords.Any(imk => message.IndexOf(imk, StringComparison.OrdinalIgnoreCase) >= 0); #endif - } /// /// The callback. @@ -644,9 +651,7 @@ private void ErrorRaised(object sender, BuildErrorEventArgs e) message: e.Message, code: AnsiCodes.Colorize(e.Code, TerminalColor.Red), file: HighlightFileName(e.File), - - // for the tracked projects the project file name is included in the final output result. - projectFile: isTrackedProject ? null : e.ProjectFile ?? string.Empty, + projectFile: e.ProjectFile ?? null, lineNumber: e.LineNumber, endLineNumber: e.EndLineNumber, columnNumber: e.ColumnNumber, @@ -658,11 +663,11 @@ private void ErrorRaised(object sender, BuildErrorEventArgs e) { project!.AddBuildMessage(MessageSeverity.Error, message); } - - // It is necessary to display error messages reported by MSBuild, even if it's not tracked in _projects collection. else { + // It is necessary to display error messages reported by MSBuild, even if it's not tracked in _projects collection. RenderImmediateMessage(message); + _buildHasErrors = true; } } From 5fc437e81365ac1fc002647d489515942e8033e9 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 27 Nov 2023 11:07:23 +0100 Subject: [PATCH 3/5] fix test data --- .../TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt | 2 +- .../TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt | 2 +- ...er_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt | 2 +- ...gger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt index 5712a82c49e..1df7926cf3f 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.Linux.verified.txt @@ -1,4 +1,4 @@ -directory/file(1,2,3,4): error AA0000: Restore Failed +directory/file(1,2,3,4): error AA0000: Restore Failed Build failed with errors in 0.0s ]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt index 5712a82c49e..1df7926cf3f 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt @@ -1,4 +1,4 @@ -directory/file(1,2,3,4): error AA0000: Restore Failed +directory/file(1,2,3,4): error AA0000: Restore Failed Build failed with errors in 0.0s ]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt index a7ebb4b009f..bc7006945a4 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.Linux.verified.txt @@ -1,4 +1,4 @@ -directory/file(1,2,3,4): warning AA0000: Restore with Warning +directory/file(1,2,3,4): warning AA0000: Restore with Warning Build succeeded with warnings in 0.0s ]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt index a7ebb4b009f..bc7006945a4 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt @@ -1,4 +1,4 @@ -directory/file(1,2,3,4): warning AA0000: Restore with Warning +directory/file(1,2,3,4): warning AA0000: Restore with Warning Build succeeded with warnings in 0.0s ]9;4;0;\ \ No newline at end of file From 4847e072a39384003a2e66826c78c21611f8e8ab Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 27 Nov 2023 11:47:21 +0100 Subject: [PATCH 4/5] fix test output for macos --- .../TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt | 1 - ...ogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt | 1 - 2 files changed, 2 deletions(-) diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt index 1df7926cf3f..c4d47333679 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_Failed.OSX.verified.txt @@ -1,4 +1,3 @@ directory/file(1,2,3,4): error AA0000: Restore Failed Build failed with errors in 0.0s -]9;4;0;\ \ No newline at end of file diff --git a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt index bc7006945a4..8756e2b3be2 100644 --- a/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt +++ b/src/MSBuild.UnitTests/Snapshots/TerminalLogger_Tests.PrintRestore_SuccessWithWarnings.OSX.verified.txt @@ -1,4 +1,3 @@ directory/file(1,2,3,4): warning AA0000: Restore with Warning Build succeeded with warnings in 0.0s -]9;4;0;\ \ No newline at end of file From fb408e510cba99a4bc83ad609998de20201c440b Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Tue, 28 Nov 2023 10:11:58 +0100 Subject: [PATCH 5/5] cleanup --- ...icrosoft.Build.CommandLine.UnitTests.csproj | 18 ------------------ src/MSBuild/TerminalLogger/TerminalLogger.cs | 12 ++++-------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj b/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj index 2e1407ed4d2..abb5f36de05 100644 --- a/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj +++ b/src/MSBuild.UnitTests/Microsoft.Build.CommandLine.UnitTests.csproj @@ -75,22 +75,4 @@ - - - $([System.String]::Copy('%(FileName)').Split('.')[0]) - %(ParentFile).cs - - - $([System.String]::Copy('%(FileName)').Split('.')[0]) - %(ParentFile).cs - - - $([System.String]::Copy('%(FileName)').Split('.')[0]) - %(ParentFile).cs - - - $([System.String]::Copy('%(FileName)').Split('.')[0]) - - - diff --git a/src/MSBuild/TerminalLogger/TerminalLogger.cs b/src/MSBuild/TerminalLogger/TerminalLogger.cs index f9a8129b127..02f09e30118 100644 --- a/src/MSBuild/TerminalLogger/TerminalLogger.cs +++ b/src/MSBuild/TerminalLogger/TerminalLogger.cs @@ -592,8 +592,6 @@ private void MessageRaised(object sender, BuildMessageEventArgs e) private void WarningRaised(object sender, BuildWarningEventArgs e) { BuildEventContext? buildEventContext = e.BuildEventContext; - Project? project = null; - bool isTrackedProject = buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out project); string message = EventArgsFormatting.FormatEventMessage( category: AnsiCodes.Colorize("warning", TerminalColor.Yellow), subcategory: e.Subcategory, @@ -608,14 +606,14 @@ private void WarningRaised(object sender, BuildWarningEventArgs e) threadId: e.ThreadId, logOutputProperties: null); - if (isTrackedProject) + if (buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out Project? project)) { if (IsImmediateMessage(message)) { RenderImmediateMessage(message); } - project!.AddBuildMessage(MessageSeverity.Warning, message); + project.AddBuildMessage(MessageSeverity.Warning, message); } else { @@ -643,8 +641,6 @@ private bool IsImmediateMessage(string message) => private void ErrorRaised(object sender, BuildErrorEventArgs e) { BuildEventContext? buildEventContext = e.BuildEventContext; - Project? project = null; - bool isTrackedProject = buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out project); string message = EventArgsFormatting.FormatEventMessage( category: AnsiCodes.Colorize("error", TerminalColor.Red), subcategory: e.Subcategory, @@ -659,9 +655,9 @@ private void ErrorRaised(object sender, BuildErrorEventArgs e) threadId: e.ThreadId, logOutputProperties: null); - if (isTrackedProject) + if (buildEventContext is not null && _projects.TryGetValue(new ProjectContext(buildEventContext), out Project? project)) { - project!.AddBuildMessage(MessageSeverity.Error, message); + project.AddBuildMessage(MessageSeverity.Error, message); } else {