Skip to content

Commit 12fa9ed

Browse files
CharliePooleCharliePoole
committed
Refactoring and changes from code review
1 parent 471c8dd commit 12fa9ed

File tree

20 files changed

+347
-482
lines changed

20 files changed

+347
-482
lines changed

‎package-tests.cake

+4-4
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ StandardRunnerTests.Add(new PackageTest(1, "Net462Test")
5757
AddToBothLists(new PackageTest(1, "Net80Test")
5858
{
5959
Description = "Run mock-assembly.dll under .NET 8.0",
60-
Arguments = "testdata/net8.0/mock-assembly.dll --trace:Debug",
60+
Arguments = "testdata/net8.0/mock-assembly.dll",
6161
ExpectedResult = new MockAssemblyExpectedResult("netcore-8.0")
6262
});
6363

@@ -293,14 +293,14 @@ StandardRunnerTests.Add(new PackageTest(1, "Net80WindowsFormsTest")
293293
AddToBothLists(new PackageTest(1, "Net60WPFTest")
294294
{
295295
Description = "Run test using WPF under .NET 6.0",
296-
Arguments = "testdata/net6.0-windows/WpfTest.dll --trace=Debug",
296+
Arguments = "testdata/net6.0-windows/WpfTest.dll",
297297
ExpectedResult = new ExpectedResult("Passed") { Assemblies = new[] { new ExpectedAssemblyResult("WpfTest.dll", "netcore-6.0") } }
298298
});
299299

300300
AddToBothLists(new PackageTest(1, "Net80WPFTest")
301301
{
302302
Description = "Run test using WPF under .NET 8.0",
303-
Arguments = "testdata/net8.0-windows/WpfTest.dll --trace=Debug",
303+
Arguments = "testdata/net8.0-windows/WpfTest.dll",
304304
ExpectedResult = new ExpectedResult("Passed") { Assemblies = new[] { new ExpectedAssemblyResult("WpfTest.dll", "netcore-8.0") } }
305305
});
306306

@@ -382,7 +382,7 @@ StandardRunnerTests.Add(new PackageTest(1, "V2ResultWriterTest_Net462")
382382
//StandardRunnerTests.Add(new PackageTest(1, "VSProjectLoaderTest_Solution")
383383
//{
384384
// Description = "Run mock-assembly using the .sln file",
385-
// Arguments = "../../src/TestData/TestData.sln --config=Release --trace=Debug",
385+
// Arguments = "../../src/TestData/TestData.sln --config=Release",
386386
// ExpectedResult = new ExpectedResult("Failed")
387387
// {
388388
// Total = 37 * 5,

‎src/NUnitCommon/nunit.agent.core/Drivers/NUnitFrameworkApi2009.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ public string Load(string testAssemblyPath, IDictionary<string, object> settings
5454

5555
_testAssemblyPath = testAssemblyPath;
5656

57-
// Normally, the caller should check for an invalid requested runtime, but we make sure here
57+
// Normally, the caller should check for an invalid requested runtime, but to be sure,
58+
// we check it. The setting value is only used for an error message.
5859
settings.TryGetValue(EnginePackageSettings.RequestedRuntimeFramework, out object? requestedRuntime);
5960

6061
var idPrefix = _driverId + "-";

‎src/NUnitEngine/nunit.engine.tests/Services/ProcessUtilsTests.cs renamed to ‎src/NUnitCommon/nunit.common.tests/ProcessUtilsTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
using NUnit.Framework;
2525
using System.Text;
2626

27-
namespace NUnit.Engine.Services
27+
namespace NUnit.Common
2828
{
2929
public static class ProcessUtilsTests
3030
{

‎src/NUnitEngine/nunit.engine/Services/ProcessUtils.cs renamed to ‎src/NUnitCommon/nunit.common/ProcessUtils.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
using System.Text;
2525

26-
namespace NUnit.Engine.Services
26+
namespace NUnit.Common
2727
{
2828
public static class ProcessUtils
2929
{

‎src/NUnitConsole/nunit4-console/ConsoleRunner.cs

+32
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using NUnit.TextDisplay;
1313
using System.Runtime.InteropServices;
1414
using System.Text;
15+
using System.Data;
1516

1617
namespace NUnit.ConsoleRunner
1718
{
@@ -438,7 +439,11 @@ public static TestPackage MakeTestPackage(ConsoleOptions options)
438439
TestPackage package = new TestPackage(options.InputFiles);
439440

440441
if (options.RuntimeFrameworkSpecified)
442+
{
443+
// Temporarily use both settings until RuntimeFramework is retired
441444
package.AddSetting(EnginePackageSettings.RequestedRuntimeFramework, options.RuntimeFramework);
445+
package.AddSetting(EnginePackageSettings.RequestedFrameworkName, MakeFrameworkName(options.RuntimeFramework));
446+
}
442447

443448
if (options.RunAsX86)
444449
package.AddSetting(EnginePackageSettings.RunAsX86, true);
@@ -516,6 +521,33 @@ public static TestPackage MakeTestPackage(ConsoleOptions options)
516521
return package;
517522
}
518523

524+
private static string MakeFrameworkName(string runtimeFramework)
525+
{
526+
var parts = runtimeFramework.Split('-');
527+
528+
if (parts.Length == 2)
529+
{
530+
string runtime = parts[0];
531+
Version version = new Version(parts[1]);
532+
string? identifier = null;
533+
534+
switch (runtime)
535+
{
536+
case "netcore":
537+
identifier = FrameworkIdentifiers.NetCoreApp;
538+
break;
539+
case "net":
540+
identifier = version.Major > 4 ? FrameworkIdentifiers.NetCoreApp : FrameworkIdentifiers.NetFramework;
541+
break;
542+
}
543+
544+
if (identifier is not null)
545+
return $"{identifier},Version=v{version}";
546+
}
547+
548+
throw new ArgumentException($"Invalid RuntimeFramework: {runtimeFramework}", nameof(runtimeFramework));
549+
}
550+
519551
/// <summary>
520552
/// Sets test parameters, handling backwards compatibility.
521553
/// </summary>

‎src/NUnitEngine/nunit.engine.api/EnginePackageSettings.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
22

33
using System;
4+
using System.Runtime.Versioning;
45

56
namespace NUnit
67
{
@@ -72,21 +73,20 @@ public static class EnginePackageSettings
7273
/// are strings like "net-4.5", "mono-4.0", etc. Default is to
7374
/// use the target framework for which an assembly was built.
7475
/// </summary>
75-
[Obsolete("Use 'RequestedRuntimeFramework' instead.")]
76-
public const string RuntimeFramework = "RequestedRuntimeFramework";
76+
public const string RequestedRuntimeFramework = "RequestedRuntimeFramework";
7777

7878
/// <summary>
7979
/// Indicates the desired runtime to use for the tests. Values
8080
/// are strings like "net-4.5", "mono-4.0", etc. Default is to
8181
/// use the target framework for which an assembly was built.
8282
/// </summary>
83-
public const string RequestedRuntimeFramework = "RequestedRuntimeFramework";
83+
public const string RequestedFrameworkName = "RequestedFrameworkName";
8484

8585
/// <summary>
8686
/// Indicates the Target runtime selected for use by the engine,
8787
/// based on the requested runtime and assembly metadata.
8888
/// </summary>
89-
public const string TargetRuntimeFramework = "TargetRuntimeFramework";
89+
public const string TargetFrameworkName = "TargetFrameworkName";
9090

9191
/// <summary>
9292
/// Indicates the name of the agent requested by the user.

‎src/NUnitEngine/nunit.engine.api/TestAgentInfo.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@ namespace NUnit.Engine
88
/// The TestAgentInfo struct provides information about an
99
/// available agent for use by a runner.
1010
/// </summary>
11-
public struct TestAgentInfo
11+
public readonly struct TestAgentInfo
1212
{
1313
/// <summary>
1414
/// The name of this agent
1515
/// </summary>
16-
public string AgentName;
16+
public readonly string AgentName;
1717

1818
/// <summary>
1919
/// The agent type: InProcess, LocalProcess or RemoteProcess
2020
/// </summary>
21-
public TestAgentType AgentType;
21+
public readonly TestAgentType AgentType;
2222

2323
/// <summary>
2424
/// The target runtime used by this agent
2525
/// </summary>
26-
public FrameworkName TargetRuntime;
26+
public readonly FrameworkName TargetRuntime;
2727

2828
/// <summary>
2929
/// Construct a TestAgent Info

‎src/NUnitEngine/nunit.engine.tests/Services/AgentProcessTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
22

3-
#if NETFRAMEWORK
3+
#if NETFRAMEWORK && false
44
using System;
55
using System.IO;
66
using System.Diagnostics;

‎src/NUnitEngine/nunit.engine.tests/Services/AgentStoreTests.cs

+17-15
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public static void IdCannotBeReused()
3232
database.Register(DummyAgent);
3333
Assert.That(() => database.AddAgent(DummyAgentId, DummyProcess), Throws.ArgumentException.With.Property("ParamName").EqualTo("agentId"));
3434

35-
database.MarkProcessTerminated(DummyProcess);
35+
database.MarkTerminated(DummyAgentId);
3636
Assert.That(() => database.AddAgent(DummyAgentId, DummyProcess), Throws.ArgumentException.With.Property("ParamName").EqualTo("agentId"));
3737
}
3838

@@ -60,7 +60,7 @@ public static void AgentMustNotRegisterAfterTerminating()
6060
var database = new AgentStore();
6161

6262
database.AddAgent(DummyAgentId, DummyProcess);
63-
database.MarkProcessTerminated(DummyProcess);
63+
database.MarkTerminated(DummyAgentId);
6464
Assert.That(() => database.Register(DummyAgent), Throws.ArgumentException.With.Property("ParamName").EqualTo("agent"));
6565
}
6666

@@ -69,7 +69,7 @@ public static void AgentMustBeStartedBeforeTerminating()
6969
{
7070
var database = new AgentStore();
7171

72-
Assert.That(() => database.MarkProcessTerminated(DummyProcess),
72+
Assert.That(() => database.MarkTerminated(DummyAgentId),
7373
Throws.Exception.TypeOf<NUnitEngineException>().With.Message.EqualTo("Process terminated without registering an agent."));
7474
}
7575

@@ -108,7 +108,7 @@ public static void AgentIsNotReadyWhenTerminated()
108108

109109
database.AddAgent(DummyAgentId, DummyProcess);
110110
database.Register(DummyAgent);
111-
database.MarkProcessTerminated(DummyProcess);
111+
database.MarkTerminated(DummyAgentId);
112112
Assert.That(database.IsReady(DummyAgentId, out _), Is.False);
113113
}
114114

@@ -117,7 +117,7 @@ public static void AgentIsNotRunningWhenNotStarted()
117117
{
118118
var database = new AgentStore();
119119

120-
Assert.That(database.IsAgentProcessActive(DummyAgentId, out _), Is.False);
120+
Assert.That(database.IsAgentActive(DummyAgentId, out _), Is.False);
121121
}
122122

123123
[Test]
@@ -126,7 +126,7 @@ public static void AgentIsRunningWhenStarted()
126126
var database = new AgentStore();
127127

128128
database.AddAgent(DummyAgentId, DummyProcess);
129-
Assert.That(database.IsAgentProcessActive(DummyAgentId, out var process), Is.True);
129+
Assert.That(database.IsAgentActive(DummyAgentId, out var process), Is.True);
130130
Assert.That(process, Is.SameAs(DummyProcess));
131131
}
132132

@@ -137,7 +137,7 @@ public static void AgentIsRunningWhenRegistered()
137137

138138
database.AddAgent(DummyAgentId, DummyProcess);
139139
database.Register(DummyAgent);
140-
Assert.That(database.IsAgentProcessActive(DummyAgentId, out var process), Is.True);
140+
Assert.That(database.IsAgentActive(DummyAgentId, out var process), Is.True);
141141
Assert.That(process, Is.SameAs(DummyProcess));
142142
}
143143

@@ -148,8 +148,8 @@ public static void AgentIsNotRunningWhenTerminated()
148148

149149
database.AddAgent(DummyAgentId, DummyProcess);
150150
database.Register(DummyAgent);
151-
database.MarkProcessTerminated(DummyProcess);
152-
Assert.That(database.IsAgentProcessActive(DummyAgentId, out _), Is.False);
151+
database.MarkTerminated(DummyAgentId);
152+
Assert.That(database.IsAgentActive(DummyAgentId, out _), Is.False);
153153
}
154154

155155
[Test]
@@ -163,21 +163,20 @@ public static void ConcurrentOperationsDoNotCorruptState()
163163
{
164164
var id = Guid.NewGuid();
165165

166-
Assert.That(database.IsAgentProcessActive(id, out _), Is.False);
166+
Assert.That(database.IsAgentActive(id, out _), Is.False);
167167
Assert.That(database.IsReady(id, out _), Is.False);
168168

169169
database.AddAgent(id, DummyProcess);
170-
Assert.That(database.IsAgentProcessActive(id, out _), Is.True);
170+
Assert.That(database.IsAgentActive(id, out _), Is.True);
171171
Assert.That(database.IsReady(id, out _), Is.False);
172172

173173
// Pretend that the agent process started and registered
174174
database.Register(new DummyTestAgent(id));
175-
Assert.That(database.IsAgentProcessActive(id, out _), Is.True);
175+
Assert.That(database.IsAgentActive(id, out _), Is.True);
176176
Assert.That(database.IsReady(id, out _), Is.True);
177177

178-
//database.MarkProcessTerminated(DummyProcess);
179-
database.MarkAgentTerminated(id);
180-
Assert.That(database.IsAgentProcessActive(id, out _), Is.False);
178+
database.MarkTerminated(id);
179+
Assert.That(database.IsAgentActive(id, out _), Is.False);
181180
Assert.That(database.IsReady(id, out _), Is.False);
182181
}
183182
}, threadCount: Environment.ProcessorCount);
@@ -221,10 +220,13 @@ private sealed class DummyTestAgent : ITestAgent
221220
public DummyTestAgent(Guid id)
222221
{
223222
Id = id;
223+
AgentType = TestAgentType.LocalProcess;
224224
}
225225

226226
public Guid Id { get; }
227227

228+
public TestAgentType AgentType { get; }
229+
228230
public ITestEngineRunner CreateRunner(TestPackage package)
229231
{
230232
throw new NotImplementedException();

‎src/NUnitEngine/nunit.engine.tests/Services/RuntimeFrameworkServiceTests.cs

+10-7
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7-
using System.Text;
7+
using NUnit.Common;
88
using NUnit.Framework;
99

1010
namespace NUnit.Engine.Services
@@ -61,10 +61,10 @@ public void SelectRuntimeFramework(string runtime, bool runAsX86)
6161
Assert.That(File.Exists(assemblyPath), $"File does not exist: {assemblyPath}");
6262
var package = new TestPackage(assemblyPath);
6363

64-
var returnValue = _runtimeService.SelectRuntimeFramework(package);
64+
var expectedFrameworkName = RuntimeFramework.Parse(_runtimeService.SelectRuntimeFramework(package)).FrameworkName.ToString();
6565

66-
Assert.That(package.GetSetting("TargetRuntimeFramework", string.Empty), Is.EqualTo(returnValue));
67-
Assert.That(package.GetSetting("RunAsX86", false), Is.EqualTo(runAsX86));
66+
Assert.That(package.GetSetting(EnginePackageSettings.TargetFrameworkName, string.Empty), Is.EqualTo(expectedFrameworkName));
67+
Assert.That(package.GetSetting(EnginePackageSettings.RunAsX86, false), Is.EqualTo(runAsX86));
6868
}
6969

7070
[Test]
@@ -154,9 +154,12 @@ public void RuntimeFrameworkIsSetForSubpackages()
154154

155155
Assert.Multiple(() =>
156156
{
157-
Assert.That(net20Package.Settings[EnginePackageSettings.TargetRuntimeFramework], Is.EqualTo("net-2.0"));
158-
Assert.That(net40Package.Settings[EnginePackageSettings.TargetRuntimeFramework], Is.EqualTo("net-4.0"));
159-
Assert.That(topLevelPackage.Settings[EnginePackageSettings.TargetRuntimeFramework], Is.EqualTo("net-4.0"));
157+
#pragma warning disable 612, 618
158+
var NF = FrameworkIdentifiers.NetFramework;
159+
Assert.That(net20Package.Settings[EnginePackageSettings.TargetFrameworkName], Is.EqualTo($"{NF},Version=v2.0"));
160+
Assert.That(net40Package.Settings[EnginePackageSettings.TargetFrameworkName], Is.EqualTo($"{NF},Version=v4.0"));
161+
Assert.That(topLevelPackage.Settings[EnginePackageSettings.TargetFrameworkName], Is.EqualTo($"{NF},Version=v4.0"));
162+
#pragma warning restore 612, 618
160163
});
161164
}
162165
}

0 commit comments

Comments
 (0)