From b2d19a173009e75b45575a9cc8c4df20a69a3fb7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 21 Jul 2026 18:42:34 -0400 Subject: [PATCH] fix(tests): repair the DriverTypeNames guard, and stop MSBuild hoarding worker nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- Directory.Build.rsp | 12 ++++ .../DriverTypeNamesGuardTests.cs | 60 +++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 Directory.Build.rsp diff --git a/Directory.Build.rsp b/Directory.Build.rsp new file mode 100644 index 00000000..7633ea7a --- /dev/null +++ b/Directory.Build.rsp @@ -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 diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverTypeNamesGuardTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverTypeNamesGuardTests.cs index 64c86d23..8f8c13a6 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverTypeNamesGuardTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/DriverTypeNamesGuardTests.cs @@ -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" }; + /// + /// A stand-in for factories that require one. + /// + /// + /// Every secret resolves to absent. That is deliberate and safe here: the test only needs the + /// type-name key each Register 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. + /// + private sealed class AbsentSecretResolver : ISecretResolver + { + public static readonly AbsentSecretResolver Instance = new(); + + public Task GetAsync(SecretName name, CancellationToken ct) => + Task.FromResult(null); + } + /// /// Build the authoritative registered-factory set by discovering every /// *DriverFactoryExtensions.Register 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 Register call installs — so is passed for the - /// optional loggerFactory/other parameters. + /// invoking it against a fresh registry. The factory func is never invoked — only the type-name + /// key each Register call installs is needed. /// + /// + /// Argument construction is by parameter type, not by position. It used to pass + /// for everything after the registry on the assumption that the + /// remaining parameters were all optional. They are not: GalaxyDriverFactoryExtensions.Register + /// takes a required and guards it with + /// ArgumentNullException.ThrowIfNull, 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. + /// (OpcUaClient'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 . + /// private static IReadOnlyCollection 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; } + /// + /// Supplies one argument for a discovered Register parameter, by type. + /// + /// The parameter to supply. + /// The registry the factories register into. + /// The declaring factory type, for the failure message. + /// The argument value to pass. + 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."); + } + /// Reflect every public const string declared on . private static IReadOnlyCollection DeclaredConstantValues() => typeof(DriverTypeNames)