Skip to content

Commit 1368429

Browse files
trgiangvtrgiangv
committed
refactor(nunit): update assembly sharing policy to restrict shared assemblies
1 parent 13505d4 commit 1368429

7 files changed

Lines changed: 139 additions & 66 deletions

File tree

‎source/DevTools.NUnit.Host/Loading/NUnitSharedAssemblyPolicy.cs‎

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,16 @@ namespace DevTools.NUnit.Host.Loading;
66
/// <summary>
77
/// Host/platform assemblies that should reuse the host-loaded copy.
88
/// Host NuGet/API prefixes come from <see cref="HostSharedAssemblies.MatchesHostPackagePrefix"/>.
9-
/// <c>System.*</c> facades are shared; <c>Microsoft.*</c> is not.
10-
/// On net48, NuGet BCL polyfills stay generation-private.
9+
/// Versioned dependencies, including <c>System.*</c> and <c>Microsoft.*</c>
10+
/// NuGet assemblies, stay generation-private unless explicitly declared as host-owned.
1111
/// </summary>
1212
public static class NUnitSharedAssemblyPolicy
1313
{
1414
private static readonly string CoreContractAssemblyName =
1515
typeof(INUnitRuntimeSession).Assembly.GetName().Name!;
1616

1717
/// <summary>
18-
/// BCL names that are not <c>System.*</c> (no dot after System, or netstandard).
19-
/// <c>System.*</c> facades are matched by prefix in <see cref="IsShared"/>.
18+
/// Runtime assemblies with fixed platform ownership, rather than a namespace prefix.
2019
/// </summary>
2120
private static readonly HashSet<string> PlatformAssemblyNames = new(StringComparer.OrdinalIgnoreCase)
2221
{
@@ -25,26 +24,8 @@ public static class NUnitSharedAssemblyPolicy
2524
"System",
2625
"System.Core",
2726
"System.Private.CoreLib",
28-
"Microsoft.Win32.Registry",
2927
};
3028

31-
#if NETFRAMEWORK
32-
/// <summary>
33-
/// NuGet polyfills that must stay generation-private on net48 so Runtime binds
34-
/// coherently. On modern TFMs the host/Default copy is preferred instead.
35-
/// </summary>
36-
private static readonly HashSet<string> NetfxPrivateFacades = new(StringComparer.OrdinalIgnoreCase)
37-
{
38-
"System.Reflection.Metadata",
39-
"System.Collections.Immutable",
40-
"System.Memory",
41-
"System.Buffers",
42-
"System.Runtime.CompilerServices.Unsafe",
43-
"System.Numerics.Vectors",
44-
"System.Text.Encoding.CodePages",
45-
};
46-
#endif
47-
4829
public static bool IsShared(string assemblySimpleName)
4930
{
5031
if (string.IsNullOrWhiteSpace(assemblySimpleName))
@@ -61,15 +42,7 @@ public static bool IsShared(string assemblySimpleName)
6142
|| HostSharedAssemblies.MatchesHostPackagePrefix(assemblySimpleName))
6243
return true;
6344

64-
if (!assemblySimpleName.StartsWith("System.", StringComparison.OrdinalIgnoreCase))
65-
return false;
66-
67-
#if NETFRAMEWORK
68-
if (NetfxPrivateFacades.Contains(assemblySimpleName))
69-
return false;
70-
#endif
71-
72-
return true;
45+
return false;
7346
}
7447

7548
public static bool ShouldExcludeFromGenerationCopy(string filePath)

‎source/DevTools.Utilities/AssemblyLoading/HostSharedAssemblies.cs‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,6 @@ namespace DevTools.Utilities.AssemblyLoading;
99
/// </summary>
1010
public static class HostSharedAssemblies
1111
{
12-
private static readonly string[] FrameworkPrefixes =
13-
[
14-
"System.",
15-
"Microsoft.",
16-
];
17-
1812
private static readonly object InitLock = new();
1913
private static HostSharedAssemblyNames? _names;
2014

@@ -40,12 +34,6 @@ public static bool IsShared(string assemblyName)
4034
if (MatchesHostPackagePrefix(assemblyName))
4135
return true;
4236

43-
foreach (var prefix in FrameworkPrefixes)
44-
{
45-
if (assemblyName.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
46-
return true;
47-
}
48-
4937
return false;
5038
}
5139

@@ -70,8 +58,7 @@ public static bool MatchesHostPackagePrefix(string assemblyName)
7058
}
7159

7260
/// <summary>
73-
/// Returns whether the name is an explicit host API assembly, without
74-
/// applying the broad System/Microsoft convenience prefixes used by command loading.
61+
/// Returns whether the name is an explicitly declared host API assembly.
7562
/// </summary>
7663
public static bool IsExplicitHostAssembly(string assemblyName)
7764
{

‎tests/DevTools.NUnit.Host.NetFramework.Tests/NetFrameworkGenerationTestEnvironment.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,29 @@ public static NUnitGenerationManifest BuildDependencyGenerationTwo()
136136
return CreateBuilder(generationsRoot).Build(testAssembly);
137137
}
138138

139+
public static NUnitGenerationManifest BuildRootDependencyGenerationOne()
140+
{
141+
using var workspace = new TempWorkspace();
142+
var testAssembly = CreateRootDependencyWorkspace(workspace.Root, "root-dependency-generation-one");
143+
var generationsRoot = CreateIsolatedGenerationsRoot();
144+
return CreateBuilder(generationsRoot).Build(testAssembly);
145+
}
146+
147+
public static NUnitGenerationManifest BuildRootDependencyGenerationTwo()
148+
{
149+
using var workspace = new TempWorkspace();
150+
const string folderName = "root-dependency-generation-two";
151+
var testAssembly = CreateRootDependencyWorkspace(workspace.Root, folderName);
152+
PatchUtf16Constant(
153+
Path.Combine(workspace.Root, folderName, "GenerationPrivateDependency.dll"),
154+
BehaviorOneMarker,
155+
BehaviorTwoMarker,
156+
replaceAll: true);
157+
PatchUtf16Constant(testAssembly, BehaviorOneMarker, BehaviorTwoMarker, replaceAll: true);
158+
var generationsRoot = CreateIsolatedGenerationsRoot();
159+
return CreateBuilder(generationsRoot).Build(testAssembly);
160+
}
161+
139162
public static Assembly LoadConflictingNUnitIntoAppDomain()
140163
{
141164
if (!File.Exists(ConflictingNUnitStubPath))
@@ -189,6 +212,13 @@ private static string CreateDependencyWorkspace(string parentDirectory, string f
189212
return Path.Combine(workspace, "DependencyConsumer.dll");
190213
}
191214

215+
private static string CreateRootDependencyWorkspace(string parentDirectory, string folderName)
216+
{
217+
var workspace = Path.Combine(parentDirectory, folderName);
218+
CopyDirectory(DependencyConsumerOutputDirectory, workspace);
219+
return Path.Combine(workspace, "DependencyConsumer.dll");
220+
}
221+
192222
private static void PatchUtf16Constant(
193223
string filePath,
194224
string original,

‎tests/DevTools.NUnit.Host.NetFramework.Tests/NetFrameworkGenerationTests.cs‎

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,29 @@ public sealed class NetFrameworkGenerationTests
2020
public void SharedAssemblyPolicy_keeps_netfx_polyfills_generation_private()
2121
{
2222
HostSharedAssemblies.Use(new HostSharedAssemblyNames(["RevitAPI"], ["Autodesk."]));
23-
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Runtime"), Is.True);
24-
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Memory"), Is.False);
23+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System"), Is.True);
24+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Core"), Is.True);
25+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Runtime"), Is.False);
26+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Custom"), Is.False);
27+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("Microsoft.Custom"), Is.False);
28+
Assert.That(NUnitSharedAssemblyPolicy.IsShared("Microsoft.Win32.Registry"), Is.False);
29+
Assert.That(
30+
new[]
31+
{
32+
"System.Buffers",
33+
"System.Collections.Immutable",
34+
"System.Diagnostics.DiagnosticSource",
35+
"System.IO.Hashing",
36+
"System.IO.Pipelines",
37+
"System.Memory",
38+
"System.Numerics.Vectors",
39+
"System.Runtime.CompilerServices.Unsafe",
40+
"System.Text.Encodings.Web",
41+
"System.Text.Json",
42+
"System.Threading.Channels",
43+
"System.Threading.Tasks.Extensions",
44+
}.All(name => !NUnitSharedAssemblyPolicy.IsShared(name)),
45+
Is.True);
2546
Assert.That(NUnitSharedAssemblyPolicy.IsManagedAssemblyFile("HostSmokeTests.exe"), Is.True);
2647
Assert.That(NUnitSharedAssemblyPolicy.IsManagedAssemblyFile("nunit.framework.dll"), Is.True);
2748
Assert.That(NUnitSharedAssemblyPolicy.IsShared("System.Reflection.Metadata"), Is.False);
@@ -30,6 +51,40 @@ public void SharedAssemblyPolicy_keeps_netfx_polyfills_generation_private()
3051
Assert.That(NUnitSharedAssemblyPolicy.IsShared("Autodesk.Revit.DB"), Is.True);
3152
}
3253

54+
[Test]
55+
public void GenerationBuilder_keeps_versioned_system_and_microsoft_dependencies_private()
56+
{
57+
var workspace = Path.Combine(Path.GetTempPath(), "DevTools", "NUnit", Guid.NewGuid().ToString("N"));
58+
Directory.CreateDirectory(workspace);
59+
try
60+
{
61+
var testAssembly = NetFrameworkGenerationTestEnvironment.CreateGenerationOneAssembly(
62+
workspace,
63+
"versioned-bcl-dependencies");
64+
var outputDirectory = Path.GetDirectoryName(testAssembly)!;
65+
var testDirectory = Path.GetDirectoryName(typeof(NetFrameworkGenerationTests).Assembly.Location)!;
66+
67+
foreach (var fileName in new[] { "System.Text.Json.dll", "Microsoft.Bcl.AsyncInterfaces.dll" })
68+
{
69+
File.Copy(
70+
Path.Combine(testDirectory, fileName),
71+
Path.Combine(outputDirectory, fileName),
72+
overwrite: true);
73+
}
74+
75+
var generationsRoot = NetFrameworkGenerationTestEnvironment.CreateIsolatedGenerationsRoot();
76+
var manifest = NetFrameworkGenerationTestEnvironment.CreateBuilder(generationsRoot).Build(testAssembly);
77+
78+
Assert.That(File.Exists(Path.Combine(manifest.ShadowDirectory, "System.Text.Json.dll")), Is.True);
79+
Assert.That(File.Exists(Path.Combine(manifest.ShadowDirectory, "Microsoft.Bcl.AsyncInterfaces.dll")), Is.True);
80+
}
81+
finally
82+
{
83+
if (Directory.Exists(workspace))
84+
Directory.Delete(workspace, recursive: true);
85+
}
86+
}
87+
3388
[Test]
3489
public void Process_runs_on_clr_48()
3590
{
@@ -155,6 +210,23 @@ public void Create_resolves_same_identity_dependency_per_requesting_generation()
155210
Assert.That(factory.RetainedGenerationCount, Is.EqualTo(2));
156211
}
157212

213+
[Test]
214+
public void Create_resolves_root_dependency_per_requesting_generation()
215+
{
216+
var generationOne = NetFrameworkGenerationTestEnvironment.BuildRootDependencyGenerationOne();
217+
var generationTwo = NetFrameworkGenerationTestEnvironment.BuildRootDependencyGenerationTwo();
218+
219+
using var factory = new NetfxNUnitRuntimeSessionFactory();
220+
using var sessionOne = factory.Create(generationOne);
221+
using var sessionTwo = factory.Create(generationTwo);
222+
223+
var caseOne = RunDependencyProbe(sessionOne, generationOne);
224+
var caseTwo = RunDependencyProbe(sessionTwo, generationTwo);
225+
226+
Assert.That(caseOne.Output, Does.Contain("dependency-behavior=behavior-one"));
227+
Assert.That(caseTwo.Output, Does.Contain("dependency-behavior=behavior-two"));
228+
}
229+
158230
[Test]
159231
public void Create_concurrent_generations_remain_isolated()
160232
{
@@ -423,4 +495,11 @@ private static NUnitCaseResult RunDependencyProbe(INUnitRuntimeSession session,
423495
Assert.That(run.GenerationId, Is.EqualTo(manifest.GenerationId));
424496
return run.Cases.Single();
425497
}
498+
499+
private sealed class NoOpEventSink : INUnitRuntimeEventSink
500+
{
501+
public void Publish(NUnitRuntimeEvent runtimeEvent)
502+
{
503+
}
504+
}
426505
}

‎tests/DevTools.NUnit.Host.Tests/NUnitGenerationBuilderTests.cs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public void Build_keeps_package_dependency_with_Microsoft_prefix_generation_priv
257257
}
258258

259259
[Fact]
260-
public void Build_excludes_shared_runtime_framework_dependencies_from_generation_copy()
260+
public void Build_keeps_runtime_framework_dependencies_generation_private()
261261
{
262262
using var workspace = new TempWorkspace();
263263
var testAssembly = NUnitGenerationTestEnvironment.CreateFixtureWorkspace(
@@ -293,14 +293,14 @@ public void Build_excludes_shared_runtime_framework_dependencies_from_generation
293293

294294
var manifest = builder.Build(testAssembly);
295295

296-
Assert.DoesNotContain(
296+
Assert.Contains(
297297
manifest.ManagedAssemblies,
298298
path => path.EndsWith("System.Reflection.Metadata.dll", StringComparison.OrdinalIgnoreCase));
299-
Assert.DoesNotContain(
299+
Assert.Contains(
300300
manifest.ManagedAssemblies,
301301
path => path.EndsWith("System.Collections.Immutable.dll", StringComparison.OrdinalIgnoreCase));
302-
Assert.False(File.Exists(Path.Combine(manifest.ShadowDirectory, "System.Reflection.Metadata.dll")));
303-
Assert.False(File.Exists(Path.Combine(manifest.ShadowDirectory, "System.Collections.Immutable.dll")));
302+
Assert.True(File.Exists(Path.Combine(manifest.ShadowDirectory, "System.Reflection.Metadata.dll")));
303+
Assert.True(File.Exists(Path.Combine(manifest.ShadowDirectory, "System.Collections.Immutable.dll")));
304304
}
305305

306306
[Fact]
@@ -558,16 +558,16 @@ public void Build_creates_new_generation_when_source_content_changes()
558558
}
559559

560560
[Fact]
561-
public void SharedAssemblyPolicy_shares_host_packages_and_system_prefix_not_microsoft_extensions()
561+
public void SharedAssemblyPolicy_shares_only_platform_and_declared_host_assemblies()
562562
{
563563
HostSharedAssemblies.Use(new HostSharedAssemblyNames(["RevitAPI"], ["Autodesk."]));
564564
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System"));
565565
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System.Private.CoreLib"));
566-
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System.Runtime"));
567-
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System.Custom"));
568-
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System.Reflection.Metadata"));
569-
Assert.True(NUnitSharedAssemblyPolicy.IsShared("System.Collections.Immutable"));
570-
Assert.True(NUnitSharedAssemblyPolicy.IsShared("Microsoft.Win32.Registry"));
566+
Assert.False(NUnitSharedAssemblyPolicy.IsShared("System.Runtime"));
567+
Assert.False(NUnitSharedAssemblyPolicy.IsShared("System.Custom"));
568+
Assert.False(NUnitSharedAssemblyPolicy.IsShared("System.Reflection.Metadata"));
569+
Assert.False(NUnitSharedAssemblyPolicy.IsShared("System.Collections.Immutable"));
570+
Assert.False(NUnitSharedAssemblyPolicy.IsShared("Microsoft.Win32.Registry"));
571571
Assert.True(NUnitSharedAssemblyPolicy.IsShared("RevitAPI"));
572572
Assert.True(NUnitSharedAssemblyPolicy.IsShared("MahApps.Metro"));
573573
Assert.True(NUnitSharedAssemblyPolicy.IsShared("Autodesk.Revit.DB"));

‎tests/DevTools.NUnit.Host.Tests/NUnitRuntimeSessionFactoryTests.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,15 @@ public void Create_uses_host_shared_nunit_distinct_from_default_context_conflict
102102
}
103103

104104
[Fact]
105-
public void ResolveAssembly_binds_system_console_from_default_context()
105+
public void ResolveAssembly_leaves_system_console_to_normal_clr_binding()
106106
{
107107
var manifest = NUnitRuntimeTestEnvironment.BuildFixtureGeneration();
108108
var loadContext = new NUnitRuntimeLoadContext(manifest);
109109
var requested = new AssemblyName("System.Console, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a");
110110

111111
var resolved = loadContext.ResolveAssemblyForTesting(requested);
112112

113-
Assert.NotNull(resolved);
114-
Assert.Same(AssemblyLoadContext.Default, AssemblyLoadContext.GetLoadContext(resolved!));
113+
Assert.Null(resolved);
115114
}
116115

117116
[Fact]

‎tests/DevTools.Utilities.Tests/AssemblyLoadingTests.cs‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,21 @@ public void ResolveFromAppDomain_finds_already_loaded_assembly()
1717
}
1818

1919
[Fact]
20-
public void HostSharedAssemblies_treats_system_prefix_as_shared()
20+
public void HostSharedAssemblies_shares_only_declared_host_assemblies()
2121
{
22-
Assert.True(HostSharedAssemblies.IsShared("System.Text.Json"));
22+
HostSharedAssemblies.Use(new HostSharedAssemblyNames(["RevitAPI"], ["Autodesk."]));
23+
Assert.True(HostSharedAssemblies.IsShared("RevitAPI"));
24+
Assert.True(HostSharedAssemblies.IsShared("Autodesk.Revit.DB"));
25+
Assert.False(HostSharedAssemblies.IsShared("System.Text.Json"));
26+
Assert.False(HostSharedAssemblies.IsShared("Microsoft.Extensions.Logging.Abstractions"));
2327
Assert.False(HostSharedAssemblies.IsShared("MyCustomPlugin"));
2428
}
2529

2630
[Fact]
27-
public void HostSharedAssemblies_package_prefix_excludes_microsoft_extensions()
31+
public void HostSharedAssemblies_package_prefixes_do_not_share_microsoft_extensions()
2832
{
29-
Assert.True(HostSharedAssemblies.IsShared("Microsoft.Extensions.Logging.Abstractions"));
33+
HostSharedAssemblies.Use(new HostSharedAssemblyNames([], []));
34+
Assert.False(HostSharedAssemblies.IsShared("Microsoft.Extensions.Logging.Abstractions"));
3035
Assert.False(HostSharedAssemblies.MatchesHostPackagePrefix("Microsoft.Extensions.Logging.Abstractions"));
3136
Assert.True(HostSharedAssemblies.MatchesHostPackagePrefix("MahApps.Metro"));
3237
Assert.True(HostSharedAssemblies.MatchesHostPackagePrefix("ControlzEx.Theming"));
@@ -48,7 +53,7 @@ public void DirectoryAssemblyLoader_returns_already_loaded_assembly_from_same_di
4853
}
4954

5055
[Fact]
51-
public void DirectoryAssemblyLoader_skips_shared_assemblies_not_in_appdomain()
56+
public void DirectoryAssemblyLoader_does_not_load_an_absent_system_dependency()
5257
{
5358
var directory = Path.GetTempPath();
5459
var resolved = DirectoryAssemblyLoader.TryLoad(directory, new AssemblyName("System.Text.Json"));

0 commit comments

Comments
 (0)