Skip to content

Commit

Permalink
Fix bug: No visible message when replaying analyzed build (#10265)
Browse files Browse the repository at this point in the history
* add BuildCheck.UnitTests to slnf

* add test for replaying binlog of analyzed build

* fixed buf for BuildCheckResultWarning

* changed the test for error case

* fixed bug for BuildCheckResultError

* set RawMessage for BuilCheck results instead of speacial casing in BuildEventArgsWriter

* remove using

* add skip back to the flaky test;
add skip to the new test
  • Loading branch information
surayya-MS committed Jun 20, 2024
1 parent e1ff6dc commit 856f50e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 36 deletions.
3 changes: 2 additions & 1 deletion MSBuild.Dev.slnf
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"projects": [
"src\\Build.OM.UnitTests\\Microsoft.Build.Engine.OM.UnitTests.csproj",
"src\\Build.UnitTests\\Microsoft.Build.Engine.UnitTests.csproj",
"src\\BuildCheck.UnitTests\\Microsoft.Build.BuildCheck.UnitTests.csproj",
"src\\Build\\Microsoft.Build.csproj",
"src\\Framework.UnitTests\\Microsoft.Build.Framework.UnitTests.csproj",
"src\\Framework\\Microsoft.Build.Framework.csproj",
Expand All @@ -18,4 +19,4 @@
"src\\Xunit.NetCore.Extensions\\Xunit.NetCore.Extensions.csproj"
]
}
}
}
93 changes: 73 additions & 20 deletions src/BuildCheck.UnitTests/EndToEndTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,76 @@ public EndToEndTests(ITestOutputHelper output)
[InlineData(true, true)]
[InlineData(false, true)]
[InlineData(false, false)]
public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool analysisRequested)
public void SampleAnalyzerIntegrationTest_AnalyzeOnBuild(bool buildInOutOfProcessNode, bool analysisRequested)
{
PrepareSampleProjectsAndConfig(buildInOutOfProcessNode, out TransientTestFile projectFile);

string output = RunnerUtilities.ExecBootstrapedMSBuild(
$"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" +
(analysisRequested ? " -analyze" : string.Empty), out bool success, false, _env.Output, timeoutMilliseconds: 120_000);
_env.Output.WriteLine(output);

success.ShouldBeTrue();

// The analyzer warnings should appear - but only if analysis was requested.
if (analysisRequested)
{
output.ShouldContain("BC0101");
output.ShouldContain("BC0102");
}
else
{
output.ShouldNotContain("BC0101");
output.ShouldNotContain("BC0102");
}
}

[Theory(Skip = "https://github.com/dotnet/msbuild/issues/10036")]
[InlineData(true, true, "warning")]
[InlineData(true, true, "error")]
[InlineData(true, true, "info")]
[InlineData(false, true, "warning")]
[InlineData(false, true, "error")]
[InlineData(false, true, "info")]
[InlineData(false, false, "warning")]
public void SampleAnalyzerIntegrationTest_ReplayBinaryLogOfAnalyzedBuild(bool buildInOutOfProcessNode, bool analysisRequested, string BC0101Severity)
{
PrepareSampleProjectsAndConfig(buildInOutOfProcessNode, out TransientTestFile projectFile, BC0101Severity);

var projectDirectory = Path.GetDirectoryName(projectFile.Path);
string logFile = _env.ExpectFile(".binlog").Path;

RunnerUtilities.ExecBootstrapedMSBuild(
$"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore {(analysisRequested ? "-analyze" : string.Empty)} -bl:{logFile}",
out bool success, false, _env.Output, timeoutMilliseconds: 120_000);

success.ShouldBeTrue();

string output = RunnerUtilities.ExecBootstrapedMSBuild(
$"{logFile} -flp:logfile={Path.Combine(projectDirectory!, "logFile.log")};verbosity=diagnostic",
out success, false, _env.Output, timeoutMilliseconds: 120_000);

_env.Output.WriteLine(output);

success.ShouldBeTrue();

// The conflicting outputs warning appears - but only if analysis was requested
if (analysisRequested)
{
output.ShouldContain("BC0101");
output.ShouldContain("BC0102");
}
else
{
output.ShouldNotContain("BC0101");
output.ShouldNotContain("BC0102");
}
}

private void PrepareSampleProjectsAndConfig(
bool buildInOutOfProcessNode,
out TransientTestFile projectFile,
string BC0101Severity = "warning")
{
TransientTestFolder workFolder = _env.CreateFolder(createFolder: true);
TransientTestFile testFile = _env.CreateFile(workFolder, "somefile");
Expand Down Expand Up @@ -87,16 +156,16 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana

</Project>
""";
TransientTestFile projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents);
projectFile = _env.CreateFile(workFolder, "FooBar.csproj", contents);
TransientTestFile projectFile2 = _env.CreateFile(workFolder, "FooBar-Copy.csproj", contents2);

TransientTestFile config = _env.CreateFile(workFolder, ".editorconfig",
"""
$"""
root=true

[*.csproj]
build_check.BC0101.IsEnabled=true
build_check.BC0101.Severity=warning
build_check.BC0101.Severity={BC0101Severity}

build_check.BC0102.IsEnabled=true
build_check.BC0102.Severity=warning
Expand All @@ -116,22 +185,6 @@ public void SampleAnalyzerIntegrationTest(bool buildInOutOfProcessNode, bool ana

_env.SetEnvironmentVariable("MSBUILDNOINPROCNODE", buildInOutOfProcessNode ? "1" : "0");
_env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1");
string output = RunnerUtilities.ExecBootstrapedMSBuild(
$"{Path.GetFileName(projectFile.Path)} /m:1 -nr:False -restore" +
(analysisRequested ? " -analyze" : string.Empty), out bool success, false, _env.Output, timeoutMilliseconds: 120_000);
_env.Output.WriteLine(output);
success.ShouldBeTrue();
// The analyzer warnings should appear - but only if analysis was requested.
if (analysisRequested)
{
output.ShouldContain("BC0101");
output.ShouldContain("BC0102");
}
else
{
output.ShouldNotContain("BC0101");
output.ShouldNotContain("BC0102");
}
}

[Theory]
Expand Down
24 changes: 9 additions & 15 deletions src/Framework/BuildCheck/BuildCheckEventArgs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ internal sealed class BuildCheckResultWarning : BuildWarningEventArgs
{
public BuildCheckResultWarning(IBuildCheckResult result)
{
this.Message = result.FormatMessage();
RawMessage = result.FormatMessage();
}

internal BuildCheckResultWarning() { }
Expand All @@ -118,24 +118,22 @@ internal override void WriteToStream(BinaryWriter writer)
{
base.WriteToStream(writer);

writer.Write(Message!);
writer.Write(RawMessage!);
}

internal override void CreateFromStream(BinaryReader reader, int version)
{
base.CreateFromStream(reader, version);

Message = reader.ReadString();
RawMessage = reader.ReadString();
}

public override string? Message { get; protected set; }
}

internal sealed class BuildCheckResultError : BuildErrorEventArgs
{
public BuildCheckResultError(IBuildCheckResult result)
{
this.Message = result.FormatMessage();
RawMessage = result.FormatMessage();
}

internal BuildCheckResultError() { }
Expand All @@ -144,24 +142,22 @@ internal override void WriteToStream(BinaryWriter writer)
{
base.WriteToStream(writer);

writer.Write(Message!);
writer.Write(RawMessage!);
}

internal override void CreateFromStream(BinaryReader reader, int version)
{
base.CreateFromStream(reader, version);

Message = reader.ReadString();
RawMessage = reader.ReadString();
}

public override string? Message { get; protected set; }
}

internal sealed class BuildCheckResultMessage : BuildMessageEventArgs
{
public BuildCheckResultMessage(IBuildCheckResult result)
{
this.Message = result.FormatMessage();
RawMessage = result.FormatMessage();
}

internal BuildCheckResultMessage() { }
Expand All @@ -170,15 +166,13 @@ internal override void WriteToStream(BinaryWriter writer)
{
base.WriteToStream(writer);

writer.Write(Message!);
writer.Write(RawMessage!);
}

internal override void CreateFromStream(BinaryReader reader, int version)
{
base.CreateFromStream(reader, version);

Message = reader.ReadString();
RawMessage = reader.ReadString();
}

public override string? Message { get; protected set; }
}

0 comments on commit 856f50e

Please sign in to comment.