Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
11 changes: 8 additions & 3 deletions src/Cli/dotnet/Commands/Test/VSTest/TestCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,16 @@ internal static int RunArtifactPostProcessingIfNeeded(string testSessionCorrelat

private static bool ContainsBuiltTestSources(string[] args)
{
foreach (string arg in args)
for (int i = 0; i < args.Length; i++)
{
if (!arg.StartsWith("-") &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We shouldn't have removed this StartsWith check. This is likely the cause of #51249.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the whole parsing changes, so if -dl is not split the same way -p is then the argument is no longer one string "-p:something.dll"

but instead it is

"-p", "something.dll"
essentially there is additioanl split on :, but maybe that parser is only applicable to property, I think I pointed to it in the issue. Will have to look , and add test for it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ignore me, I am tired :D

(arg.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || arg.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)))
string arg = args[i];
if (arg.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || arg.EndsWith(".exe", StringComparison.OrdinalIgnoreCase))
{
var previousArg = i > 0 ? args[i - 1] : null;
if (previousArg != null && CommonOptions.PropertiesOption.Aliases.Contains(previousArg))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This approach will probably not work for #51249 as the dll isn't provided in -p. We probably need to get a proper fix soon and get approval for getting a fix for GA?

{
return false;
}
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,38 @@ public void ArgumentsEndWithDllOrExeShouldNotFail(string arg)
}
}


[Theory]
[InlineData("-p:ABC=C:\\my.dll")]
[InlineData("/p:ABC=C:\\my.dll")]
[InlineData("-property:ABC=C:\\my.dll")]
public void PropertiesEndingWithDotDllShouldNotFail(string property)
{
var testProjectDirectory = CopyAndRestoreVSTestDotNetCoreTestApp([]);

// Call test
// The test will complain about --property:VsTestUseMSBuildOutput=false but
// it is the .dll parameter that is causing this. It forces the command to offload work
// to vstest.console.exe directly, because it thinks there is some test .dll that we should run
// directly, rather than a project file.
// Vstest.console.exe will then complain just about the first unknown parameter.
CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true)
.WithWorkingDirectory(testProjectDirectory)
.Execute(ConsoleLoggerOutputNormal.Concat([property]));

// Verify
if (!TestContext.IsLocalized())
{
result.StdOut.Should().Contain("Total tests: 2");
result.StdOut.Should().Contain("Passed: 1");
result.StdOut.Should().Contain("Failed: 1");
result.StdOut.Should().Contain("Passed VSTestPassTest");
result.StdOut.Should().Contain("Failed VSTestFailTest");
}

result.ExitCode.Should().Be(1);
}

private string CopyAndRestoreVSTestDotNetCoreTestApp(object[] parameters, [CallerMemberName] string callingMethod = "")
{
// Copy VSTestCore project in output directory of project dotnet-vstest.Tests
Expand Down