fix(tests): repair the DriverTypeNames guard, and stop MSBuild hoarding worker nodes

Two independent housekeeping defects.

DriverTypeNamesGuardTests (3 failures, pre-existing). The guard discovers each
driver's *DriverFactoryExtensions.Register by reflection and invokes it, then
asserts the registered type-name set matches the DriverTypeNames constants. It
built the argument array positionally — registry first, null for everything after
— on the assumption that the remaining parameters were optional. Galaxy's are
not: its Register takes a REQUIRED ISecretResolver and guards it with
ArgumentNullException.ThrowIfNull, so the reflective call threw and all three
parity facts failed. OpcUaClient's equivalent parameter is optional, which is why
only Galaxy tripped it.

Worth stating plainly: the guard was not partially broken, it was completely
inert. The throw happened while building the registered set, so NO driver's
parity was ever checked — a genuine drift in any of the nine would have been
invisible behind the same three red tests. Arguments are now supplied by
parameter type, and a parameter the helper cannot satisfy fails with a message
naming the factory and the parameter instead of an opaque ArgumentNullException.
The stand-in resolver reports every secret absent, so if the "factory func is
never invoked" assumption is ever broken, the driver fails closed.
Core.Abstractions.Tests: 138 passed, 0 failed.

MSBuild node reuse. MSBuild leaves worker nodes resident between builds so the
next one starts warm; across this session's repeated solution builds and test
runs that reached 55 idle processes holding ~6 GB. That is not just untidy — the
integration suites assert on timeouts, so memory pressure produces failures
indistinguishable from real regressions, the same failure mode the Workstation-GC
fix addressed from a different direction.

Directory.Build.rsp now passes -nodeReuse:false. Measured on the same incremental
solution build: 15 residual nodes without it, 2 with, and elapsed time 6.12s vs
6.11s — no cost worth the memory on this machine.

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-21 18:42:34 -04:00
parent 50b55ee4b9
commit b2d19a1730
2 changed files with 67 additions and 5 deletions
+12
View File
@@ -0,0 +1,12 @@
# MSBuild auto-response file, applied to every build run from this repo root.
#
# Disable the MSBuild node pool. By default MSBuild leaves worker nodes resident
# after a build so the next one starts warm. Across a session of repeated
# solution-wide builds and test runs this repo accumulated 55 idle nodes holding
# ~6 GB, which is not merely untidy: the integration suites assert on timeouts,
# and memory pressure turns those into failures indistinguishable from real
# regressions (see the Workstation-GC note in Host.IntegrationTests.csproj for
# the same failure mode from a different cause).
#
# The cost is a small startup penalty per build. That is a good trade here.
-nodeReuse:false
@@ -3,6 +3,7 @@ using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
using ZB.MOM.WW.OtOpcUa.Core.Hosting;
using ZB.MOM.WW.Secrets.Abstractions;
namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests;
@@ -23,13 +24,40 @@ public sealed class DriverTypeNamesGuardTests
private static readonly string[] NonFactoryDriverAssemblySuffixes =
{ ".Contracts", ".Addressing", ".Cli" };
/// <summary>
/// A stand-in <see cref="ISecretResolver"/> for factories that require one.
/// </summary>
/// <remarks>
/// Every secret resolves to absent. That is deliberate and safe here: the test only needs the
/// type-name key each <c>Register</c> call installs, and never invokes the registered factory
/// func, so no secret is ever requested. Resolving to absent rather than to a fake value means
/// that if this assumption is ever broken, the driver fails closed.
/// </remarks>
private sealed class AbsentSecretResolver : ISecretResolver
{
public static readonly AbsentSecretResolver Instance = new();
public Task<string?> GetAsync(SecretName name, CancellationToken ct) =>
Task.FromResult<string?>(null);
}
/// <summary>
/// Build the authoritative registered-factory set by discovering every
/// <c>*DriverFactoryExtensions.Register</c> in the deployed driver assemblies (by convention) and
/// invoking it against a fresh registry. The factory func is never invoked — we only need the
/// type-name key each <c>Register</c> call installs — so <see langword="null"/> is passed for the
/// optional <c>loggerFactory</c>/other parameters.
/// invoking it against a fresh registry. The factory func is never invoked — only the type-name
/// key each <c>Register</c> call installs is needed.
/// </summary>
/// <remarks>
/// Argument construction is by parameter <i>type</i>, not by position. It used to pass
/// <see langword="null"/> for everything after the registry on the assumption that the
/// remaining parameters were all optional. They are not: <c>GalaxyDriverFactoryExtensions.Register</c>
/// takes a <b>required</b> <see cref="ISecretResolver"/> and guards it with
/// <c>ArgumentNullException.ThrowIfNull</c>, so the reflective call threw and all three parity
/// facts failed — for a reason that had nothing to do with the drift they exist to catch.
/// (<c>OpcUaClient</c>'s equivalent parameter is optional, which is why only Galaxy tripped it.)
/// A parameter this method cannot satisfy now fails with a message naming the factory and the
/// parameter, rather than surfacing as an opaque <see cref="ArgumentNullException"/>.
/// </remarks>
private static IReadOnlyCollection<string> RegisteredDriverTypeNames()
{
var registry = new DriverFactoryRegistry();
@@ -62,8 +90,7 @@ public sealed class DriverTypeNamesGuardTests
|| ps[0].ParameterType != typeof(DriverFactoryRegistry))
continue;
var args = new object?[ps.Length];
args[0] = registry; // remaining params (optional loggerFactory, etc.) default to null
var args = ps.Select(p => ArgumentFor(p, registry, type.Name)).ToArray();
register.Invoke(null, args);
invoked++;
}
@@ -78,6 +105,29 @@ public sealed class DriverTypeNamesGuardTests
return registry.RegisteredTypes;
}
/// <summary>
/// Supplies one argument for a discovered <c>Register</c> parameter, by type.
/// </summary>
/// <param name="parameter">The parameter to supply.</param>
/// <param name="registry">The registry the factories register into.</param>
/// <param name="factoryTypeName">The declaring factory type, for the failure message.</param>
/// <returns>The argument value to pass.</returns>
private static object? ArgumentFor(ParameterInfo parameter, DriverFactoryRegistry registry, string factoryTypeName)
{
if (parameter.ParameterType == typeof(DriverFactoryRegistry)) return registry;
if (parameter.ParameterType == typeof(ISecretResolver)) return AbsentSecretResolver.Instance;
// Optional or nullable-reference parameters are safe to omit; the factory is documented to
// cope. Anything else is a required dependency this method does not know how to build, and
// guessing null would resurface as an opaque ArgumentNullException from inside the factory.
if (parameter.IsOptional || !parameter.ParameterType.IsValueType) return null;
throw new InvalidOperationException(
$"{factoryTypeName}.Register requires parameter '{parameter.Name}' of type "
+ $"{parameter.ParameterType.Name}, which the DriverTypeNames guard does not know how to "
+ "supply. Add a case to ArgumentFor, or give the parameter a default on the factory.");
}
/// <summary>Reflect every <c>public const string</c> declared on <see cref="DriverTypeNames"/>.</summary>
private static IReadOnlyCollection<string> DeclaredConstantValues() =>
typeof(DriverTypeNames)