Skip to content

Commit f19a78b

Browse files
baronfelbaronfel
committed
addres code review
1 parent 17206e7 commit f19a78b

File tree

8 files changed

+43
-42
lines changed

8 files changed

+43
-42
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public override bool Execute()
161161
if (!ContainerHelpers.NormalizeImageName(ContainerImageName, out var normalizedImageName))
162162
{
163163
Log.LogMessage(null, KnownStrings.ErrorCodes.CONTAINER001, "Container.InvalidImageName", null, 0, 0, 0, 0, MessageImportance.High, "'{0}' was not a valid container image name, it was normalized to '{1}'", nameof(ContainerImageName), normalizedImageName);
164-
NewContainerImageName = normalizedImageName;
164+
NewContainerImageName = normalizedImageName!; // known to be not null due to output of NormalizeImageName
165165
}
166166
else
167167
{

‎Test.Microsoft.NET.Build.Containers.Filesystem/CapturingLogger.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using Microsoft.Build.Framework;
2+
3+
namespace Test.Microsoft.NET.Build.Containers;
4+
5+
public class CapturingLogger : ILogger
6+
{
7+
public LoggerVerbosity Verbosity { get => LoggerVerbosity.Diagnostic; set { } }
8+
public string Parameters { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
9+
10+
private List<BuildMessageEventArgs> _messages = new();
11+
public IReadOnlyList<BuildMessageEventArgs> Messages {get { return _messages; } }
12+
13+
private List<BuildWarningEventArgs> _warnings = new();
14+
public IReadOnlyList<BuildWarningEventArgs> Warnings {get { return _warnings; } }
15+
16+
private List<BuildErrorEventArgs> _errors = new();
17+
public IReadOnlyList<BuildErrorEventArgs> Errors {get { return _errors; } }
18+
19+
public void Initialize(IEventSource eventSource)
20+
{
21+
eventSource.MessageRaised += (o, e) => _messages.Add(e);
22+
eventSource.WarningRaised += (o, e) => _warnings.Add(e);
23+
eventSource.ErrorRaised += (o, e) => _errors.Add(e);
24+
}
25+
26+
27+
public void Shutdown()
28+
{
29+
}
30+
}

‎Test.Microsoft.NET.Build.Containers.Filesystem/CurrentFile.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ public static class CurrentFile
77
public static string Path([CallerFilePath] string file = "") => file;
88

99
public static string Relative(string relative, [CallerFilePath] string file = "") {
10-
return global::System.IO.Path.Combine(global::System.IO.Path.GetDirectoryName(file), relative);
10+
return global::System.IO.Path.Combine(global::System.IO.Path.GetDirectoryName(file)!, relative); // file known to be not-null due to the mechanics of CallerFilePath
1111
}
1212
}

‎Test.Microsoft.NET.Build.Containers.Filesystem/ParseContainerPropertiesTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ public class ParseContainerPropertiesTests
99
[TestMethod]
1010
public void Baseline()
1111
{
12-
var (project, _) = Evaluator.InitProject(new () {
12+
var (project, _) = ProjectInitializer.InitProject(new () {
1313
[ContainerBaseImage] = "mcr.microsoft.com/dotnet/runtime:7.0",
1414
[ContainerRegistry] = "localhost:5010",
1515
[ContainerImageName] = "dotnet/testimage",
@@ -29,7 +29,7 @@ public void Baseline()
2929
[TestMethod]
3030
public void SpacesGetReplacedWithDashes()
3131
{
32-
var (project, _) = Evaluator.InitProject(new () {
32+
var (project, _) = ProjectInitializer.InitProject(new () {
3333
[ContainerBaseImage] = "mcr microsoft com/dotnet runtime:7.0",
3434
[ContainerRegistry] = "localhost:5010"
3535
});
@@ -45,7 +45,7 @@ public void SpacesGetReplacedWithDashes()
4545
[TestMethod]
4646
public void RegexCatchesInvalidContainerNames()
4747
{
48-
var (project, logs) = Evaluator.InitProject(new () {
48+
var (project, logs) = ProjectInitializer.InitProject(new () {
4949
[ContainerBaseImage] = "mcr.microsoft.com/dotnet/runtime:7.0",
5050
[ContainerRegistry] = "localhost:5010",
5151
[ContainerImageName] = "dotnet testimage",
@@ -60,7 +60,7 @@ public void RegexCatchesInvalidContainerNames()
6060
[TestMethod]
6161
public void RegexCatchesInvalidContainerTags()
6262
{
63-
var (project, logs) = Evaluator.InitProject(new () {
63+
var (project, logs) = ProjectInitializer.InitProject(new () {
6464
[ContainerBaseImage] = "mcr.microsoft.com/dotnet/runtime:7.0",
6565
[ContainerRegistry] = "localhost:5010",
6666
[ContainerImageName] = "dotnet/testimage",
@@ -77,7 +77,7 @@ public void RegexCatchesInvalidContainerTags()
7777
[TestMethod]
7878
public void CanOnlySupplyOneOfTagAndTags()
7979
{
80-
var (project, logs) = Evaluator.InitProject(new () {
80+
var (project, logs) = ProjectInitializer.InitProject(new () {
8181
[ContainerBaseImage] = "mcr.microsoft.com/dotnet/runtime:7.0",
8282
[ContainerRegistry] = "localhost:5010",
8383
[ContainerImageName] = "dotnet/testimage",

‎Test.Microsoft.NET.Build.Containers.Filesystem/Evaluator.cs renamed to ‎Test.Microsoft.NET.Build.Containers.Filesystem/ProjectInitializer.cs

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
using System.Diagnostics.CodeAnalysis;
21
using System.Runtime.CompilerServices;
32
using Microsoft.Build.Evaluation;
43
using Microsoft.Build.Framework;
54
using Microsoft.Build.Locator;
65

76
namespace Test.Microsoft.NET.Build.Containers;
87

9-
public static class Evaluator {
8+
public static class ProjectInitializer {
109
private static string CombinedTargetsLocation;
1110

1211
private static string CombineFiles(string propsFile, string targetsFile)
@@ -67,30 +66,3 @@ public static (Project, CapturingLogger) InitProject(Dictionary<string, string>
6766
return (collection.LoadProject(CombinedTargetsLocation, props, null), logs);
6867
}
6968
}
70-
71-
public class CapturingLogger : ILogger
72-
{
73-
public LoggerVerbosity Verbosity { get => LoggerVerbosity.Diagnostic; set { } }
74-
public string Parameters { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
75-
76-
private List<BuildMessageEventArgs> _messages = new();
77-
public IReadOnlyList<BuildMessageEventArgs> Messages {get { return _messages; } }
78-
79-
private List<BuildWarningEventArgs> _warnings = new();
80-
public IReadOnlyList<BuildWarningEventArgs> Warnings {get { return _warnings; } }
81-
82-
private List<BuildErrorEventArgs> _errors = new();
83-
public IReadOnlyList<BuildErrorEventArgs> Errors {get { return _errors; } }
84-
85-
public void Initialize(IEventSource eventSource)
86-
{
87-
eventSource.MessageRaised += (o, e) => _messages.Add(e);
88-
eventSource.WarningRaised += (o, e) => _warnings.Add(e);
89-
eventSource.ErrorRaised += (o, e) => _errors.Add(e);
90-
}
91-
92-
93-
public void Shutdown()
94-
{
95-
}
96-
}

‎Test.Microsoft.NET.Build.Containers.Filesystem/TargetsTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ public class TargetsTests
1111
[TestMethod]
1212
public void CanSetEntrypointArgsToUseAppHost(bool useAppHost, params string[] entrypointArgs)
1313
{
14-
var (project, _) = Evaluator.InitProject(new()
14+
var (project, _) = ProjectInitializer.InitProject(new()
1515
{
1616
[UseAppHost] = useAppHost.ToString()
1717
}, projectName: $"{nameof(CanSetEntrypointArgsToUseAppHost)}_{useAppHost}_{String.Join("_", entrypointArgs)}");
@@ -31,7 +31,7 @@ public void CanSetEntrypointArgsToUseAppHost(bool useAppHost, params string[] en
3131
[TestMethod]
3232
public void CanNormalizeInputContainerNames(string projectName, string expectedContainerImageName, bool shouldPass)
3333
{
34-
var (project, _) = Evaluator.InitProject(new()
34+
var (project, _) = ProjectInitializer.InitProject(new()
3535
{
3636
[AssemblyName] = projectName
3737
}, projectName: $"{nameof(CanNormalizeInputContainerNames)}_{projectName}_{expectedContainerImageName}_{shouldPass}");
@@ -48,7 +48,7 @@ public void CanNormalizeInputContainerNames(string projectName, string expectedC
4848
[DataRow("7.0.100-preview.1", false)]
4949
[TestMethod]
5050
public void CanWarnOnInvalidSDKVersions(string sdkVersion, bool isAllowed) {
51-
var (project, _) = Evaluator.InitProject(new()
51+
var (project, _) = ProjectInitializer.InitProject(new()
5252
{
5353
["NETCoreSdkVersion"] = sdkVersion,
5454
["PublishProfile"] = "DefaultContainer"

‎Test.Microsoft.NET.Build.Containers.Filesystem/Test.Microsoft.NET.Build.Containers.Filesystem.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
<TargetFramework>net7.0</TargetFramework>
55
<ImplicitUsings>enable</ImplicitUsings>
66
<IsPackable>false</IsPackable>
7-
<Nullable>enable</Nullable>
87
</PropertyGroup>
98

109
<ItemGroup>

‎Test.Microsoft.NET.Build.Containers.Filesystem/TestInitialization.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ public static class TestInitialization {
66
[AssemblyInitialize]
77
public static void Initialize(TestContext ctx) {
88
DockerRegistryManager.StartAndPopulateDockerRegistry(ctx);
9-
Evaluator.LocateMSBuild(ctx);
9+
ProjectInitializer.LocateMSBuild(ctx);
1010
}
1111

1212
[AssemblyCleanup]
1313
public static void Cleanup() {
1414
DockerRegistryManager.ShutdownDockerRegistry();
15-
Evaluator.Cleanup();
15+
ProjectInitializer.Cleanup();
1616
}
1717
}

0 commit comments

Comments
 (0)