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.

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

Fix terminal logger encoding & error #4853

Merged
merged 13 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions global.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"sdk": {
"version": "9.0.100-alpha.1.23615.4",
"version": "9.0.100-alpha.1.24073.1",
"rollForward": "minor",
"allowPrerelease": false,
"architecture": "x64"
Expand Down Expand Up @@ -29,7 +29,7 @@
},
"xcopy-msbuild": "17.8.1-2",
"vswhere": "2.2.7",
"dotnet": "9.0.100-alpha.1.23615.4"
"dotnet": "9.0.100-alpha.1.24073.1"
},
"msbuild-sdks": {
"Microsoft.DotNet.Arcade.Sdk": "9.0.0-beta.24062.5"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
<NuspecBasePath>$(OutputPath)</NuspecBasePath>
<PackageId>Microsoft.TestPlatform.Build</PackageId>
<PackageTags>vstest visual-studio unittest testplatform mstest microsoft test testing</PackageTags>
<RepositoryUrl>https://github.com/microsoft/vstest</RepositoryUrl>
<PackageProjectUrl>https://github.com/microsoft/vstest</PackageProjectUrl>
<PackageDescription>
Build tasks and targets for running tests with Test Platform
</PackageDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Copyright (c) .NET Foundation. All rights reserved.
-->
<Target Name="VSTest" DependsOnTargets="ShowInfoMessageIfProjectHasNoIsTestProjectProperty">
<!-- Unloggable colorized output (cf. https://github.com/microsoft/vstest/issues/680) -->
<CallTarget Targets="_VSTestConsole" Condition="!$(VsTestUseMSBuildOutput)" />
<CallTarget Targets="_VSTestConsole" Condition="!$(VsTestUseMSBuildOutput) OR $(MSBUILDENSURESTDOUTFORTASKPROCESSES) == '1'" />
<!-- Proper MSBuild integration, but no custom colorization -->
<CallTarget Targets="_VSTestMSBuild" Condition="$(VsTestUseMSBuildOutput)" />
</Target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ override Microsoft.TestPlatform.Build.Tasks.VSTestTask.Execute() -> bool
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.GenerateCommandLineCommands() -> string?
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.GenerateFullPathToTool() -> string?
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.LogEventsFromTextOutput(string! singleLine, Microsoft.Build.Framework.MessageImportance messageImportance) -> void
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.StandardErrorEncoding.get -> System.Text.Encoding!
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.StandardOutputEncoding.get -> System.Text.Encoding!
override Microsoft.TestPlatform.Build.Tasks.VSTestTask2.ToolName.get -> string?
static Microsoft.TestPlatform.Build.Trace.Tracing.Trace(string! message) -> void
static Microsoft.TestPlatform.Build.Trace.Tracing.traceEnabled -> bool
17 changes: 16 additions & 1 deletion src/Microsoft.TestPlatform.Build/Tasks/VSTestTask2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Diagnostics;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;

using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
Expand Down Expand Up @@ -40,6 +42,9 @@ public class VSTestTask2 : ToolTask, ITestTask
public string? VSTestArtifactsProcessingMode { get; set; }
public string? VSTestSessionCorrelationId { get; set; }

protected override Encoding StandardErrorEncoding => _disableUtf8ConsoleEncoding ? base.StandardErrorEncoding : Encoding.UTF8;
protected override Encoding StandardOutputEncoding => _disableUtf8ConsoleEncoding ? base.StandardOutputEncoding : Encoding.UTF8;

private readonly string _testResultSplitter = "++++";
private readonly string[] _testResultSplitterArray = new[] { "++++" };

Expand All @@ -50,6 +55,7 @@ public class VSTestTask2 : ToolTask, ITestTask
private readonly string[] _fullErrorSplitterArray = new[] { "~~~~" };

private readonly string _fullErrorNewlineSplitter = "!!!!";
private readonly bool _disableUtf8ConsoleEncoding;

protected override string? ToolName
{
Expand All @@ -64,12 +70,15 @@ protected override string? ToolName

public VSTestTask2()
{
// Unless user opted out, use UTF encoding, which we force in vstest.console.
_disableUtf8ConsoleEncoding = Environment.GetEnvironmentVariable("VSTEST_DISABLE_UTF8_CONSOLE_ENCODING") == "1";
LogStandardErrorAsError = false;
StandardOutputImportance = "Normal";
}

protected override void LogEventsFromTextOutput(string singleLine, MessageImportance messageImportance)
{
Debug.WriteLine($"vstestTask2: received output {singleLine}, importance {messageImportance}");
if (singleLine.StartsWith(_errorSplitter))
{
var parts = singleLine.Split(_errorSplitterArray, StringSplitOptions.None);
Expand Down Expand Up @@ -132,8 +141,14 @@ protected override void LogEventsFromTextOutput(string singleLine, MessageImport
return;
}
}
else
{
Log.LogMessage(MessageImportance.Low, singleLine);
}

base.LogEventsFromTextOutput(singleLine, messageImportance);
// Do not call the base, it parses out the output, and if it sees "error" in any place it will log it as error
// we don't want this, we only want to log errors from the text messages we receive that start error splitter.
// base.LogEventsFromTextOutput(singleLine, messageImportance);
}

protected override string? GenerateCommandLineCommands()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
<NuspecBasePath>$(OutputPath)</NuspecBasePath>
<PackageId>Microsoft.TestPlatform.CLI</PackageId>
<PackageTags>vstest visual-studio unittest testplatform mstest microsoft test testing</PackageTags>
<PackageProjectUrl>https://github.com/microsoft/vstest</PackageProjectUrl>
Copy link
Member Author

Choose a reason for hiding this comment

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

dunno why this is not picking up in source build, all the published nugets have link to repo (when I look at nuget.org). Let's merge now, and figure it out later.

<RepositoryUrl>https://github.com/microsoft/vstest</RepositoryUrl>
<PackageDescription>
The cross platform Microsoft Test Platform.
</PackageDescription>
Expand Down
3 changes: 2 additions & 1 deletion src/vstest.console/Internal/MSBuildLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ private void TestResultHandler(object? sender, TestResultEventArgs e)
Output.Information(false, info);
break;
case TestOutcome.Failed:

var result = e.Result;
Debug.WriteLine(">>>>ERR:" + result.ErrorMessage);
Debug.WriteLine(">>>>STK:" + result.ErrorStackTrace);
if (!StringUtils.IsNullOrWhiteSpace(result.ErrorStackTrace))
{
var maxLength = 1000;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Generic;

using Microsoft.TestPlatform.TestUtilities;
using Microsoft.VisualStudio.TestTools.UnitTesting;

Expand All @@ -16,12 +18,12 @@ public class DotnetTestMSBuildOutputTests : AcceptanceTestBase
// patched dotnet is not published on non-windows systems
[TestCategory("Windows-Review")]
[NetCoreTargetFrameworkDataSource(useDesktopRunner: false)]
public void RunDotnetTestWithCsproj(RunnerInfo runnerInfo)
public void MSBuildLoggerCanBeEnabledByBuildPropertyAndDoesNotEatSpecialChars(RunnerInfo runnerInfo)
{
SetTestEnvironment(_testEnvironment, runnerInfo);

var projectPath = GetIsolatedTestAsset("SimpleTestProject.csproj");
InvokeDotnetTest($@"{projectPath} /p:VsTestUseMSBuildOutput=true /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
var projectPath = GetIsolatedTestAsset("TerminalLoggerTestProject.csproj");
InvokeDotnetTest($@"{projectPath} -nodereuse:false /p:VsTestUseMSBuildOutput=true /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");

// The output:
// Determining projects to restore...
Expand All @@ -31,10 +33,47 @@ public void RunDotnetTestWithCsproj(RunnerInfo runnerInfo)
// C:\Users\nohwnd\AppData\Local\Temp\vstest\xvoVt\UnitTest1.cs(41): error VSTEST1: (FailingTest) SampleUnitTestProject.UnitTest1.FailingTest() Assert.AreEqual failed. Expected:<2>. Actual:<3>. [C:\Users\nohwnd\AppData\Local\Temp\vstest\xvoVt\SimpleTestProject.csproj::TargetFramework=net462]
// C:\Users\nohwnd\AppData\Local\Temp\vstest\xvoVt\UnitTest1.cs(41): error VSTEST1: (FailingTest) SampleUnitTestProject.UnitTest1.FailingTest() Assert.AreEqual failed. Expected:<2>. Actual:<3>. [C:\Users\nohwnd\AppData\Local\Temp\vstest\xvoVt\SimpleTestProject.csproj::TargetFramework=netcoreapp3.1]

StdOutputContains("error VSTEST1: (FailingTest) SampleUnitTestProject.UnitTest1.FailingTest() Assert.AreEqual failed. Expected:<2>. Actual:<3>.");
StdOutputContains("TerminalLoggerUnitTests.UnitTest1.FailingTest() Assert.AreEqual failed. Expected:<ğğğ𦮙我們剛才從𓋴𓅓𓏏𓇏𓇌𓀀 (System.String)>. Actual:<3 (System.Int32)>.");
// We are sending those as low prio messages, they won't show up on screen but will be in binlog.
//StdOutputContains("passed PassingTest");
//StdOutputContains("skipped SkippingTest");
ExitCodeEquals(1);
}

[TestMethod]
// patched dotnet is not published on non-windows systems
[TestCategory("Windows-Review")]
[NetCoreTargetFrameworkDataSource(useDesktopRunner: false)]
public void MSBuildLoggerCanBeDisabledByBuildProperty(RunnerInfo runnerInfo)
{
SetTestEnvironment(_testEnvironment, runnerInfo);

var projectPath = GetIsolatedTestAsset("TerminalLoggerTestProject.csproj");
InvokeDotnetTest($@"{projectPath} -nodereuse:false /p:VsTestUseMSBuildOutput=false /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");

// Check that we see the summary that is printed from the console logger, meaning the new output is disabled.
StdOutputContains("Failed! - Failed: 1, Passed: 1, Skipped: 1, Total: 3, Duration:");
// We are sending those as low prio messages, they won't show up on screen but will be in binlog.
//StdOutputContains("passed PassingTest");
//StdOutputContains("skipped SkippingTest");
ExitCodeEquals(1);
}


[TestMethod]
// patched dotnet is not published on non-windows systems
[TestCategory("Windows-Review")]
[NetCoreTargetFrameworkDataSource(useDesktopRunner: false)]
public void MSBuildLoggerCanBeDisabledByEnvironmentVariableProperty(RunnerInfo runnerInfo)
{
SetTestEnvironment(_testEnvironment, runnerInfo);

var projectPath = GetIsolatedTestAsset("TerminalLoggerTestProject.csproj");
InvokeDotnetTest($@"{projectPath} -nodereuse:false /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}", environmentVariables: new Dictionary<string, string?> { ["MSBUILDENSURESTDOUTFORTASKPROCESSES"] = "1" });

// Check that we see the summary that is printed from the console logger, meaning the new output is disabled.
StdOutputContains("Failed! - Failed: 1, Passed: 1, Skipped: 1, Total: 3, Duration:");

ExitCodeEquals(1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void RunDotnetTestWithCsproj(RunnerInfo runnerInfo)
SetTestEnvironment(_testEnvironment, runnerInfo);

var projectPath = GetIsolatedTestAsset("SimpleTestProject.csproj");
InvokeDotnetTest($@"{projectPath} --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
InvokeDotnetTest($@"{projectPath} -p:VSTestUseMSBuildOutput=false --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");

// ensure our dev version is used
StdOutputContains(GetFinalVersion(IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion));
Expand Down Expand Up @@ -60,7 +60,7 @@ public void RunDotnetTestWithCsprojPassInlineSettings(RunnerInfo runnerInfo)
SetTestEnvironment(_testEnvironment, runnerInfo);

var projectPath = GetIsolatedTestAsset("ParametrizedTestProject.csproj");
InvokeDotnetTest($@"{projectPath} --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion} -- TestRunParameters.Parameter(name =\""weburl\"", value=\""http://localhost//def\"")");
InvokeDotnetTest($@"{projectPath} --logger:""Console;Verbosity=normal"" -p:VSTestUseMSBuildOutput=false /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion} -- TestRunParameters.Parameter(name =\""weburl\"", value=\""http://localhost//def\"")");

ValidateSummaryStatus(1, 0, 0);
ExitCodeEquals(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,15 +514,15 @@ public void RunSettingsAreLoadedFromProject(RunnerInfo runnerInfo)

var projectName = "ProjectFileRunSettingsTestProject.csproj";
var projectPath = GetIsolatedTestAsset(projectName);
InvokeDotnetTest($@"{projectPath} --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
InvokeDotnetTest($@"{projectPath} /p:VSTestUseMSBuildOutput=false --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
ValidateSummaryStatus(0, 1, 0);

// make sure that we can revert the project settings back by providing a config from command line
// keeping this in the same test, because it is easier to see that we are reverting settings that
// are honored by dotnet test, instead of just using the default, which would produce the same
// result
var settingsPath = GetProjectAssetFullPath(projectName, "inconclusive.runsettings");
InvokeDotnetTest($@"{projectPath} --settings {settingsPath} --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
InvokeDotnetTest($@"{projectPath} --settings {settingsPath} /p:VSTestUseMSBuildOutput=false --logger:""Console;Verbosity=normal"" /p:PackageVersion={IntegrationTestEnvironment.LatestLocallyBuiltNugetVersion}");
ValidateSummaryStatus(0, 0, 1);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<AssemblyName>SimpleTestProject</AssemblyName>
<TargetFrameworks>$(NetFrameworkMinimum);$(NetCoreAppMinimum)</TargetFrameworks>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="MSTest.TestFramework">
<Version>$(MSTestTestFrameworkVersion)</Version>
</PackageReference>
<PackageReference Include="MSTest.TestAdapter">
<Version>$(MSTestTestAdapterVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.NET.Test.Sdk">
<Version>$(PackageVersion)</Version>
</PackageReference>
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == '$(NetFrameworkMinimum)' ">
<Reference Include="System.Runtime" />
<Reference Include="System" />
<Reference Include="Microsoft.CSharp" />
</ItemGroup>
<ItemGroup>
<Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
</ItemGroup>
</Project>
46 changes: 46 additions & 0 deletions test/TestAssets/TerminalLoggerTestProject/UnitTest1.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#if NETFRAMEWORK
using System;
using System.IO;
#endif
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TerminalLoggerUnitTests
{
/// <summary>
/// The unit test 1.
/// </summary>
[TestClass]
public class UnitTest1
{
/// <summary>
/// The passing test.
/// </summary>
[TestMethod]
public void PassingTest()
{
Assert.AreEqual(2, 2);
}

/// <summary>
/// The failing test.
/// </summary>
[TestMethod]
public void FailingTest()
{
// test characters taken from https://pages.ucsd.edu/~dkjordan/chin/unitestuni.html
Assert.AreEqual("ğğğ𦮙我們剛才從𓋴𓅓𓏏𓇏𓇌𓀀", 3);
}

/// <summary>
/// The skipping test.
/// </summary>
[Ignore]
[TestMethod]
public void SkippingTest()
{
}
}
}
8 changes: 7 additions & 1 deletion test/TestAssets/TestAssets.sln
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,9 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MultiHostTestExecutionProje
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NonDll.TestAdapter", "NonDll.TestAdapter\NonDll.TestAdapter.csproj", "{429552A4-4C18-4355-94C5-80DC88C48405}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NetStandard2Library", "NetStandard2Library\NetStandard2Library.csproj", "{080F0AD2-E7AE-42C8-B867-56D78576735D}"
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NetStandard2Library", "NetStandard2Library\NetStandard2Library.csproj", "{080F0AD2-E7AE-42C8-B867-56D78576735D}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TerminalLoggerTestProject", "TerminalLoggerTestProject\TerminalLoggerTestProject.csproj", "{C69BEEA4-BE37-4155-8DD0-130C62D8BF6D}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Expand Down Expand Up @@ -384,6 +386,10 @@ Global
{080F0AD2-E7AE-42C8-B867-56D78576735D}.Debug|Any CPU.Build.0 = Debug|Any CPU
{080F0AD2-E7AE-42C8-B867-56D78576735D}.Release|Any CPU.ActiveCfg = Release|Any CPU
{080F0AD2-E7AE-42C8-B867-56D78576735D}.Release|Any CPU.Build.0 = Release|Any CPU
{C69BEEA4-BE37-4155-8DD0-130C62D8BF6D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{C69BEEA4-BE37-4155-8DD0-130C62D8BF6D}.Debug|Any CPU.Build.0 = Debug|Any CPU
{C69BEEA4-BE37-4155-8DD0-130C62D8BF6D}.Release|Any CPU.ActiveCfg = Release|Any CPU
{C69BEEA4-BE37-4155-8DD0-130C62D8BF6D}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down