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

vstest to honor nologo input from dotnet.exe #1717

Merged
merged 1 commit into from
Aug 3, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Copyright (c) .NET Foundation. All rights reserved.
VSTestCollect="$(VSTestCollect)"
VSTestBlame="$(VSTestBlame)"
VSTestTraceDataCollectorDirectoryPath="$(TraceDataCollectorDirectoryPath)"
VSTestNoLogo="$(VSTestNoLogo)"
/>
</Target>

Expand Down Expand Up @@ -87,6 +88,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<Message Text="VSTestCollect = $(VSTestCollect)" Importance="low" />
<Message Text="VSTestBlame = $(VSTestBlame)" Importance="low" />
<Message Text="VSTestTraceDataCollectorDirectoryPath = $(TraceDataCollectorDirectoryPath)" Importance="low" />
<Message Text="VSTestNoLogo = $(VSTestNoLogo)" Importance="low" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Create corresponding PR in dotnet cli repo.

</Target>

</Project>
11 changes: 11 additions & 0 deletions src/Microsoft.TestPlatform.Build/Tasks/VSTestTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ public string VSTestTraceDataCollectorDirectoryPath
set;
}

public string VSTestNoLogo
{
get;
set;
}

public override bool Execute()
{
var traceEnabledValue = Environment.GetEnvironmentVariable("VSTEST_BUILD_TRACE");
Expand Down Expand Up @@ -297,6 +303,11 @@ private List<string> AddArgs()
}
}

if(!string.IsNullOrWhiteSpace(this.VSTestNoLogo))
{
allArgs.Add("--nologo");
}

return allArgs;
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/vstest.console/CommandLine/Executor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,16 @@ internal int Execute(params string[] args)
{
this.testPlatformEventSource.VsTestConsoleStart();

this.PrintSplashScreen();
// If User specifies --nologo via dotnet, donot print splat screen
if (args != null && args.Length !=0 && args.Contains("--nologo"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this logic to function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Case insensitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are controlling this argument, can make case insensitive, but didn't need it now

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we support /nologo? Reference https://msdn.microsoft.com/en-us/library/ms164311.aspx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are not supporting this as first class argument yet, only if it comes from dotnet cli, & from dotnet cli it will always come as --nologo. I don't want to add additional checks for something we are not supporting

{
// Sanitizing this list, as I don't think we should write Argument processor for this.
args = args.Where(val => val != "--nologo").ToArray();
}
else
{
this.PrintSplashScreen();
}

int exitCode = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,5 +283,14 @@ public void CreateArgumentShouldNotAddTestAdapterPathIfVSTestTraceDataCollectorD

Assert.IsNull(allArguments.FirstOrDefault(arg => arg.Contains("--testAdapterPath:")));
}

[TestMethod]
public void CreateArgumentShouldAddNoLogoOptionIfSpecifiedByUser()
{
this.vsTestTask.VSTestNoLogo = "--nologo";
var allArguments = this.vsTestTask.CreateArgument().ToArray();

Assert.IsNotNull(allArguments.FirstOrDefault(arg => arg.Contains("--nologo")));
}
}
}
49 changes: 49 additions & 0 deletions test/vstest.console.UnitTests/ExecutorUnitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,55 @@ public void ExecutorPrintsSplashScreenTest()
Assert.IsTrue(mockOutput.Messages.First().Message.EndsWith(assemblyVersion));
}

[TestMethod]
public void ExecutorShouldNotPrintsSplashScreenIfNoLogoPassed()
{
var mockOutput = new MockOutput();
var exitCode = new Executor(mockOutput, this.mockTestPlatformEventSource.Object).Execute("--nologo");

Assert.AreEqual(1, exitCode, "Exit code must be One for bad arguments");

// Verify that messages exist
Assert.IsTrue(mockOutput.Messages.Count == 1, "Executor should not print no valid arguments provided");

// Just check first 20 characters - don't need to check whole thing as assembly version is variable
Assert.IsFalse(
mockOutput.Messages.First()
.Message.Contains(CommandLineResources.MicrosoftCommandLineTitle.Substring(0, 20)),
"First Printed message must be Microsoft Copyright");
}

[TestMethod]
public void ExecutorShouldSanitizeNoLogoInput()
{
var mockOutput = new MockOutput();
var exitCode = new Executor(mockOutput, this.mockTestPlatformEventSource.Object).Execute("--nologo");

Assert.AreEqual(1, exitCode, "Exit code must be One when no arguments are provided.");

Assert.IsTrue(mockOutput.Messages.Any(message => message.Message.Contains(CommandLineResources.NoArgumentsProvided)));
}

[TestMethod]
public void ExecutorShouldSanitizeNoLogoInputAndShouldProcessOtherArgs()
{
// Create temp file for testsource dll to pass FileUtil.Exits()
var testSourceDllPath = Path.GetTempFileName();
string[] args = { testSourceDllPath, "/tests:Test1", "/testCasefilter:Test", "--nologo" };
var mockOutput = new MockOutput();

var exitCode = new Executor(mockOutput, this.mockTestPlatformEventSource.Object).Execute(args);

var errorMessageCount = mockOutput.Messages.Count(msg => msg.Level == OutputLevel.Error && msg.Message.Contains(CommandLineResources.InvalidTestCaseFilterValueForSpecificTests));
Assert.AreEqual(1, errorMessageCount, "Invalid Arguments Combination should display error.");
Assert.AreEqual(1, exitCode, "Invalid Arguments Combination execution should exit with error.");

Assert.IsFalse(mockOutput.Messages.First().Message.Contains(CommandLineResources.MicrosoftCommandLineTitle.Substring(0, 20)),
"First Printed message must be Microsoft Copyright");

File.Delete(testSourceDllPath);
}


/// <summary>
/// Executor should Print Error message and Help contents when no arguments are provided.
Expand Down