Improve runtime of Client Tests via fixture-level parallelization and fix parallel test isolation

[Copilot]

Client Tests in CI took ~25 minutes. The bottleneck was sequential execution of multi-transport-scheme fixtures and long combinatorial test methods (TransferSubscription ~5 min, ReconnectWithSavedSessionSecrets ~3 min). After enabling fixture-level parallelism, bugs in the test infrastructure caused test failures due to insufficient test isolation; these have been fixed.

Proposed changes

Fixture-level parallelization for multi-scheme tests

Added [Parallelizable(ParallelScope.Fixtures)] to ClientTest, ClientBatchTest, NodeCacheAsyncTest, and LoadTest. All four use [TestFixtureSource] with opc.tcp / https / opc.https — the three scheme instances now run concurrently.

Extract TransferSubscriptionTest — new parallelizable fixture

Moved TransferSubscriptionOnlyAsync (and debug variant) from SubscriptionTest into a dedicated TransferSubscriptionTest class:

• [TestFixtureSource] with one fixture instance per TransferType (5 values) + [Parallelizable(ParallelScope.Fixtures)]
• Each fixture runs its 4 combinations (sendInitialValues × sequentialPublishing) with [Parallelizable] on the test method
• SingleSession = true — one shared background session for namespace URI lookups; tests create their own origin/target sessions independently
• Result: 20 tests in ~2 min vs ~5 min

Extract ReconnectWithSavedSessionSecretsTest — new parallelizable fixture

Moved ReconnectWithSavedSessionSecretsOnlyAsync (and ECC variant) from SubscriptionTest into ReconnectWithSavedSessionSecretsTest:

• [TestFixtureSource] with one fixture per security policy (None, ECC_nistP256, Basic256Sha256) + [Parallelizable(ParallelScope.Fixtures)]
• 8 combinatorial cases (anonymous × sequentialPublishing × sendInitialValues) per fixture, marked [Parallelizable]
• ClientFixture.SessionFactory set once in OneTimeSetUpAsync (thread-safe; was previously set per-test)
• Result: 24 tests in ~1.5 min vs ~3 min

ClientTestFramework — shared helpers promoted

CreateSubscriptionsAsync and CreateMonitoredItemTestSet moved from SubscriptionTest to ClientTestFramework as protected methods. CreateSubscriptionsAsync now uses session.NamespaceUris from the explicitly passed session parameter rather than the fixture's shared this.Session, which is required for correct parallel execution.

Fix test isolation: retryStartServer bug, broadened ConnectAsync retry, and HTTPS static singleton race

retryStartServer flag never reset on success (ServerFixture.StartAsync, ClientFixture.StartReverseConnectHostAsync, Gds.Tests/Common.StartGDSAsync): retryStartServer was set to true in the catch block but never reset to false at the top of the do-while loop. When a retry succeeded, the flag stayed true, causing the loop to continue and attempt to start another server on the already-bound port—spawning orphaned servers and eventually exhausting all retries. Fixed by adding retryStartServer = false at the start of each loop iteration, with a random port selection on retry.

Port registry removed from GetNextFreeIPPort() (ServerFixtureUtils.cs): A global s_allocatedPorts registry introduced to prevent TOCTOU races proved harmful — tests that create a new server per [SetUp] (e.g. ClientLockoutIntegrationTests) caused the registry to grow unbounded, eventually exhausting all 100 attempts and throwing. The registry has been removed; random-port retry combined with the retryStartServer fix is sufficient to handle concurrent port allocation safely.

HttpRequestException not retried in ConnectAsync (ClientFixture.cs): For HTTPS/opc.https fixtures starting in parallel, a server-not-ready condition throws HttpRequestException (not a ServiceResultException), which the original retry logic did not catch. Fixed by adding a broad catch (Exception e) when (e is not IgnoreException && attempt < maxAttempts) retry clause. maxAttempts is also raised from 5 to 25 to give parallel HTTPS servers sufficient startup time under load.

Startup static state removed from HttpsTransportListener (Stack/Opc.Ua.Bindings.Https): Startup previously held a static field that every HTTPS server overwrote on startup, causing cross-fixture request misrouting when multiple server instances ran in parallel. All static state has been removed. The HttpsTransportListener instance is now registered in each web host's DI container via IWebHostBuilder.ConfigureServices(services => services.AddSingleton(this)) and injected into Startup.Configure as a method parameter. Method injection (rather than constructor injection) is used because on .NET 8+ HostBuilder activates Startup from the host-level service provider, which does not include services registered via IWebHostBuilder.ConfigureServices; the Configure method always resolves its parameters from the web-host app container, ensuring correct behaviour on all target frameworks.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply. You can also fill these out after creating the PR.

• Bugfix (non-breaking change which fixes an issue)
• Enhancement (non-breaking change which adds functionality)
• Test enhancement (non-breaking change to increase test coverage)
• Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
• Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING doc.
• I have signed the CLA.
• I ran tests locally with my changes, all passed.
• I fixed all failing tests in the CI pipelines.
• I fixed all introduced issues with CodeQL and LGTM.
• I have added tests that prove my fix is effective or that my feature works and increased code coverage.
• I have added necessary documentation (if appropriate).
• Any dependent changes have been merged and published in downstream modules.

Further comments

The parallelization is safe because:

• Each fixture instance spins up its own server on an independent port (GetNextFreeIPPort + random-port retry on failure)
• Tests that create sessions use ClientFixture.ConnectAsync which is stateless per call
• SingleSession = true suppresses per-test SetUpAsync/TearDownAsync session churn, eliminating shared-state races between parallel test cases within one fixture
• The retryStartServer fix ensures each server start attempt is independent; orphaned servers (previously started during spurious loop iterations) no longer accumulate
• ConnectAsync now retries on all transient errors (including HttpRequestException from HTTPS servers not yet ready) with up to 25 attempts, ensuring robust parallel fixture startup
• Startup in HttpsTransportListener now carries no static state; each Kestrel web host resolves its own listener instance from the DI container, ensuring complete isolation between concurrent HTTPS server instances

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.00%. Comparing base (edbbfa3) to head (b47f40b).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3655      +/-   ##
==========================================
+ Coverage   69.96%   70.00%   +0.03%     
==========================================
  Files         477      477              
  Lines      103300   103300              
  Branches    17400    17400              
==========================================
+ Hits        72270    72310      +40     
+ Misses      25870    25828      -42     
- Partials     5160     5162       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[romanett]

@copilot Test runtime is better, but now there are failing Tests due to insuficficient Test isolation. Improve Test isolation while keeping the runtime at the current Level

[Copilot]

@copilot Test runtime is better, but now there are failing Tests due to insuficficient Test isolation. Improve Test isolation while keeping the runtime at the current Level

Fixed in commit 04fe94b. Two root causes were identified from the CI logs (127 NullReferenceException failures on session null + BadServerHalted):

1. TOCTOU race in GetNextFreeIPPort() — with many fixtures starting in parallel, the temporary socket was disposed before the server bound the port, allowing a second fixture to receive the same ephemeral port. Fixed by adding a global port registry (s_allocatedPorts + s_portRegistryLock) in ServerFixtureUtils; the lock is held while binding to port 0 so no two concurrent callers can receive the same value. Added a 100-attempt safety limit with a descriptive exception.

2. retryStartServer never reset on success — in ServerFixture.StartAsync, ClientFixture.StartReverseConnectHostAsync, and Gds.Tests/Common.StartGDSAsync, retryStartServer was set to true in the catch block but never reset at the top of the do-while loop. When a retry succeeded, the flag stayed true, causing the loop to continue and start additional servers on already-bound ports, eventually exhausting all retries and crashing OneTimeSetUp. Fixed by adding retryStartServer = false at the start of each iteration. On retry, the old auto-allocated port is now released via ReleasePort() before allocating a new unique one.

No test code (fixture parallelism, test methods) was changed — only the underlying port allocation infrastructure.

[romanett]

@copilot Please fix the failing Client Tests, there seems to be another isolation issue.

[Copilot]

@copilot Please fix the failing Client Tests, there seems to be another isolation issue.

Fixed in commit 516aa8f. Two issues were causing the remaining failures:

1. Port registry exhaustion (ServerFixtureUtils): The global s_allocatedPorts set grew unbounded — tests like ClientLockoutIntegrationTests create a server per [SetUp] test, so recycled OS ports kept appearing "allocated," eventually exhausting 100 attempts and throwing. Fixed by removing the registry entirely. The retryStartServer = false fix (from the prior commit) already handles the TOCTOU scenario; random-port retry is sufficient.

2. HttpRequestException not retried (ClientFixture.ConnectAsync): For HTTPS/opc.https fixtures starting in parallel, a server-not-ready condition throws HttpRequestException (not ServiceResultException), which the original retry logic didn't catch. Fixed by adding a broad catch (Exception e) when (e is not IgnoreException && attempt < maxAttempts) retry clause, and increased maxAttempts from 5 → 25 to give parallel HTTPS servers enough time to start.

Note: ReadDisplayNamesAsync in ClientTest shows a BadSessionIdInvalid keep-alive error when run in isolation — confirmed as pre-existing (reproduces with the original binary before any of these changes).

[romanett]

@copilot Please fix the failing Client Tests, there seems to be another isolation issue.

[Copilot]

@copilot Please fix the failing Client Tests, there seems to be another isolation issue.

Fixed in commit c1fa3f7 (with minor follow-up in 9c2845f).

Root cause: HttpsTransportListener.Startup.Listener was a static singleton that every HTTPS server overwrote on startup. With multiple [Parallelizable(ParallelScope.Fixtures)] test classes (ClientTest, ClientBatchTest, NodeCacheAsyncTest, LoadTest) each launching https and opc.https fixture instances simultaneously, the last server to start captured the singleton. Kestrel middleware on all earlier servers then routed every request to that "last" listener — a different server instance entirely — causing BadServerHalted / NullReferenceException cascades.

Fix: replaced Startup.Listener (single static reference) with Startup.Listeners (ConcurrentDictionary<int, HttpsTransportListener> keyed by local TCP port). Start() registers the listener under its port; Dispose() removes it. The Kestrel middleware now dispatches each request to the correct listener via context.Connection.LocalPort, allowing unlimited concurrent HTTPS server instances on distinct ports.

[romanett]

@copilot please remove the static Field from HttpsTransportListener Startup completely instead use the integrated DI container of IWebHostBuilder to retrieve the transportlistener to handle a request

[Copilot]

@copilot please remove the static Field from HttpsTransportListener Startup completely instead use the integrated DI container of IWebHostBuilder to retrieve the transportlistener to handle a request

Done in commit b47f40b. The static Listeners dictionary has been removed entirely.

The listener is now registered in the web host's DI container via webHostBuilder.ConfigureServices(services => services.AddSingleton(this)) inside ConfigureWebHost. Startup.Configure receives it as a method-injected parameter:

public void Configure(IApplicationBuilder appBuilder, HttpsTransportListener listener)
{
    appBuilder.Run(context => listener.SendAsync(context));
}

Method injection (rather than constructor injection) is used because on .NET 8+ HostBuilder activates the Startup class from the host-level service provider, which does not include services registered via IWebHostBuilder.ConfigureServices. The Configure method always resolves its parameters from the web-host app container, so method injection works correctly on all target frameworks.