Files
UA-.NETStandard/tests/Opc.Ua.Server.Tests/ServerStartupTests.cs
romanett a451053758 Fix the recurring and permanent CI failures on master (#4446)
# Description

Almost every `master` build was reporting yellow or red, which meant the
pipeline signal had stopped being useful: a genuinely broken build
looked the same as a healthy one. I triaged every `master` build from
the last week across Azure DevOps (definition 14, 38 builds) and GitHub
Actions, clustered the errors by root cause, and fixed the nine that
were still live.

The headline finding is that the two loudest problems were not test bugs
at all. Codecov rejects the upload on **28 out of 28 builds**, and that
single warning is what turned every master build `partiallySucceeded`.
Separately, the weekly HA long-haul soak was mathematically incapable of
passing: the configured duration is per *scenario*, and the two
`[NonParallelizable]` soaks run back to back, so the floor is 2 x 60
minutes plus teardown against a 110-minute step budget.

A tenth cluster (the `Opc.Ua.Sessions.Tests` reverse-connect flake, 6
hits around Aug 31) was already fixed by #4366 and has not recurred, so
it needed no work.

## Clusters and root causes

| Cluster | Frequency | Root cause |
|---|---|---|
| codecov upload exits 1 | **28/28 builds** | Codecov returns
`Repository not found`; the resulting warning made every master build
yellow |
| `No space left on device` in coverage merge | nightly | Hosted Ubuntu
agent disk exhausted by the per-matrix coverage artifacts |
| Sample HA Long-Haul timeout | weekly | 2 soaks x 60 min against a 110
min budget, so it could never pass |
| `Artifact coverage-report already exists` | on stage re-run | Pipeline
artifact names must be unique per build |
| `NodeCache*` -> `address already in use` | macOS | `ServerFixture`
retried only `BadNoCommunication`; Kestrel https throws
`IOException`/`AddressInUseException` and escaped the retry, failing the
whole `OneTimeSetUp` |
| `MetricsAreEmittedForChannelLifetimeAsync` | macOS | Entry teardown
emits `close` *before* the `active(-1)` up/down counter, but the test
only waited for `close` |
| `OnPublishReceivedKeepAliveShouldDispatchKeepAliveAsync` | net472 |
The processor queues the ack *after* dispatching the notification the
test waits on |
| `ACarriedPartIsAuthoredAtTheGripperItRidesInAsync` | macOS | A single
starved sample (0.2125 m) tripped a 0.20 m absolute worst-case bound |
| `RobotIntentLiveChannelTests` fixture timeout | net472 | 30 s startup
budget too tight, and the failure text interpolated `lastException`
*before* the polling that assigns it, so it always read `Last error: .`
|

## Approach

**Pipeline.** Coverage reporting is best-effort by design, so the
codecov step now swallows the uploader's exit code and reports the
failure as plain log output rather than a warning that colours the run.
The coverage merge job reclaims the unused Android/GHC/Swift/CodeQL
toolchains before anything writes to the agent, and qualifies its
artifact name on a retry only, so the normal first attempt still
publishes `coverage-report`. The long-haul budgets were raised above the
real floor in both the workflow and the ADO pipeline, and the display
names now say `min/scenario` so the next reader does not repeat the same
miscalculation.

**Tests.** Each test fix targets the specific ordering or bound that was
wrong, rather than adding sleeps. The two metric/ack races now wait on
the measurement the assertions actually depend on. The OpenUSD test
gates the 95th percentile instead of the absolute worst case: the defect
it guards is systematic and moves *every* sample 0.25-0.49 m while the
robot drives, so it still fails that gate loudly, while one straddled
sample no longer can.

## Worth a careful look

- **The port-collision fix is the one real behavioural change.**
`ServerFixtureUtils.IsPortUnavailable` now classifies every transport's
form of the collision, including matching Kestrel's
`AddressInUseException` by type name so the test framework needs no
reference to the ASP.NET Core connection abstractions. It deliberately
does *not* match unrelated failures, which is covered by a negative
test, so a certificate error still surfaces immediately instead of being
retried 25 times on different ports.
- **I dropped an integration test I had written for that fix.** While
validating it I found the UA-TCP listener tolerates a partial bind on
Windows, so `StartAsync` succeeds on the same port and the test asserted
something untrue. I replaced it with deterministic unit tests of the
classifier instead of shipping a test that would itself become flaky.
- **The codecov fix is a mitigation, not a cure.** The `Repository not
found` response is account-side: the `CODECOV_TOKEN` is stale or the
repository is not activated. This change stops it from colouring the
build and logs the failure plainly, but uploads will not actually land
until someone refreshes the token. Flagging that explicitly so it is not
mistaken for a full fix.

## Related Issues

N/A - this addresses CI failures observed directly on `master` builds
rather than a filed issue.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [x] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### Notes on the checklist

- Documentation: no doc updates were needed. The changes are CI
configuration and test-harness fixes with no public API surface, and the
rationale is captured in comments at each changed site.
- Local verification: all touched projects build clean on **net10.0**
and **net48**, and the targeted tests pass (Server 5/5, Client 46/46,
Features 31/31, Robotics.Intent 20/20, plus the Di and Core.Tests
cases). One pre-existing local `net48` PKI-store failure in
`Opc.Ua.Robotics.Intent.Tests` reproduces on unmodified `master` and is
unrelated to this change; CI's `net472` leg runs that project fine.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
2026-09-09 09:16:35 +00:00

130 lines
5.3 KiB
C#

/* ========================================================================
* Copyright (c) 2005-2025 The OPC Foundation, Inc. All rights reserved.
*
* OPC Foundation MIT License 1.00
*
* Permission is hereby granted, free of charge, to any person
* obtaining a copy of this software and associated documentation
* files (the "Software"), to deal in the Software without
* restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following
* conditions:
*
* The above copyright notice and this permission notice shall be
* included in all copies or substantial portions of the Software.
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
* OTHER DEALINGS IN THE SOFTWARE.
*
* The complete license agreement can be found here:
* http://opcfoundation.org/License/MIT/1.00/
* ======================================================================*/
using System;
using System.IO;
using System.Net.Sockets;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Connections;
using NUnit.Framework;
using Opc.Ua.Server.TestFramework;
namespace Opc.Ua.Server.Tests
{
/// <summary>
/// Test Standard Server startup.
/// </summary>
[TestFixture]
[Category("Server")]
[SetCulture("en-us")]
[SetUICulture("en-us")]
[Parallelizable]
public class ServerStartupTests
{
[DatapointSource]
public string[] UriSchemes =
[
Utils.UriSchemeOpcTcp,
Utils.UriSchemeHttps,
Utils.UriSchemeOpcHttps
];
/// <summary>
/// Start a server fixture with different uri schemes.
/// </summary>
[Theory]
public async Task StartServerAsync(string uriScheme)
{
var fixture = new ServerFixture<StandardServer>(t => new StandardServer(t));
Assert.That(fixture, Is.Not.Null);
fixture.UriScheme = uriScheme;
StandardServer server = await fixture.StartAsync().ConfigureAwait(false);
Assert.That(server, Is.Not.Null);
await Task.Delay(1000).ConfigureAwait(false);
await fixture.StopAsync().ConfigureAwait(false);
}
/// <summary>
/// The port a fixture is handed is only known to be free at the moment
/// it is picked, so a parallel fixture or an unrelated process on the
/// agent can take it before the server binds. Every transport's way of
/// reporting that has to be recognised as retryable: the https
/// listeners bind through Kestrel, which surfaces the collision as an
/// IOException wrapping an AddressInUseException rather than as a
/// ServiceResultException, and used to escape the retry and fail the
/// caller's whole OneTimeSetUp.
/// </summary>
[Test]
public void IsPortUnavailableRecognisesEveryTransportsPortCollision()
{
var uaTcp = new ServiceResultException(
StatusCodes.BadNoCommunication, "Failed to open a listening socket.");
var inUse = new SocketException((int)SocketError.AddressAlreadyInUse);
var kestrel = new IOException(
"Failed to bind to address https://[::]:49254: address already in use.",
new AddressInUseException("Address already in use.", inUse));
Assert.Multiple(() =>
{
Assert.That(ServerFixtureUtils.IsPortUnavailable(uaTcp), Is.True);
Assert.That(ServerFixtureUtils.IsPortUnavailable(inUse), Is.True);
Assert.That(ServerFixtureUtils.IsPortUnavailable(kestrel), Is.True);
Assert.That(
ServerFixtureUtils.IsPortUnavailable(new AggregateException(kestrel)),
Is.True);
});
}
/// <summary>
/// A failure that has nothing to do with the port must not be retried
/// on a different one - it would only hide the real error behind a
/// second, identical failure.
/// </summary>
[Test]
public void IsPortUnavailableIgnoresUnrelatedFailures()
{
Assert.Multiple(() =>
{
Assert.That(
ServerFixtureUtils.IsPortUnavailable(
new InvalidOperationException("Application instance certificate invalid!")),
Is.False);
Assert.That(
ServerFixtureUtils.IsPortUnavailable(
new ServiceResultException(StatusCodes.BadCertificateInvalid)),
Is.False);
Assert.That(
ServerFixtureUtils.IsPortUnavailable(
new SocketException((int)SocketError.ConnectionRefused)),
Is.False);
});
}
}
}