From ba8b392a68a81d7e4e7579ef4f3798cc50bb32cd Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 15 Oct 2026 10:59:03 +0200 Subject: [PATCH] Align Machinery transfer regressions with retained-handle cleanup Retain all 101 original transfer scenarios and add eight missing or malformed handle cases for non-Good generation statuses. Require declaration-based Close without discovery, retain primary errors and cleanup ownership checks, and adjust the two affected browse counts. Both target frameworks pass 109 focused transfer cases and 443 full Machinery cases. --- .../MachineryResultTransferTests.Failures.cs | 31 ++++++++++++++----- .../MachineryResultTransferTests.Harness.cs | 12 +++---- .../MachineryResultTransferTests.Paths.cs | 12 +++---- .../MachineryResultTransferTests.cs | 6 +++- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Failures.cs b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Failures.cs index 519acbe436..26b16ff74f 100644 --- a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Failures.cs +++ b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Failures.cs @@ -82,12 +82,19 @@ namespace Opc.Ua.Machinery.Tests public void NonGoodServiceOrMethodStatusIsRejected( string phase, StatusCode status, - bool serviceStatus) + bool serviceStatus, + string handleKind) { CallResponse response = TransferHarness.Response( phase == "generate" ? [Variant.From(TransferHarness.FileId), Variant.From(37u), Variant.From(NodeId.Null)] : phase == "read" ? [Variant.From(ByteString.Empty)] : []); + if (phase == "generate" && handleKind != "valid") + { + response.Results[0].OutputArguments = handleKind == "missing" + ? [Variant.From(TransferHarness.FileId)] + : [Variant.From(TransferHarness.FileId), Variant.From(37ul), Variant.From(NodeId.Null)]; + } if (serviceStatus) { response.ResponseHeader.ServiceResult = status; @@ -117,10 +124,15 @@ namespace Opc.Ua.Machinery.Tests Assert.That(error.StatusCode, Is.EqualTo(status)); Assert.That(destination.Length, Is.Zero); Assert.That(m_transfer.GenerateCount, Is.EqualTo(1)); - Assert.That(m_transfer.CloseCount, Is.EqualTo(phase == "generate" ? 0 : 1)); + bool acquiredHandle = phase != "generate" || handleKind == "valid"; + Assert.That(m_transfer.CloseCount, Is.EqualTo(acquiredHandle ? 1 : 0)); Assert.That(m_transfer.ReadLengths, Has.Count.EqualTo(phase == "generate" ? 0 : 1)); Assert.That(m_transfer.OwnsHandle, Is.EqualTo(phase == "close")); m_transfer.Session.Verify(session => session.Dispose(), Times.Never); + if (acquiredHandle && phase != "close") + { + AssertSuccessfulCleanup(); + } } [Test] @@ -136,10 +148,11 @@ namespace Opc.Ua.Machinery.Tests TransferHarness.MachineId, "selected", destination, 8))!; Assert.That(error.StatusCode, Is.EqualTo(StatusCodes.BadMethodInvalid)); - Assert.That(m_transfer.Calls, Has.Count.EqualTo(1)); + Assert.That(m_transfer.Calls, Has.Count.EqualTo(2)); Assert.That(m_transfer.Paths, Has.Count.EqualTo(2)); - Assert.That(m_transfer.CloseCount, Is.Zero); - Assert.That(m_transfer.OwnsHandle, Is.False); + Assert.That(m_transfer.ReadLengths, Is.Empty); + Assert.That(destination.Length, Is.Zero); + AssertSuccessfulCleanup(); } [TestCase("missing")] @@ -502,14 +515,18 @@ namespace Opc.Ua.Machinery.Tests { foreach (string phase in new[] { "generate", "read", "close" }) { + string[] handleKinds = phase == "generate" ? ["valid", "missing", "type"] : ["valid"]; foreach (StatusCode status in new StatusCode[] { StatusCodes.Uncertain, StatusCodes.BadCommunicationError }) { - yield return new TestCaseData(phase, status, true); - yield return new TestCaseData(phase, status, false); + foreach (string handleKind in handleKinds) + { + yield return new TestCaseData(phase, status, true, handleKind); + yield return new TestCaseData(phase, status, false, handleKind); + } } } } diff --git a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Harness.cs b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Harness.cs index 8b63102b65..25125365a7 100644 --- a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Harness.cs +++ b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Harness.cs @@ -195,13 +195,11 @@ namespace Opc.Ua.Machinery.Tests GenerateCount++; CallResponse response = GenerateResponse ?? Response( [Variant.From(FileId), Variant.From(Handle), Variant.From(NodeId.Null)]); - if (response.ResponseHeader is not null && - StatusCode.IsGood(response.ResponseHeader.ServiceResult) && - response.Results.Count == 1 && - StatusCode.IsGood(response.Results[0].StatusCode) && - response.Results[0].OutputArguments.Count >= 2 && - response.Results[0].OutputArguments[0].TryGetValue(out NodeId node) && node == FileId && - response.Results[0].OutputArguments[1].TryGetValue(out uint returnedHandle)) + if (response.Results.Count > 0 && + response.Results[0] is { } generated && + generated.OutputArguments.Count >= 2 && + generated.OutputArguments[0].TryGetValue(out NodeId node) && node == FileId && + generated.OutputArguments[1].TryGetValue(out uint returnedHandle)) { Handle = returnedHandle; OwnsHandle = true; diff --git a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Paths.cs b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Paths.cs index bcf8a9bf0a..26dd5df88a 100644 --- a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Paths.cs +++ b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.Paths.cs @@ -146,7 +146,7 @@ namespace Opc.Ua.Machinery.Tests } [Test] - public void CloseMethodResolutionFailureStillAttemptsTheDeclarationClose() + public async Task CloseMethodResolutionFailureStillAttemptsTheDeclarationClose() { m_transfer.Translate = path => { @@ -160,11 +160,11 @@ namespace Opc.Ua.Machinery.Tests }; using var destination = new MemoryStream(); - ServiceResultException error = Assert.ThrowsAsync(async () => - await m_transfer.Client.DownloadResultAsync( - TransferHarness.MachineId, "selected", destination, 0))!; + long copied = await m_transfer.Client.DownloadResultAsync( + TransferHarness.MachineId, "selected", destination, 0); - Assert.That(error.StatusCode, Is.EqualTo(StatusCodes.Uncertain)); + Assert.That(copied, Is.Zero); + Assert.That(destination.Length, Is.Zero); Assert.That(m_transfer.Calls[^1].MethodId, Is.EqualTo(MethodIds.FileType_Close)); AssertSuccessfulCleanup(); } @@ -186,7 +186,7 @@ namespace Opc.Ua.Machinery.Tests Assert.That(copied, Is.EqualTo(2)); Assert.That(destination.ToArray(), Is.EqualTo(m_transfer.Payload.ToArray())); Assert.That(m_transfer.AttributeReads, Is.EqualTo(2)); - Assert.That(m_transfer.Paths, Has.Count.EqualTo(5)); + Assert.That(m_transfer.Paths, Has.Count.EqualTo(4)); Assert.That(m_transfer.Paths[2].StartingNode, Is.EqualTo(TransferHarness.StateMachineId)); Assert.That(m_transfer.Paths[2].RelativePath.Elements[0].TargetName.Name, Is.EqualTo("CurrentState")); Assert.That(m_transfer.Paths[2].RelativePath.Elements[1].TargetName.Name, Is.EqualTo("Id")); diff --git a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.cs b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.cs index 32e1c64df3..7cad221e29 100644 --- a/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.cs +++ b/tests/Opc.Ua.Machinery.Tests/MachineryResultTransferTests.cs @@ -193,7 +193,7 @@ namespace Opc.Ua.Machinery.Tests await client.DownloadResultAsync(TransferHarness.MachineId, selected, destination, 2); Assert.That(m_transfer.SelectedResultId, Is.EqualTo(selected)); - Assert.That(m_transfer.Paths, Has.Count.EqualTo(4)); + Assert.That(m_transfer.Paths, Has.Count.EqualTo(3)); BrowsePath path = m_transfer.Paths[0]; Assert.That(path.StartingNode, Is.EqualTo(TransferHarness.MachineId)); Assert.That(path.RelativePath.Elements, Has.Count.EqualTo(2)); @@ -418,8 +418,12 @@ namespace Opc.Ua.Machinery.Tests Assert.That(m_transfer.CloseToken.CanBeCanceled, Is.True); Assert.That(m_transfer.CloseWasCancelledOnEntry, Is.False); Assert.That(m_transfer.Calls[^1].ObjectId, Is.EqualTo(TransferHarness.FileId)); + Assert.That(m_transfer.Calls[^1].MethodId, Is.EqualTo(MethodIds.FileType_Close)); Assert.That(m_transfer.Calls[^1].InputArguments[0].TryGetValue(out uint handle), Is.True); Assert.That(handle, Is.EqualTo(m_transfer.Handle)); + Assert.That(m_transfer.Paths, Has.None.Matches(static path => + path.StartingNode == TransferHarness.FileId && + path.RelativePath.Elements[0].TargetName.Name == "Close")); m_transfer.Session.Verify(session => session.Dispose(), Times.Never); }