From 6e10e45bb9aa2762ce305a4dfc40a253db1f3cb1 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 9 Oct 2026 14:21:32 -0400 Subject: [PATCH] fix(cli): historyupdate exits non-zero unless every item is Good, and never prints a blank status A Good service result with missing or too few per-item results, a missing result entry, or an Uncertain item now fails the command; a status code the SDK has no name for prints as hex. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- .../Commands/HistoryUpdateCommand.cs | 69 +++++++++++++---- .../HistoryUpdateCommandTests.cs | 75 +++++++++++++++++++ 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/Commands/HistoryUpdateCommand.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/Commands/HistoryUpdateCommand.cs index 12669527..38a05bcc 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/Commands/HistoryUpdateCommand.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/Commands/HistoryUpdateCommand.cs @@ -159,32 +159,54 @@ public class HistoryUpdateCommand : CommandBase await console.Output.WriteLineAsync($"HistoryUpdate {Mode.Trim().ToLowerInvariant()} on {NodeId}"); await console.Output.WriteLineAsync($"{"Index",-6} {"Time",-35} {"Status"}"); - var anyBad = false; + // Exit non-zero unless EVERY expected result came back Good: a Bad or Uncertain item, a Bad result or + // service status, or a Good service result that is missing results (the server answered fewer items + // than were sent) all fail the command. + var failed = false; + var expected = ExpectedOperationCount(details); var ops = outcome.OperationResults.Count > 0 ? outcome.OperationResults[0] : []; - if (ops.Count > 0) + if (expected > 0) { - for (var i = 0; i < ops.Count; i++) + for (var i = 0; i < Math.Max(expected, ops.Count); i++) { var time = i < times.Count ? times[i] : "-"; - anyBad |= StatusCode.IsBad(ops[i]); - await console.Output.WriteLineAsync($"{i,-6} {time,-35} {StatusCodes.GetBrowseName(ops[i].Code)}"); + string status; + if (i < ops.Count) + { + failed |= !StatusCode.IsGood(ops[i]); + status = StatusName(ops[i]); + } + else + { + failed = true; + status = "(no result returned)"; + } + await console.Output.WriteLineAsync($"{i,-6} {time,-35} {status}"); } } else if (outcome.ResultStatus.Count > 0) { // delete-raw reports a single result-level status for the whole range. - anyBad |= StatusCode.IsBad(outcome.ResultStatus[0]); await console.Output.WriteLineAsync( - $"{0,-6} {(times.Count > 0 ? times[0] : "-"),-35} {StatusCodes.GetBrowseName(outcome.ResultStatus[0].Code)}"); + $"{0,-6} {(times.Count > 0 ? times[0] : "-"),-35} {StatusName(outcome.ResultStatus[0])}"); } - var overall = StatusCodes.GetBrowseName(outcome.ServiceResult.Code); - await console.Output.WriteLineAsync($"Service result: {overall}"); - anyBad |= StatusCode.IsBad(outcome.ServiceResult); - if (outcome.ResultStatus.Count > 0 && StatusCode.IsBad(outcome.ResultStatus[0])) anyBad = true; + if (outcome.ResultStatus.Count == 0) + { + failed = true; + await console.Output.WriteLineAsync("No history-update result was returned for the node."); + } + else + { + failed |= !StatusCode.IsGood(outcome.ResultStatus[0]); + } - if (anyBad) - throw new CommandException("HistoryUpdate reported one or more Bad results."); + await console.Output.WriteLineAsync($"Service result: {StatusName(outcome.ServiceResult)}"); + failed |= !StatusCode.IsGood(outcome.ServiceResult); + + if (failed) + throw new CommandException( + "HistoryUpdate did not report Good for every item (a Bad or Uncertain result, or a missing result)."); } finally { @@ -196,6 +218,27 @@ public class HistoryUpdateCommand : CommandBase } } + /// How many per-item operation results the server owes for (0 for delete-raw). + private static int ExpectedOperationCount(HistoryUpdateDetails details) => details switch + { + UpdateDataDetails u => u.UpdateValues.Count, + DeleteAtTimeDetails d => d.ReqTimes.Count, + _ => 0 + }; + + /// + /// The SDK's symbolic name for , or its hex value when the SDK has none — a status is + /// never printed blank. + /// + /// The status code to describe. + /// The status name, or 0xXXXXXXXX. + private static string StatusName(StatusCode code) + { + var name = StatusCodes.GetBrowseName(code.Code); + if (string.IsNullOrEmpty(name)) name = StatusCodes.GetBrowseName(code.Code & 0xFFFF0000u); + return string.IsNullOrEmpty(name) ? $"0x{code.Code:X8}" : name; + } + private static List DescribeTimes(HistoryUpdateDetails details) { return details switch diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/HistoryUpdateCommandTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/HistoryUpdateCommandTests.cs index 9f299457..f7137a34 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/HistoryUpdateCommandTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/HistoryUpdateCommandTests.cs @@ -130,4 +130,79 @@ public class HistoryUpdateCommandTests await Should.ThrowAsync(async () => await cmd.ExecuteAsync(console)); TestConsoleHelper.GetOutput(console).ShouldContain("BadEntryExists"); } + + private static readonly string[] TwoTimes = ["2026-01-15T08:00:00Z", "2026-01-15T09:00:00Z"]; + + /// A Good service result that carries NO per-item results must not exit 0. + [Fact] + public async Task Execute_GoodServiceResult_WithMissingItemResults_Throws() + { + var svc = new FakeOpcUaClientService + { + HistoryUpdateResult = new HistoryUpdateOutcome(StatusCodes.Good, [StatusCodes.Good], [new List()]) + }; + using var console = TestConsoleHelper.CreateConsole(); + await Should.ThrowAsync(async () => + await Make(svc, values: ["1", "2"], times: TwoTimes).ExecuteAsync(console)); + TestConsoleHelper.GetOutput(console).ShouldContain("(no result returned)"); + } + + /// Fewer per-item results than items sent must not exit 0, even when every returned one is Good. + [Fact] + public async Task Execute_TooFewItemResults_Throws() + { + var svc = new FakeOpcUaClientService + { + HistoryUpdateResult = new HistoryUpdateOutcome(StatusCodes.Good, [StatusCodes.Good], + [new List { StatusCodes.GoodEntryInserted }]) + }; + using var console = TestConsoleHelper.CreateConsole(); + await Should.ThrowAsync(async () => + await Make(svc, values: ["1", "2"], times: TwoTimes).ExecuteAsync(console)); + } + + /// A Good service result with no history-update result at all must not exit 0. + [Fact] + public async Task Execute_NoResultEntry_Throws() + { + var svc = new FakeOpcUaClientService + { + HistoryUpdateResult = new HistoryUpdateOutcome(StatusCodes.Good, [], []) + }; + using var console = TestConsoleHelper.CreateConsole(); + await Should.ThrowAsync(async () => + await Make(svc, mode: "delete-raw", start: TwoTimes[0], end: TwoTimes[1]).ExecuteAsync(console)); + } + + /// An Uncertain item result is not Good and fails the command. + [Fact] + public async Task Execute_UncertainItem_Throws() + { + var svc = new FakeOpcUaClientService + { + HistoryUpdateResult = new HistoryUpdateOutcome(StatusCodes.Good, [StatusCodes.Good], + [new List { StatusCodes.UncertainDataSubNormal }]) + }; + using var console = TestConsoleHelper.CreateConsole(); + await Should.ThrowAsync(async () => + await Make(svc, values: ["1"], times: ["2026-01-15T08:00:00Z"]).ExecuteAsync(console)); + TestConsoleHelper.GetOutput(console).ShouldContain("UncertainDataSubNormal"); + } + + /// A status the SDK has no name for prints as hex, never blank. + [Fact] + public async Task Execute_UnnamedStatus_PrintsHex() + { + const uint unnamed = 0x8FFF0000u; + StatusCodes.GetBrowseName(unnamed).ShouldBeNullOrEmpty(); // precondition: genuinely unnamed + var svc = new FakeOpcUaClientService + { + HistoryUpdateResult = new HistoryUpdateOutcome(StatusCodes.Good, [StatusCodes.Good], + [new List { new(unnamed) }]) + }; + using var console = TestConsoleHelper.CreateConsole(); + await Should.ThrowAsync(async () => + await Make(svc, values: ["1"], times: ["2026-01-15T08:00:00Z"]).ExecuteAsync(console)); + TestConsoleHelper.GetOutput(console).ShouldContain("0x8FFF0000"); + } }