Skip to content

Commit 8549159

Browse files
baronfelbaronfel
authored
Merge pull request #392 from dotnet/containerize-error-formatting
2 parents 6a6571f + 0fc03f5 commit 8549159

File tree

21 files changed

+652
-219
lines changed

21 files changed

+652
-219
lines changed

‎Microsoft.NET.Build.Containers.IntegrationTests/CommandUtils/CommandResultAssertions.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Linq;
54
using System.Text.RegularExpressions;
65
using FluentAssertions;
76
using FluentAssertions.Execution;

‎Microsoft.NET.Build.Containers.IntegrationTests/FullFramework/CreateNewImageToolTaskTests.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,39 +27,39 @@ public void GenerateCommandLineCommands_ThrowsWhenRequiredPropertiesNotSet()
2727
CreateNewImage task = new();
2828

2929
Exception e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
30-
Assert.Equal("Required property 'PublishDirectory' was not set or empty.", e.Message);
30+
Assert.Equal("CONTAINER4001: Required property 'PublishDirectory' was not set or empty.", e.Message);
3131

3232
DirectoryInfo publishDir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), DateTime.Now.ToString("yyyyMMddHHmmssfff")));
3333

3434
task.PublishDirectory = publishDir.FullName;
3535

3636
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
37-
Assert.Equal("Required property 'BaseRegistry' was not set or empty.", e.Message);
37+
Assert.Equal("CONTAINER4001: Required property 'BaseRegistry' was not set or empty.", e.Message);
3838

3939
task.BaseRegistry = "MyBaseRegistry";
4040

4141
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
42-
Assert.Equal("Required property 'BaseImageName' was not set or empty.", e.Message);
42+
Assert.Equal("CONTAINER4001: Required property 'BaseImageName' was not set or empty.", e.Message);
4343

4444
task.BaseImageName = "MyBaseImageName";
4545

4646
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
47-
Assert.Equal("Required property 'ImageName' was not set or empty.", e.Message);
47+
Assert.Equal("CONTAINER4001: Required property 'ImageName' was not set or empty.", e.Message);
4848

4949
task.ImageName = "MyImageName";
5050

5151
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
52-
Assert.Equal("Required property 'WorkingDirectory' was not set or empty.", e.Message);
52+
Assert.Equal("CONTAINER4001: Required property 'WorkingDirectory' was not set or empty.", e.Message);
5353

5454
task.WorkingDirectory = "MyWorkingDirectory";
5555

5656
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
57-
Assert.Equal("Required 'Entrypoint' items were not set.", e.Message);
57+
Assert.Equal("CONTAINER4002: Required 'Entrypoint' items were not set.", e.Message);
5858

5959
task.Entrypoint = new[] { new TaskItem("") };
6060

6161
e = Assert.Throws<InvalidOperationException>(() => task.GenerateCommandLineCommandsInt());
62-
Assert.Equal("Required 'Entrypoint' items contain empty items.", e.Message);
62+
Assert.Equal("CONTAINER4003: Required 'Entrypoint' items contain empty items.", e.Message);
6363

6464
task.Entrypoint = new[] { new TaskItem("MyEntryPoint") };
6565

‎Microsoft.NET.Build.Containers.IntegrationTests/ParseContainerPropertiesTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void RegexCatchesInvalidContainerNames()
6262

6363
var instance = project.CreateProjectInstance(global::Microsoft.Build.Execution.ProjectInstanceSettings.None);
6464
Assert.True(instance.Build(new[]{ComputeContainerConfig}, new [] { logs }, null, out var outputs));
65-
Assert.Contains(logs.Messages, m => m.Code == ErrorCodes.CONTAINER001 && m.Importance == global::Microsoft.Build.Framework.MessageImportance.High);
65+
Assert.Contains(logs.Messages, m => m.Message?.Contains("'ContainerImageName' was not a valid container image name, it was normalized to 'dotnet-testimage'") == true);
6666
}
6767

6868
[DockerDaemonAvailableFact]
@@ -79,7 +79,7 @@ public void RegexCatchesInvalidContainerTags()
7979
Assert.False(instance.Build(new[]{ComputeContainerConfig}, new [] { logs }, null, out var outputs));
8080

8181
Assert.True(logs.Errors.Count > 0);
82-
Assert.Equal(logs.Errors[0].Code, ErrorCodes.CONTAINER004);
82+
Assert.Equal(logs.Errors[0].Code, ErrorCodes.CONTAINER2007);
8383
}
8484

8585
[DockerDaemonAvailableFact]
@@ -97,6 +97,6 @@ public void CanOnlySupplyOneOfTagAndTags()
9797
Assert.False(instance.Build(new[]{ComputeContainerConfig}, new [] { logs }, null, out var outputs));
9898

9999
Assert.True(logs.Errors.Count > 0);
100-
Assert.Equal(logs.Errors[0].Code, ErrorCodes.CONTAINER005);
100+
Assert.Equal(logs.Errors[0].Code, ErrorCodes.CONTAINER2008);
101101
}
102102
}

‎Microsoft.NET.Build.Containers.UnitTests/DiagnosticMessageTests.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.NET.Build.Containers.Resources;
5+
using Xunit;
6+
7+
namespace Microsoft.NET.Build.Containers.UnitTests
8+
{
9+
public class DiagnosticMessageTests
10+
{
11+
[Fact]
12+
public void Error_ShouldCreateValidMessage()
13+
{
14+
string result = DiagnosticMessage.Error("code", "text");
15+
16+
Assert.Equal("Containerize : error code : text", result);
17+
}
18+
19+
[Fact]
20+
public void Warning_ShouldCreateValidMessage()
21+
{
22+
string result = DiagnosticMessage.Error("code", "text");
23+
24+
Assert.Equal("Containerize : error code : text", result);
25+
}
26+
27+
[Fact]
28+
public void Error_ShouldCreateValidMessageFromResource()
29+
{
30+
string result = DiagnosticMessage.ErrorFromResourceWithCode(nameof(Strings._Test), "param");
31+
32+
Assert.Equal("Containerize : error CONTAINER0000: Value for unit test param", result);
33+
}
34+
35+
[Fact]
36+
public void Warning_ShouldCreateValidMessageFromResource()
37+
{
38+
string result = DiagnosticMessage.WarningFromResourceWithCode(nameof(Strings._Test), "param");
39+
40+
Assert.Equal("Containerize : warning CONTAINER0000: Value for unit test param", result);
41+
}
42+
}
43+
}

‎Microsoft.NET.Build.Containers.UnitTests/Resources/ResourceTests.cs‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections;
5+
using System.Globalization;
6+
using System.Resources;
47
using Microsoft.NET.Build.Containers.Resources;
58
using Xunit;
69

@@ -11,13 +14,40 @@ public class ResourceTests
1114
[Fact]
1215
public void GetString_ReturnsValueFromResources()
1316
{
14-
Assert.Equal("Value for unit test {0}", Resource.GetString(nameof(Strings._Test)));
17+
Assert.Equal("CONTAINER0000: Value for unit test {0}", Resource.GetString(nameof(Strings._Test)));
1518
}
1619

1720
[Fact]
1821
public void FormatString_ReturnsValueFromResources()
1922
{
20-
Assert.Equal("Value for unit test 1", Resource.FormatString(nameof(Strings._Test), 1));
23+
Assert.Equal("CONTAINER0000: Value for unit test 1", Resource.FormatString(nameof(Strings._Test), 1));
24+
}
25+
26+
[Fact]
27+
public void EnsureErrorCodeUniqueness()
28+
{
29+
ResourceSet? resourceSet = Resource.Manager.GetResourceSet(CultureInfo.InvariantCulture, true, true);
30+
Assert.NotNull(resourceSet);
31+
32+
IEnumerable<IGrouping<string, DictionaryEntry>> groups = resourceSet
33+
.OfType<DictionaryEntry>()
34+
.Where(e => e.Value is string value && value.StartsWith("CONTAINER", StringComparison.OrdinalIgnoreCase))
35+
.GroupBy(e => e.Value!.ToString()!.Substring(9, 4))
36+
.Where(g => g.Count() > 1);
37+
38+
foreach(IGrouping<string, DictionaryEntry> group in groups)
39+
{
40+
if (!group.First().Key!.ToString()!.Contains('_'))
41+
{
42+
Assert.Fail($"Code with number '{group.Key}' is used for multiple resources. You can use single code for multiple messages, but the name of these resources must share same prefix delimited by underscore.");
43+
}
44+
else
45+
{
46+
string prefix = group.First().Key!.ToString()!.Split('_')[0];
47+
48+
Assert.All(group, e => e.Key!.ToString()!.StartsWith(prefix, StringComparison.Ordinal));
49+
}
50+
}
2151
}
2252
}
2353
}

‎Microsoft.NET.Build.Containers/ContainerBuilder.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ await outReg.PushAsync(
9999
}
100100
catch (Exception e)
101101
{
102-
Console.WriteLine($"Containerize: error CONTAINER001: Failed to push to output registry: {e.Message}");
102+
Console.WriteLine(DiagnosticMessage.ErrorFromResourceWithCode(nameof(Strings.RegistryOutputPushFailed), e.Message));
103103
Environment.ExitCode = 1;
104104
}
105105
}
@@ -109,7 +109,7 @@ await outReg.PushAsync(
109109
var localDaemon = GetLocalDaemon(localContainerDaemon, Console.WriteLine);
110110
if (!(await localDaemon.IsAvailableAsync(cancellationToken).ConfigureAwait(false)))
111111
{
112-
Console.WriteLine("Containerize: error CONTAINER007: The Docker daemon is not available, but pushing to a local daemon was requested. Please start Docker and try again.");
112+
Console.WriteLine(DiagnosticMessage.ErrorFromResourceWithCode(nameof(Strings.LocalDaemondNotAvailable)));
113113
Environment.ExitCode = 7;
114114
return;
115115
}
@@ -120,7 +120,7 @@ await outReg.PushAsync(
120120
}
121121
catch (Exception e)
122122
{
123-
Console.WriteLine($"Containerize: error CONTAINER001: Failed to push to local docker registry: {e.Message}");
123+
Console.WriteLine(DiagnosticMessage.ErrorFromResourceWithCode(nameof(Strings.RegistryOutputPushFailed), e.Message));
124124
Environment.ExitCode = 1;
125125
}
126126
}
@@ -132,7 +132,7 @@ private static LocalDocker GetLocalDaemon(string localDaemonType, Action<string>
132132
var daemon = localDaemonType switch
133133
{
134134
KnownDaemonTypes.Docker => new LocalDocker(logger),
135-
_ => throw new ArgumentException($"Unknown local container daemon type '{localDaemonType}'. Valid local container daemon types are {String.Join(",", KnownDaemonTypes.SupportedLocalDaemonTypes)}", nameof(localDaemonType))
135+
_ => throw new ArgumentException(Resource.FormatString(nameof(Strings.UnknownDaemonType), localDaemonType, String.Join(",", KnownDaemonTypes.SupportedLocalDaemonTypes)), nameof(localDaemonType))
136136
};
137137
return daemon;
138138
}

‎Microsoft.NET.Build.Containers/DiagnosticMessage.cs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Text;
5+
using Microsoft.NET.Build.Containers.Resources;
6+
7+
namespace Microsoft.NET.Build.Containers;
8+
9+
/// <summary>
10+
/// Represents a diagnostic message that could be parsed by VS.
11+
/// https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks?view=vs-2022
12+
/// </summary>
13+
internal static class DiagnosticMessage
14+
{
15+
public static string Warning(string code, string text) => Create("warning", code, text);
16+
17+
public static string Error(string code, string text) => Create("error", code, text);
18+
19+
public static string WarningFromResourceWithCode(string resourceName, params object?[] args) => CreateFromResourceWithCode("warning", resourceName, args);
20+
21+
public static string ErrorFromResourceWithCode(string resourceName, params object?[] args) => CreateFromResourceWithCode("error", resourceName, args);
22+
23+
private static string Create(string category, string code, string text)
24+
{
25+
StringBuilder builder = new();
26+
27+
builder.Append("Containerize : "); // tool name as the origin
28+
builder.Append(category);
29+
builder.Append(' ');
30+
builder.Append(code);
31+
builder.Append(" : ");
32+
builder.Append(text);
33+
34+
return builder.ToString();
35+
}
36+
37+
private static string CreateFromResourceWithCode(string category, string resourceName, params object?[] args)
38+
{
39+
string textWithCode = Resource.FormatString(resourceName, args);
40+
41+
StringBuilder builder = new();
42+
43+
builder.Append("Containerize : "); // tool name as the origin
44+
builder.Append(category);
45+
builder.Append(' ');
46+
builder.Append(textWithCode);
47+
48+
return builder.ToString();
49+
}
50+
}

‎Microsoft.NET.Build.Containers/Globals.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,9 @@
33

44
using System.Runtime.CompilerServices;
55

6+
using System.Resources;
7+
8+
[assembly: NeutralResourcesLanguage("en")]
9+
[assembly: InternalsVisibleTo("containerize")]
610
[assembly: InternalsVisibleTo("Microsoft.NET.Build.Containers.UnitTests")]
711
[assembly: InternalsVisibleTo("Microsoft.NET.Build.Containers.IntegrationTests")]

‎Microsoft.NET.Build.Containers/KnownStrings.cs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
namespace Microsoft.NET.Build.Containers;
55

6-
public static class KnownStrings
6+
internal static class KnownStrings
77
{
88
public static class Properties
99
{
@@ -31,8 +31,19 @@ public static class Properties
3131

3232
public static class ErrorCodes
3333
{
34-
public static readonly string CONTAINER001 = nameof(CONTAINER001);
35-
public static readonly string CONTAINER004 = nameof(CONTAINER004);
36-
public static readonly string CONTAINER005 = nameof(CONTAINER005);
34+
public static readonly string CONTAINER1011 = nameof(CONTAINER1011);
35+
public static readonly string CONTAINER1012 = nameof(CONTAINER1012);
36+
public static readonly string CONTAINER1013 = nameof(CONTAINER1013);
37+
38+
public static readonly string CONTAINER2007 = nameof(CONTAINER2007);
39+
public static readonly string CONTAINER2008 = nameof(CONTAINER2008);
40+
public static readonly string CONTAINER2009 = nameof(CONTAINER2009);
41+
public static readonly string CONTAINER2010 = nameof(CONTAINER2010);
42+
public static readonly string CONTAINER2012 = nameof(CONTAINER2012);
43+
44+
public static readonly string CONTAINER4001 = nameof(CONTAINER4001);
45+
public static readonly string CONTAINER4002 = nameof(CONTAINER4002);
46+
public static readonly string CONTAINER4003 = nameof(CONTAINER4003);
47+
public static readonly string CONTAINER4004 = nameof(CONTAINER4004);
3748
}
38-
}
49+
}

0 commit comments

Comments
 (0)