From 30caede284f69bd04d8542a19000eb934322177f Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Thu, 15 Mar 2018 14:40:10 -0700 Subject: [PATCH] Fix list tool command tests to be localizable. Currently the list tool command tests, while localizing the column headers, didn't properly take into account the fact that localized builds might produce strings longer than the English versions of the column header strings. This results in a mismatch of the actual from the expected due to additional column padding. The fix is to stop using a static expected table and do a simple calculation of the expected table based on the length of the localized strings. Fixes issue related to PR #8799. --- src/dotnet/PrintableTable.cs | 2 +- .../dotnet-list-tool/ListToolCommand.cs | 8 +- .../CommandTests/ListToolCommandTests.cs | 116 ++++++++++-------- 3 files changed, 70 insertions(+), 56 deletions(-) diff --git a/src/dotnet/PrintableTable.cs b/src/dotnet/PrintableTable.cs index ce31e13cc..f5a0afd58 100644 --- a/src/dotnet/PrintableTable.cs +++ b/src/dotnet/PrintableTable.cs @@ -13,7 +13,7 @@ namespace Microsoft.DotNet.Cli // Represents a table (with rows of type T) that can be printed to a terminal. internal class PrintableTable { - private const string ColumnDelimiter = " "; + public const string ColumnDelimiter = " "; private List _columns = new List(); private class Column diff --git a/src/dotnet/commands/dotnet-list/dotnet-list-tool/ListToolCommand.cs b/src/dotnet/commands/dotnet-list/dotnet-list-tool/ListToolCommand.cs index b567ac74d..2c3608d3c 100644 --- a/src/dotnet/commands/dotnet-list/dotnet-list-tool/ListToolCommand.cs +++ b/src/dotnet/commands/dotnet-list/dotnet-list-tool/ListToolCommand.cs @@ -15,7 +15,7 @@ namespace Microsoft.DotNet.Tools.List.Tool { internal class ListToolCommand : CommandBase { - private const string CommandDelimiter = ", "; + public const string CommandDelimiter = ", "; private readonly AppliedOption _options; private readonly IToolPackageStore _toolPackageStore; private readonly IReporter _reporter; @@ -66,20 +66,20 @@ namespace Microsoft.DotNet.Tools.List.Tool .ToArray(); } - private bool PackageHasCommands(IToolPackage p) + private bool PackageHasCommands(IToolPackage package) { try { // Attempt to read the commands collection // If it fails, print a warning and treat as no commands - return p.Commands.Count >= 0; + return package.Commands.Count >= 0; } catch (Exception ex) when (ex is ToolConfigurationException) { _errorReporter.WriteLine( string.Format( LocalizableStrings.InvalidPackageWarning, - p.Id, + package.Id, ex.Message).Yellow()); return false; } diff --git a/test/dotnet.Tests/CommandTests/ListToolCommandTests.cs b/test/dotnet.Tests/CommandTests/ListToolCommandTests.cs index 3b764abb4..c378e0803 100644 --- a/test/dotnet.Tests/CommandTests/ListToolCommandTests.cs +++ b/test/dotnet.Tests/CommandTests/ListToolCommandTests.cs @@ -62,14 +62,7 @@ namespace Microsoft.DotNet.Tests.Commands command.Execute().Should().Be(0); - _reporter.Lines.Should().Equal( - string.Format( - "{0} {1} {2}", - LocalizableStrings.PackageIdColumn, - LocalizableStrings.VersionColumn, - LocalizableStrings.CommandsColumn - ), - "-------------------------------------"); + _reporter.Lines.Should().Equal(EnumerateExpectedTableLines(store.Object)); } [Fact] @@ -92,15 +85,7 @@ namespace Microsoft.DotNet.Tests.Commands command.Execute().Should().Be(0); - _reporter.Lines.Should().Equal( - string.Format( - "{0} {1} {2}", - LocalizableStrings.PackageIdColumn, - LocalizableStrings.VersionColumn, - LocalizableStrings.CommandsColumn - ), - "-------------------------------------------", - "test.tool 1.3.5-preview foo "); + _reporter.Lines.Should().Equal(EnumerateExpectedTableLines(store.Object)); } [Fact] @@ -137,17 +122,7 @@ namespace Microsoft.DotNet.Tests.Commands command.Execute().Should().Be(0); - _reporter.Lines.Should().Equal( - string.Format( - "{0} {1} {2} ", - LocalizableStrings.PackageIdColumn, - LocalizableStrings.VersionColumn, - LocalizableStrings.CommandsColumn - ), - "----------------------------------------------", - "another.tool 2.7.3 bar ", - "some.tool 1.0.0 fancy-foo", - "test.tool 1.3.5-preview foo "); + _reporter.Lines.Should().Equal(EnumerateExpectedTableLines(store.Object)); } [Fact] @@ -172,15 +147,7 @@ namespace Microsoft.DotNet.Tests.Commands command.Execute().Should().Be(0); - _reporter.Lines.Should().Equal( - string.Format( - "{0} {1} {2} ", - LocalizableStrings.PackageIdColumn, - LocalizableStrings.VersionColumn, - LocalizableStrings.CommandsColumn - ), - "------------------------------------------------", - "test.tool 1.3.5-preview foo, bar, baz"); + _reporter.Lines.Should().Equal(EnumerateExpectedTableLines(store.Object)); } [Fact] @@ -212,20 +179,11 @@ namespace Microsoft.DotNet.Tests.Commands command.Execute().Should().Be(0); _reporter.Lines.Should().Equal( - string.Format( - LocalizableStrings.InvalidPackageWarning, - "another.tool", - "broken" - ).Yellow(), - string.Format( - "{0} {1} {2} ", - LocalizableStrings.PackageIdColumn, - LocalizableStrings.VersionColumn, - LocalizableStrings.CommandsColumn - ), - "--------------------------------------------", - "some.tool 1.0.0 fancy-foo", - "test.tool 1.3.5-preview foo "); + EnumerateExpectedTableLines(store.Object).Prepend( + string.Format( + LocalizableStrings.InvalidPackageWarning, + "another.tool", + "broken").Yellow())); } private IToolPackage CreateMockToolPackage(string id, string version, IReadOnlyList commands) @@ -257,5 +215,61 @@ namespace Microsoft.DotNet.Tests.Commands store, _reporter); } + + private IEnumerable EnumerateExpectedTableLines(IToolPackageStore store) + { + string GetCommandsString(IToolPackage package) + { + return string.Join(ListToolCommand.CommandDelimiter, package.Commands.Select(c => c.Name)); + } + + var packages = store.EnumeratePackages().Where(PackageHasCommands).OrderBy(package => package.Id); + var columnDelimiter = PrintableTable.ColumnDelimiter; + + int packageIdColumnWidth = LocalizableStrings.PackageIdColumn.Length; + int versionColumnWidth = LocalizableStrings.VersionColumn.Length; + int commandsColumnWidth = LocalizableStrings.CommandsColumn.Length; + foreach (var package in packages) + { + packageIdColumnWidth = Math.Max(packageIdColumnWidth, package.Id.ToString().Length); + versionColumnWidth = Math.Max(versionColumnWidth, package.Version.ToNormalizedString().Length); + commandsColumnWidth = Math.Max(commandsColumnWidth, GetCommandsString(package).Length); + } + + yield return string.Format( + "{0}{1}{2}{3}{4}", + LocalizableStrings.PackageIdColumn.PadRight(packageIdColumnWidth), + columnDelimiter, + LocalizableStrings.VersionColumn.PadRight(versionColumnWidth), + columnDelimiter, + LocalizableStrings.CommandsColumn.PadRight(commandsColumnWidth)); + + yield return new string( + '-', + packageIdColumnWidth + versionColumnWidth + commandsColumnWidth + (columnDelimiter.Length * 2)); + + foreach (var package in packages) + { + yield return string.Format( + "{0}{1}{2}{3}{4}", + package.Id.ToString().PadRight(packageIdColumnWidth), + columnDelimiter, + package.Version.ToNormalizedString().PadRight(versionColumnWidth), + columnDelimiter, + GetCommandsString(package).PadRight(commandsColumnWidth)); + } + } + + private static bool PackageHasCommands(IToolPackage package) + { + try + { + return package.Commands.Count >= 0; + } + catch (Exception ex) when (ex is ToolConfigurationException) + { + return false; + } + } } }