Code review feedback changes.

This commit implements the requested changes from the PR code review.

- Remove unnecessary comments
- Use Directory.Exists rather than getting file attributes
- Use GracefulException where appripriate
- Improve test names and fix spacing
- Don't force trailing slash on directory
- Don't use option forwarding
- Delete unnecessary MSBuild.exe[.config] files from test project
- Use forwarded options
This commit is contained in:
Andy Zivkovic 2018-09-11 12:09:16 -07:00 committed by Peter Huene
parent 22b4b8451e
commit 2b84ebe054
No known key found for this signature in database
GPG key ID: E1D265D820213D6A
35 changed files with 415 additions and 566 deletions

View file

@ -93,6 +93,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "dotnet-install-tool.Tests",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "InsertionTests", "InsertionTests\InsertionTests.csproj", "{A9713391-3D44-4664-9C41-75765218FD6C}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "dotnet-list-package.Tests", "dotnet-list-package.Tests\dotnet-list-package.Tests.csproj", "{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
@ -607,6 +609,18 @@ Global
{A9713391-3D44-4664-9C41-75765218FD6C}.Release|x64.Build.0 = Release|Any CPU
{A9713391-3D44-4664-9C41-75765218FD6C}.Release|x86.ActiveCfg = Release|Any CPU
{A9713391-3D44-4664-9C41-75765218FD6C}.Release|x86.Build.0 = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|Any CPU.Build.0 = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|x64.ActiveCfg = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|x64.Build.0 = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|x86.ActiveCfg = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Debug|x86.Build.0 = Debug|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|Any CPU.ActiveCfg = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|Any CPU.Build.0 = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|x64.ActiveCfg = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|x64.Build.0 = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|x86.ActiveCfg = Release|Any CPU
{377363FE-0B87-4ACB-AEF3-3FF6454EC8D4}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE

View file

@ -36,11 +36,11 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
cmd.Should().Pass();
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
new ListPackageCommand()
.WithPath(projectDirectory)
@ -52,7 +52,7 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
}
[Fact]
public void AutoReferencedPackages()
public void ItListsAutoReferencedPackages()
{
var testAsset = "TestAppSimple";
var projectDirectory = TestAssets
@ -63,11 +63,11 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
new ListPackageCommand()
.WithPath(projectDirectory)
@ -80,7 +80,7 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
}
[Fact]
public void RunOnSolution()
public void ItRunOnSolution()
{
var sln = "TestAppWithSlnAndSolutionFolders";
var projectDirectory = TestAssets
@ -91,11 +91,11 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
new ListPackageCommand()
.WithPath(projectDirectory)
@ -126,7 +126,7 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
}
[Fact]
public void TransitivePackagePrinted()
public void ItListsTransitivePackage()
{
var testAsset = "NewtonSoftDependentProject";
var projectDirectory = TestAssets
@ -137,11 +137,11 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
new ListPackageCommand()
.WithPath(projectDirectory)
@ -161,13 +161,13 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
}
[Theory]
[InlineData("", "[.NETFramework,Version=v4.5.1]", null)]
[InlineData("", "[.NETCoreApp,Version=v2.2]", null)]
[InlineData("--framework netcoreapp2.2 --framework net451", "[.NETFramework,Version=v4.5.1]", null)]
[InlineData("--framework netcoreapp2.2 --framework net451", "[.NETCoreApp,Version=v2.2]", null)]
[InlineData("--framework netcoreapp2.2", "[.NETCoreApp,Version=v2.2]", "[.NETFramework,Version=v4.5.1]")]
[InlineData("--framework net451", "[.NETFramework,Version=v4.5.1]", "[.NETCoreApp,Version=v2.2]")]
public void FrameworkSpecificList_Success(string args, string shouldInclude, string shouldntInclude)
[InlineData("", "[net451]", null)]
[InlineData("", "[netcoreapp2.2]", null)]
[InlineData("--framework netcoreapp2.2 --framework net451", "[net451]", null)]
[InlineData("--framework netcoreapp2.2 --framework net451", "[netcoreapp2.2]", null)]
[InlineData("--framework netcoreapp2.2", "[netcoreapp2.2]", "[net451]")]
[InlineData("--framework net451", "[net451]", "[netcoreapp2.2]")]
public void ItListsValidFrameworks(string args, string shouldInclude, string shouldntInclude)
{
var testAsset = "MSBuildAppWithMultipleFrameworks";
var projectDirectory = TestAssets
@ -178,38 +178,38 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
if (shouldntInclude == null)
{
new ListPackageCommand()
.WithPath(projectDirectory)
.Execute(args)
.Should()
.Pass()
.And.NotHaveStdErr()
.And.HaveStdOutContainingIgnoreSpaces(shouldInclude.Replace(" ", ""));
.WithPath(projectDirectory)
.Execute(args)
.Should()
.Pass()
.And.NotHaveStdErr()
.And.HaveStdOutContainingIgnoreSpaces(shouldInclude.Replace(" ", ""));
}
else
{
new ListPackageCommand()
.WithPath(projectDirectory)
.Execute(args)
.Should()
.Pass()
.And.NotHaveStdErr()
.And.HaveStdOutContainingIgnoreSpaces(shouldInclude.Replace(" ", ""))
.And.NotHaveStdOutContaining(shouldntInclude.Replace(" ", ""));
.WithPath(projectDirectory)
.Execute(args)
.Should()
.Pass()
.And.NotHaveStdErr()
.And.HaveStdOutContainingIgnoreSpaces(shouldInclude.Replace(" ", ""))
.And.NotHaveStdOutContaining(shouldntInclude.Replace(" ", ""));
}
}
[Fact]
public void FrameworkSpecificList_Fail()
public void ItDoesNotAcceptInvalidFramework()
{
var testAsset = "MSBuildAppWithMultipleFrameworks";
var projectDirectory = TestAssets
@ -220,20 +220,20 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass();
new ListPackageCommand()
.WithPath(projectDirectory)
.Execute("--framework invalid")
.Should()
.Fail();
.WithPath(projectDirectory)
.Execute("--framework invalid")
.Should()
.Fail();
}
[Fact]
public void FSharpProject()
public void ItListsFSharpProject()
{
var testAsset = "FSharpTestAppSimple";
var projectDirectory = TestAssets
@ -244,11 +244,11 @@ namespace Microsoft.DotNet.Cli.List.Package.Tests
.FullName;
new RestoreCommand()
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
.WithWorkingDirectory(projectDirectory)
.Execute()
.Should()
.Pass()
.And.NotHaveStdErr();
new ListPackageCommand()
.WithPath(projectDirectory)

View file

@ -1 +0,0 @@
https://github.com/Microsoft/msbuild/issues/927

View file

@ -1 +0,0 @@
https://github.com/Microsoft/msbuild/issues/927

View file

@ -14,23 +14,24 @@ namespace Microsoft.DotNet.Cli.List.Reference.Tests
{
public class GivenDotnetListReference : TestBase
{
private const string HelpText = @"Usage: dotnet list <PROJECT> reference [options]
private const string HelpText = @"Usage: dotnet list <PROJECT | SOLUTION> reference [options]
Arguments:
<PROJECT> The project file to operate on. If a file is not specified, the command will search the current directory for one.
<PROJECT | SOLUTION> The project or solution file to operate on. If a file is not specified, the command will search the current directory for one.
Options:
-h, --help Show command line help.";
private const string ListCommandHelpText = @"Usage: dotnet list [options] <PROJECT> [command]
private const string ListCommandHelpText = @"Usage: dotnet list [options] <PROJECT | SOLUTION> [command]
Arguments:
<PROJECT> The project file to operate on. If a file is not specified, the command will search the current directory for one.
<PROJECT | SOLUTION> The project or solution file to operate on. If a file is not specified, the command will search the current directory for one.
Options:
-h, --help Show command line help.
Commands:
package List all package references of the project or solution.
reference List all project-to-project references of the project.";
const string FrameworkNet451Arg = "-f net451";