diff --git a/Directory.Packages.props b/Directory.Packages.props index a30e3e2d..87785628 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -77,6 +77,10 @@ + + + all @@ -35,6 +37,10 @@ + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs index c048431c..3ec18e17 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/StatusCodeMapTests.cs @@ -19,12 +19,12 @@ public sealed class StatusCodeMapTests /// The expected OPC UA status code. [Theory] [InlineData((byte)192, 0x00000000u)] // Good - [InlineData((byte)216, 0x00D80000u)] // Good_LocalOverride + [InlineData((byte)216, 0x00960000u)] // Good_LocalOverride [InlineData((byte)64, 0x40000000u)] // Uncertain - [InlineData((byte)68, 0x40A40000u)] // Uncertain_LastUsableValue - [InlineData((byte)80, 0x408D0000u)] // Uncertain_SensorNotAccurate - [InlineData((byte)84, 0x408E0000u)] // Uncertain_EngineeringUnitsExceeded - [InlineData((byte)88, 0x408F0000u)] // Uncertain_SubNormal + [InlineData((byte)68, 0x40900000u)] // Uncertain_LastUsableValue + [InlineData((byte)80, 0x40930000u)] // Uncertain_SensorNotAccurate + [InlineData((byte)84, 0x40940000u)] // Uncertain_EngineeringUnitsExceeded + [InlineData((byte)88, 0x40950000u)] // Uncertain_SubNormal [InlineData((byte)0, 0x80000000u)] // Bad [InlineData((byte)4, 0x80890000u)] // Bad_ConfigurationError [InlineData((byte)8, 0x808A0000u)] // Bad_NotConnected @@ -132,9 +132,9 @@ public sealed class StatusCodeMapTests /// The expected OPC DA category byte. [Theory] [InlineData(0x00000000u, (byte)192)] // Good - [InlineData(0x00D80000u, (byte)192)] // GoodLocalOverride — still Good category + [InlineData(0x00960000u, (byte)192)] // GoodLocalOverride — still Good category [InlineData(0x40000000u, (byte)64)] // Uncertain - [InlineData(0x408F0000u, (byte)64)] // UncertainSubNormal — still Uncertain category + [InlineData(0x40950000u, (byte)64)] // UncertainSubNormal — still Uncertain category [InlineData(0x80000000u, (byte)0)] // Bad [InlineData(0x808A0000u, (byte)0)] // BadNotConnected — still Bad category [InlineData(0x80020000u, (byte)0)] // BadInternalError — still Bad category diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/GatewayQualityMapperTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/GatewayQualityMapperTests.cs index 15c7fced..c2cba1f6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/GatewayQualityMapperTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/GatewayQualityMapperTests.cs @@ -7,7 +7,7 @@ public sealed class GatewayQualityMapperTests { [Theory] [InlineData(192, 0x00000000u)] // Good - [InlineData(216, 0x00D80000u)] // Good_LocalOverride + [InlineData(216, 0x00960000u)] // Good_LocalOverride [InlineData(64, 0x40000000u)] // Uncertain [InlineData(0, 0x80000000u)] // Bad [InlineData(8, 0x808A0000u)] // Bad_NotConnected diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/SampleMapperTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/SampleMapperTests.cs index 969b9764..29092ae4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/SampleMapperTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Mapping/SampleMapperTests.cs @@ -37,7 +37,7 @@ public sealed class SampleMapperTests { var a = new HistorianAggregateSample { Tag = "T", /* Value unset, no Good quality */ EndTime = Ts(2026, 1, 1, 0, 0, 0) }; var snap = SampleMapper.ToAggregateSnapshot(a); - Assert.Equal(0x800E0000u, snap.StatusCode); // BadNoData + Assert.Equal(0x809B0000u, snap.StatusCode); // BadNoData Assert.Null(snap.Value); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs index c3fe2e99..ebf2d212 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/SqlServerReadTests.cs @@ -475,6 +475,99 @@ public sealed class SqlServerReadTests // ---- helpers ---- + // ---- design §8.1 catalog gate (Gitea #496), against a real INFORMATION_SCHEMA ---- + + /// + /// The gate's live proof: an authored column that does not exist is refused by the allow-list at + /// Initialize, so it never reaches a query — and its neighbour on the same table keeps reading. + /// + /// + /// Only a real SQL Server exercises the real SELECT SCHEMA_NAME() + INFORMATION_SCHEMA + /// path the loader depends on; the SQLite-backed suites prove the logic but not that T-SQL's catalog + /// answers the shape the loader expects. + /// + [Fact] + public async Task CatalogGate_realSqlServer_rejectsAnUnknownColumn_andSparesItsNeighbour() + { + SkipUnlessLive(); + + var bogus = Tag("Sql/Line1/Bogus", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable), + ["keyColumn"] = SqlPollServerFixture.KeyColumn, + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = "no_such_column", + ["timestampColumn"] = SqlPollServerFixture.TimestampColumn, + }); + + await using var driver = await StartAsync( + KeyValue("Sql/Line1/Speed", SqlPollServerFixture.PresentKey), bogus); + + // An authoring typo is not a database fault: the driver stays Healthy. + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + + var snapshots = await driver.ReadAsync( + ["Sql/Line1/Speed", "Sql/Line1/Bogus"], TestContext.Current.CancellationToken); + + SqlStatusCodes.IsGood(snapshots[0].StatusCode).ShouldBeTrue(); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + /// + /// T-SQL object names are case-insensitive under the default collation, so a case-variant tag has + /// always been valid and must keep working — the gate substitutes the catalog's spelling rather than + /// rejecting the operator's. + /// + [Fact] + public async Task CatalogGate_realSqlServer_acceptsACaseVariantIdentifierAndStillReads() + { + SkipUnlessLive(); + + var upper = Tag("Sql/Line1/Speed", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable).ToUpperInvariant(), + ["keyColumn"] = SqlPollServerFixture.KeyColumn.ToUpperInvariant(), + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = SqlPollServerFixture.ValueColumn.ToUpperInvariant(), + ["timestampColumn"] = SqlPollServerFixture.TimestampColumn.ToUpperInvariant(), + }); + + await using var driver = await StartAsync(upper); + + var snapshot = (await driver.ReadAsync(["Sql/Line1/Speed"], TestContext.Current.CancellationToken)) + .ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + } + + /// + /// A cross-database name cannot be validated from this connection's catalog, so it is refused rather + /// than quoted into a query — the documented v1 limitation, asserted so it stays deliberate. + /// + [Fact] + public async Task CatalogGate_realSqlServer_rejectsAThreePartName() + { + SkipUnlessLive(); + + var threePart = Tag("Sql/Line1/Remote", new JsonObject + { + ["driver"] = "Sql", + ["model"] = "KeyValue", + ["table"] = $"otherdb.{SqlPollServerFixture.Qualified(SqlPollServerFixture.KeyValueTable)}", + ["keyColumn"] = SqlPollServerFixture.KeyColumn, + ["keyValue"] = SqlPollServerFixture.PresentKey, + ["valueColumn"] = SqlPollServerFixture.ValueColumn, + }); + + await using var driver = await StartAsync(threePart); + + (await driver.ReadAsync(["Sql/Line1/Remote"], TestContext.Current.CancellationToken)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + /// Skips the calling test when the live-server gate is not configured or not reachable. private void SkipUnlessLive() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs new file mode 100644 index 00000000..9236f3e3 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateDriverTests.cs @@ -0,0 +1,354 @@ +using System.Globalization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// The design §8.1 catalog gate end-to-end through , against the real SQLite +/// catalog the poll fixture creates — real ListSchemas/ListTables/ListColumns +/// round-trips, not a hand-built . +/// The two facts that matter most here are the ones a pure unit test cannot show: that a +/// rejected tag still has a node and reads BadNodeIdUnknown (rather than silently +/// vanishing from the address space), and that a catalog the driver cannot read faults Initialize instead +/// of rejecting every tag. +/// +public sealed class SqlCatalogGateDriverTests +{ + private const string DriverInstanceId = "sql-gate"; + private const string ConfigJson = """{"provider":"SqlServer"}"""; + + [Fact] + public async Task A_tag_naming_a_real_table_and_columns_polls_normally() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + var snapshot = (await driver.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + snapshot.Value.ShouldBe(SqlitePollFixture.PresentValue); + } + + /// + /// §8.1's specified outcome, in full: the tag is refused by the allow-list, so it never reaches a + /// query — but its node still exists and reads BadNodeIdUnknown. Dropping the node instead would + /// turn a diagnosable Bad quality into a missing address-space entry. + /// + [Fact] + public async Task A_tag_naming_an_unknown_column_keeps_its_node_and_reads_BadNodeIdUnknown() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, + KvEntry("Speed", SqlitePollFixture.PresentKey), + KvEntry("Bogus", SqlitePollFixture.PresentKey, valueColumn: "no_such_column")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + // The driver is healthy: an authoring typo is not a database fault. + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + + // The node is still materialized... + var capture = new CapturingBuilder(); + await ((ITagDiscovery)driver).DiscoverAsync(capture, CancellationToken.None); + capture.Variables.Select(v => v.Info.FullName).ShouldBe(["Speed", "Bogus"], ignoreOrder: true); + + // ...and it is the rejected tag — and only it — that reads BadNodeIdUnknown. + var snapshots = await driver.ReadAsync(["Speed", "Bogus"], CancellationToken.None); + SqlStatusCodes.IsGood(snapshots[0].StatusCode).ShouldBeTrue(); + snapshots[1].StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + [Fact] + public async Task A_tag_naming_an_unknown_table_reads_BadNodeIdUnknown() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, KvEntry("Bogus", SqlitePollFixture.PresentKey, table: "NoSuchTable")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + (await driver.ReadAsync(["Bogus"], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + /// + /// The injection shape #496 exists to close. Before the gate, this name was bracket-quoted into a real + /// query that failed only once the connection was open; now it never reaches a query at all. + /// + [Fact] + public async Task A_hostile_table_name_never_reaches_a_query() + { + using var fixture = new SqlitePollFixture(); + var logger = new CapturingLogger(); + await using var driver = NewDriver( + fixture, logger, + KvEntry("Evil", SqlitePollFixture.PresentKey, table: "TagValues\"; DROP TABLE TagValues--")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + (await driver.ReadAsync(["Evil"], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + + // The fixture's table is untouched — proven by a second tag still reading through it. + await using var honest = NewDriver(fixture, KvEntry("Speed", SqlitePollFixture.PresentKey)); + await honest.InitializeAsync(ConfigJson, CancellationToken.None); + var snapshot = (await honest.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + + logger.Entries.ShouldContain(e => + e.Level == LogLevel.Warning && e.Message.Contains("rejected by the catalog gate", StringComparison.Ordinal)); + } + + /// + /// A node that goes Bad with nothing in the log is unsupportable, so every drop names the tag, the + /// field and the reason. + /// + [Fact] + public async Task Every_rejection_is_logged_with_the_tag_the_field_and_the_reason() + { + using var fixture = new SqlitePollFixture(); + var logger = new CapturingLogger(); + await using var driver = NewDriver( + fixture, logger, KvEntry("Bogus", SqlitePollFixture.PresentKey, valueColumn: "num_valeu")); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var warning = logger.Entries + .Where(e => e.Level == LogLevel.Warning) + .Select(e => e.Message) + .ShouldHaveSingleItem(); + warning.ShouldContain("Bogus"); + warning.ShouldContain(nameof(SqlTagDefinition.ValueColumn)); + warning.ShouldContain("num_valeu"); + } + + /// + /// Case-insensitive authoring has always worked on SQL Server's default collation, so the gate must + /// accept it — and it substitutes the catalog's spelling, which is what makes the emitted SQL carry + /// catalog strings rather than operator input. + /// + [Fact] + public async Task A_case_variant_identifier_is_accepted_and_polls() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver( + fixture, + KvEntry( + "Speed", SqlitePollFixture.PresentKey, + table: SqlitePollFixture.KeyValueTable.ToUpperInvariant(), + valueColumn: SqlitePollFixture.ValueColumn.ToUpperInvariant())); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + var snapshot = (await driver.ReadAsync(["Speed"], CancellationToken.None)).ShouldHaveSingleItem(); + SqlStatusCodes.IsGood(snapshot.StatusCode).ShouldBeTrue(); + snapshot.Value.ShouldBe(SqlitePollFixture.PresentValue); + } + + /// + /// A driver with nothing authored has nothing to validate; issuing catalog queries to prove that would + /// be a round-trip that can only fail. + /// + [Fact] + public async Task A_driver_with_no_authored_tags_initializes_without_touching_the_catalog() + { + using var fixture = new SqlitePollFixture(); + await using var driver = NewDriver(fixture); + + await driver.InitializeAsync(ConfigJson, CancellationToken.None); + + driver.GetHealth().State.ShouldBe(DriverState.Healthy); + } + + /// + /// The fail-closed rule. A catalog that cannot be read is the ABSENCE of evidence about the + /// tags, not evidence against them. Rejecting every tag would serve a confidently-empty address space + /// and send the operator hunting typos that do not exist; faulting Initialize instead lands + /// DriverInstanceActor in Reconnecting with its retry timer running. + /// + [Fact] + public async Task A_catalog_that_cannot_be_read_faults_Initialize_rather_than_rejecting_every_tag() + { + using var fixture = new SqlitePollFixture(); + await using var driver = new SqlDriver( + new SqlDriverOptions + { + RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)], + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new CatalogSqlOverride("SELECT this is not valid sql"), + fixture.ConnectionString, + factory: fixture.Factory, + logger: NullLogger.Instance); + + var thrown = await Should.ThrowAsync( + async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None)); + + // The operator surface names the stage that actually failed — "reached it but could not read the + // catalog" and "could not reach it" send an operator to different systems. + thrown.Message.ShouldContain("catalog"); + driver.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + /// + /// Zero visible schemas is a grant problem, not an empty database, so it must fault rather than reject + /// every tag for an authoring fault the operator does not have. + /// + [Fact] + public async Task A_catalog_reporting_no_schemas_at_all_faults_Initialize() + { + using var fixture = new SqlitePollFixture(); + await using var driver = new SqlDriver( + new SqlDriverOptions + { + RawTags = [KvEntry("Speed", SqlitePollFixture.PresentKey)], + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new CatalogSqlOverride("SELECT 'x' AS TABLE_SCHEMA WHERE 1 = 0"), + fixture.ConnectionString, + factory: fixture.Factory, + logger: NullLogger.Instance); + + await Should.ThrowAsync( + async () => await driver.InitializeAsync(ConfigJson, CancellationToken.None)); + + driver.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + // ---- helpers ---- + + /// + /// Delegates every member to except , so the + /// catalog-load failure modes can be driven against an otherwise-real dialect. + /// + /// + /// A decorator rather than a subclass: is sealed, and even if it were not, + /// new-hiding a property would leave interface dispatch calling the base — the substitution + /// would silently not happen and both tests below would pass for the wrong reason. + /// + private sealed class CatalogSqlOverride(string listSchemasSql) : ISqlDialect + { + private static readonly SqliteDialect Inner = new(); + + public SqlProvider Provider => Inner.Provider; + + public System.Data.Common.DbProviderFactory Factory => Inner.Factory; + + public string LivenessSql => Inner.LivenessSql; + + public string SingleRowLimitPrefix => Inner.SingleRowLimitPrefix; + + public string SingleRowLimitSuffix => Inner.SingleRowLimitSuffix; + + public string ListSchemasSql { get; } = listSchemasSql; + + public string DefaultSchemaSql => Inner.DefaultSchemaSql; + + public string ListTablesSql => Inner.ListTablesSql; + + public string ListColumnsSql => Inner.ListColumnsSql; + + public string QuoteIdentifier(string ident) => Inner.QuoteIdentifier(ident); + + public DriverDataType MapColumnType(string sqlDataType) => Inner.MapColumnType(sqlDataType); + } + + private static SqlDriver NewDriver(SqlitePollFixture fixture, params RawTagEntry[] rawTags) + => NewDriver(fixture, new CapturingLogger(), rawTags); + + private static SqlDriver NewDriver( + SqlitePollFixture fixture, CapturingLogger logger, params RawTagEntry[] rawTags) + => new( + new SqlDriverOptions + { + RawTags = rawTags, + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + }, + DriverInstanceId, + new SqliteDialect(), + fixture.ConnectionString, + factory: fixture.Factory, + logger: logger); + + /// One authored raw tag, with the table and value column overridable so the gate can be exercised. + private static RawTagEntry KvEntry( + string rawPath, + string keyValue, + string? table = null, + string? valueColumn = null) + => new(rawPath, string.Create(CultureInfo.InvariantCulture, $$""" + { + "driver": "Sql", + "model": "KeyValue", + "table": "{{(table ?? SqlitePollFixture.KeyValueTable).Replace("\"", "\\\"", StringComparison.Ordinal)}}", + "keyColumn": "{{SqlitePollFixture.KeyColumn}}", + "keyValue": "{{keyValue}}", + "valueColumn": "{{valueColumn ?? SqlitePollFixture.ValueColumn}}", + "timestampColumn": "{{SqlitePollFixture.TimestampColumn}}" + } + """), WriteIdempotent: false); + + /// Records everything the driver streams into the address space. + private sealed class CapturingBuilder : IAddressSpaceBuilder + { + /// The variables registered, in order. + public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = []; + + public IAddressSpaceBuilder Folder(string browseName, string displayName) => this; + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo attributeInfo) + { + Variables.Add((browseName, attributeInfo)); + return new Handle(attributeInfo.FullName); + } + + public void AddProperty(string browseName, DriverDataType dataType, object? value) { } + + private sealed class Handle(string fullReference) : IVariableHandle + { + public string FullReference => fullReference; + + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new Sink(); + + private sealed class Sink : IAlarmConditionSink + { + public void OnTransition(AlarmEventArgs args) { } + } + } + } + + /// Records every log record, level + rendered message. + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Entries.Add((logLevel, formatter(state, exception))); + + private sealed class NullScope : IDisposable + { + public static NullScope Instance { get; } = new(); + + public void Dispose() { } + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs new file mode 100644 index 00000000..51d9acbf --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlCatalogGateTests.cs @@ -0,0 +1,316 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// The design §8.1 catalog gate (Gitea #496), tested as a pure function over a hand-built +/// . The end-to-end behaviour against a real catalog — and the proof that a +/// rejected tag keeps its node and publishes BadNodeIdUnknown — lives in +/// . +/// +public sealed class SqlCatalogGateTests +{ + private static readonly SqliteDialect Dialect = new(); + + /// A catalog with one schema, two relations, and deliberately mixed-case column spellings. + private static SqlCatalog Catalog(string defaultSchema = "dbo") => new( + defaultSchema, + ["dbo", "mes"], + new Dictionary>(StringComparer.Ordinal) + { + ["dbo"] = ["TagValues", "LatestStatus"], + ["mes"] = ["Orders"], + }, + new Dictionary>(StringComparer.Ordinal) + { + ["dbo.TagValues"] = ["tag_name", "num_value", "sample_ts"], + ["dbo.LatestStatus"] = ["station_id", "oven_temp", "pressure", "sample_ts"], + ["mes.Orders"] = ["order_id", "qty"], + }); + + private static SqlTagDefinition KeyValueTag( + string table = "dbo.TagValues", + string keyColumn = "tag_name", + string valueColumn = "num_value", + string? timestampColumn = "sample_ts") => + new("plant/sql/Speed", SqlTagModel.KeyValue, table, + KeyColumn: keyColumn, KeyValue: "Line1.Speed", + ValueColumn: valueColumn, TimestampColumn: timestampColumn); + + private static SqlCatalogGateResult Apply(params SqlTagDefinition[] tags) => + SqlCatalogGate.Apply(tags, Catalog(), Dialect); + + [Fact] + public void A_fully_resolvable_tag_is_accepted() + { + var result = Apply(KeyValueTag()); + + result.Rejected.ShouldBeEmpty(); + result.Accepted.Count.ShouldBe(1); + } + + /// + /// The heart of §8.1: what reaches the planner must be a string the catalog gave us, not the string an + /// operator typed. Authoring every identifier in the wrong case proves the substitution actually + /// happens rather than the input merely being waved through. + /// + [Fact] + public void Accepted_identifiers_are_rewritten_to_the_catalogs_own_spelling() + { + var authored = KeyValueTag( + table: "DBO.TAGVALUES", keyColumn: "TAG_NAME", valueColumn: "Num_Value", timestampColumn: "SAMPLE_TS"); + + var accepted = Apply(authored).Accepted.ShouldHaveSingleItem(); + + accepted.Table.ShouldBe("dbo.TagValues"); + accepted.KeyColumn.ShouldBe("tag_name"); + accepted.ValueColumn.ShouldBe("num_value"); + accepted.TimestampColumn.ShouldBe("sample_ts"); + // Identity and bound VALUES are untouched — the gate rewrites identifiers only. + accepted.Name.ShouldBe(authored.Name); + accepted.KeyValue.ShouldBe(authored.KeyValue); + } + + [Fact] + public void An_unqualified_table_resolves_in_the_default_schema() + { + var accepted = Apply(KeyValueTag(table: "TagValues")).Accepted.ShouldHaveSingleItem(); + + accepted.Table.ShouldBe("dbo.TagValues"); + } + + /// + /// Guessing dbo would be a silent lie on an estate that maps service accounts to their own + /// default schema, so the gate must resolve an unqualified name in whatever schema the server reports. + /// + [Fact] + public void An_unqualified_table_follows_a_non_dbo_default_schema() + { + var catalog = Catalog(defaultSchema: "mes"); + + var result = SqlCatalogGate.Apply([KeyValueTag(table: "Orders", keyColumn: "order_id", valueColumn: "qty", timestampColumn: null)], catalog, Dialect); + + result.Accepted.ShouldHaveSingleItem().Table.ShouldBe("mes.Orders"); + } + + [Fact] + public void An_unknown_table_rejects_the_tag() + { + var rejection = Apply(KeyValueTag(table: "dbo.NoSuchTable")).Rejected.ShouldHaveSingleItem(); + + rejection.RawPath.ShouldBe("plant/sql/Speed"); + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("NoSuchTable"); + } + + [Fact] + public void An_unknown_schema_rejects_the_tag() + { + var rejection = Apply(KeyValueTag(table: "nope.TagValues")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("nope"); + } + + [Fact] + public void An_unknown_column_rejects_the_tag_and_names_the_field() + { + var rejection = Apply(KeyValueTag(valueColumn: "no_such_column")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + rejection.Reason.ShouldContain("no_such_column"); + rejection.Reason.ShouldContain("dbo.TagValues"); + } + + /// + /// A table in the right catalog but the wrong schema must not resolve — otherwise the gate would + /// allow-list a column set from a relation the query will never read. + /// + [Fact] + public void A_table_from_another_schema_does_not_resolve_unqualified() + { + var rejection = Apply(KeyValueTag(table: "Orders", keyColumn: "order_id", valueColumn: "qty", timestampColumn: null)) + .Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + } + + /// + /// A cross-database or linked-server name addresses a catalog this connection cannot enumerate, so it + /// cannot be allow-listed. Rejecting is the fail-closed answer, and the message says what to do instead. + /// + [Fact] + public void A_three_part_name_is_rejected_with_an_actionable_message() + { + var rejection = Apply(KeyValueTag(table: "otherdb.dbo.TagValues")).Rejected.ShouldHaveSingleItem(); + + rejection.Field.ShouldBe(nameof(SqlTagDefinition.Table)); + rejection.Reason.ShouldContain("3-part"); + rejection.Reason.ShouldContain("view"); + } + + /// + /// The injection shape the whole gate exists for: a hostile identifier must be refused by the + /// allow-list, not merely quoted into a query against a nonexistent object. + /// + [Theory] + [InlineData("TagValues]; DROP TABLE TagValues--")] + [InlineData("'; DROP TABLE TagValues--")] + [InlineData("TagValues WHERE 1=1 OR 1=1")] + public void A_hostile_table_name_is_rejected_by_the_allow_list(string table) + { + Apply(KeyValueTag(table: table)).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.Table)); + } + + [Theory] + [InlineData("num_value]; DROP TABLE TagValues--")] + [InlineData("(SELECT password FROM users)")] + public void A_hostile_column_name_is_rejected_by_the_allow_list(string column) + { + Apply(KeyValueTag(valueColumn: column)).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + } + + /// + /// A name carrying a control or Unicode format character cannot be safely rendered into a log line + /// (the Trojan-Source class, CVE-2021-42574), so the gate rejects it without echoing it — the + /// charset check runs before the catalog lookup precisely so no such string can reach a message. + /// + /// + /// Driven against , not the test-only : the + /// charset rules belong to the dialect (the gate delegates to + /// rather than duplicating them), and SQLite's rules + /// deliberately stop at control characters. Asserting SQL Server's rules means using SQL Server's + /// dialect — the first draft of this test asserted them against SQLite's and failed for that reason. + /// + [Theory] + [InlineData("num\u0000value")] // Cc — embedded NUL + [InlineData("num\u0009value")] // Cc — tab; truncates or corrupts a logged statement + [InlineData("num\u202Evalue")] // Cf — right-to-left override + [InlineData("num\u200Bvalue")] // Cf — zero-width space + public void An_unrenderable_identifier_is_rejected_without_being_echoed(string column) + { + var result = SqlCatalogGate.Apply( + [KeyValueTag(valueColumn: column)], Catalog(), new SqlServerDialect()); + + var rejection = result.Rejected.ShouldHaveSingleItem(); + rejection.Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + rejection.Reason.ShouldContain("withheld"); + rejection.Reason.ShouldNotContain(column); + } + + /// + /// An over-long name cannot name a real object and is likewise withheld, rather than pasting an + /// unbounded slab of operator input into a log line. + /// + [Fact] + public void An_over_long_identifier_is_rejected_without_being_echoed() + { + var column = new string('x', SqlServerDialect.MaxIdentifierLength + 1); + + var result = SqlCatalogGate.Apply( + [KeyValueTag(valueColumn: column)], Catalog(), new SqlServerDialect()); + + result.Rejected.ShouldHaveSingleItem().Reason.ShouldContain("withheld"); + } + + /// + /// The complement, and the reason the charset check is not simply "withhold everything": a name that + /// IS safely renderable gets echoed, because an operator hunting a typo has to see what they wrote. + /// + [Fact] + public void A_renderable_but_unknown_identifier_IS_echoed_so_the_typo_is_findable() + { + var rejection = Apply(KeyValueTag(valueColumn: "num_valeu")).Rejected.ShouldHaveSingleItem(); + + rejection.Reason.ShouldContain("num_valeu"); + rejection.Reason.ShouldNotContain("withheld"); + } + + /// + /// On a case-sensitive collation a relation may legitimately carry both Value and value. + /// Picking one would publish a different column's data under the operator's node, so the only safe + /// answer is to refuse — but an EXACT match must still win, or a valid config would break. + /// + [Fact] + public void An_ambiguous_case_insensitive_column_match_is_rejected_but_an_exact_match_still_wins() + { + var catalog = new SqlCatalog( + "dbo", + ["dbo"], + new Dictionary>(StringComparer.Ordinal) { ["dbo"] = ["T"] }, + new Dictionary>(StringComparer.Ordinal) + { + ["dbo.T"] = ["k", "Value", "value"], + }); + + var ambiguous = new SqlTagDefinition( + "p/Ambiguous", SqlTagModel.KeyValue, "dbo.T", + KeyColumn: "k", KeyValue: "x", ValueColumn: "VALUE"); + SqlCatalogGate.Apply([ambiguous], catalog, Dialect).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ValueColumn)); + + var exact = ambiguous with { Name = "p/Exact", ValueColumn = "value" }; + SqlCatalogGate.Apply([exact], catalog, Dialect).Accepted.ShouldHaveSingleItem() + .ValueColumn.ShouldBe("value"); + } + + /// + /// One operator typo must not stop the other tags on the same database — the same rule the tag-table + /// build already follows for a malformed blob. + /// + [Fact] + public void A_rejected_tag_does_not_take_its_healthy_neighbours_with_it() + { + var good = KeyValueTag() with { Name = "plant/sql/Good" }; + var bad = KeyValueTag(valueColumn: "typo") with { Name = "plant/sql/Bad" }; + + var result = Apply(good, bad); + + result.Accepted.ShouldHaveSingleItem().Name.ShouldBe("plant/sql/Good"); + result.Rejected.ShouldHaveSingleItem().RawPath.ShouldBe("plant/sql/Bad"); + } + + /// Every identifier-bearing field is checked, not just the two the key-value model happens to use. + [Fact] + public void The_wide_row_models_identifier_fields_are_validated_too() + { + var selectorTypo = new SqlTagDefinition( + "p/Oven", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "oven_temp", RowSelectorColumn: "no_such_selector", RowSelectorValue: "7"); + Apply(selectorTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.RowSelectorColumn)); + + var orderTypo = new SqlTagDefinition( + "p/Newest", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "oven_temp", RowSelectorTopByTimestamp: "no_such_ts"); + Apply(orderTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.RowSelectorTopByTimestamp)); + + var columnTypo = new SqlTagDefinition( + "p/Bad", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "no_such_column", RowSelectorColumn: "station_id", RowSelectorValue: "7"); + Apply(columnTypo).Rejected.ShouldHaveSingleItem() + .Field.ShouldBe(nameof(SqlTagDefinition.ColumnName)); + + var ok = new SqlTagDefinition( + "p/Ok", SqlTagModel.WideRow, "dbo.LatestStatus", + ColumnName: "OVEN_TEMP", RowSelectorColumn: "STATION_ID", RowSelectorValue: "7"); + var accepted = Apply(ok).Accepted.ShouldHaveSingleItem(); + accepted.ColumnName.ShouldBe("oven_temp"); + accepted.RowSelectorColumn.ShouldBe("station_id"); + accepted.RowSelectorValue.ShouldBe("7"); // a bound value, never canonicalized + } + + /// An absent optional identifier is not a rejection — only a present, unresolvable one is. + [Fact] + public void An_absent_optional_timestamp_column_is_left_alone() + { + var accepted = Apply(KeyValueTag(timestampColumn: null)).Accepted.ShouldHaveSingleItem(); + + accepted.TimestampColumn.ShouldBeNull(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs index 784b3dd7..a49be22d 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlInjectionRegressionTests.cs @@ -12,16 +12,19 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; /// identifier — which cannot be parameterized in SQL — is dialect-quoted so a hostile one becomes /// an inert reference to a nonexistent object rather than executable text. If these pass, a malicious tag /// cannot alter the database; the seed table is intact after every hostile poll. -/// Scope note — what this suite does NOT assume. Design §8.1 also specifies a catalog gate: -/// validate every authored table/column against INFORMATION_SCHEMA before quoting it, so an unknown -/// identifier rejects the tag. No such gate exists in the driver yet (see the "Not yet -/// implemented" note on ), and no task in this workstream builds one. So a hostile -/// identifier here is not rejected as BadNodeIdUnknown — it is bracket-/double-quoted into a -/// single nonexistent identifier, the query then fails after the connection opened, and the tag Bad-codes -/// as a query failure (). This suite proves the payload -/// is inert — it does not execute and the table survives — which is the guarantee the code -/// actually makes. The catalog gate is a separate follow-up; a test that pretended it existed would be -/// asserting fiction. +/// Scope note — this suite deliberately tests the layer BELOW the catalog gate. Design +/// §8.1's catalog gate now exists (Gitea #496): validates every authored +/// table/column against the live catalog at Initialize, so in the assembled driver a hostile identifier +/// is rejected up front and its tag reads — proven by +/// SqlCatalogGateDriverTests. +/// The tests here construct a directly, with hand-built +/// definitions that never pass through the gate, and that is the point: defence in depth is only worth +/// the name if each layer holds on its own. These assertions pin the quoting backstop — that +/// even with the allow-list bypassed entirely, a hostile identifier becomes an inert reference to a +/// nonexistent object, the query fails after the connection opened, the tag Bad-codes as a query failure +/// (), and the seed table survives. Rewriting them to +/// expect BadNodeIdUnknown would delete the only coverage the backstop has and leave the gate as a +/// single point of failure. /// public sealed class SqlInjectionRegressionTests { @@ -68,9 +71,11 @@ public sealed class SqlInjectionRegressionTests using var fixture = new SqlitePollFixture(); var seedRows = await RowCountAsync(fixture, SqlitePollFixture.KeyValueTable); - // A table name carrying a statement terminator + DROP. There is NO catalog gate, so this is not - // rejected up front — it is quoted into one nonexistent identifier. The query fails (no such table), - // the connection having opened, so the tag Bad-codes as a query failure. The DROP never runs. + // A table name carrying a statement terminator + DROP. The reader is driven directly, so the + // catalog gate never sees it and the quoting backstop is on its own: the name is quoted into one + // nonexistent identifier, the query fails (no such table) with the connection already open, so the + // tag Bad-codes as a query failure. The DROP never runs. In the assembled driver the gate rejects + // this first and the tag reads BadNodeIdUnknown instead — see SqlCatalogGateDriverTests. var reader = NewReader(fixture, new SqlTagDefinition( Name: "Evil", @@ -83,7 +88,7 @@ public sealed class SqlInjectionRegressionTests var snapshots = await reader.ReadAsync(["Evil"], CancellationToken.None); - // Bad-coded — but as a QUERY failure, not BadNodeIdUnknown (there is no identifier-catalog gate). + // Bad-coded as a QUERY failure, not BadNodeIdUnknown — this reader was built without the gate. SqlStatusCodes.IsBad(snapshots[0].StatusCode).ShouldBeTrue(); snapshots[0].StatusCode.ShouldBe(SqlStatusCodes.BadCommunicationError); // The load-bearing assertion: the payload did NOT execute. The table and its rows are untouched. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs index 8a7a7527..a3ec1ca6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqliteDialect.cs @@ -64,6 +64,12 @@ public sealed class SqliteDialect : ISqlDialect /// public string ListSchemasSql => $"SELECT '{MainSchema}' AS TABLE_SCHEMA"; + /// + /// SQLite has no per-principal default schema, so an unqualified name always resolves in + /// — the same single schema reports. + /// + public string DefaultSchemaSql => $"SELECT '{MainSchema}'"; + /// /// Tables + views from sqlite_schema (the modern name for sqlite_master), with the /// internal sqlite_* objects filtered out and the type folded onto the diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorWriteRoutingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorWriteRoutingTests.cs index e5ac2a8c..baf11d1a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorWriteRoutingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorWriteRoutingTests.cs @@ -55,11 +55,9 @@ public sealed class DriverHostActorWriteRoutingTests : RuntimeActorTestBase var actor = SpawnHostAndApply(db, deploymentId, recorder); - var asker = CreateTestProbe(); // The node manager passes the FULL ns-qualified id; the host must normalise + resolve it. - actor.Tell(new DriverHostActor.RouteNodeWrite($"ns=3;s={UnsNodeId}", 123.0, AddressSpaceRealm.Uns), asker.Ref); + RouteWriteUntilAccepted(actor, $"ns=3;s={UnsNodeId}", 123.0, AddressSpaceRealm.Uns); - asker.ExpectMsg(Timeout).Success.ShouldBeTrue(); AwaitAssert(() => { recorder.Writes.Count.ShouldBe(1); @@ -79,10 +77,8 @@ public sealed class DriverHostActorWriteRoutingTests : RuntimeActorTestBase var actor = SpawnHostAndApply(db, deploymentId, recorder); - var asker = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite($"ns=2;s={RawPath}", 456.0, AddressSpaceRealm.Raw), asker.Ref); + RouteWriteUntilAccepted(actor, $"ns=2;s={RawPath}", 456.0, AddressSpaceRealm.Raw); - asker.ExpectMsg(Timeout).Success.ShouldBeTrue(); AwaitAssert(() => { recorder.Writes.Count.ShouldBe(1); @@ -91,6 +87,48 @@ public sealed class DriverHostActorWriteRoutingTests : RuntimeActorTestBase }, duration: Timeout); } + /// + /// Routes a write, retrying until the host accepts it, and fails with the host's own rejection + /// Reason if it never does. + /// + /// + /// Why a retry rather than a single Tell. ApplyAck — which + /// waits for — marks the end of the apply, not the point at + /// which a write can succeed. Several things still have to happen after it: the spawned child has to + /// finish InitializeAsync and leave Connecting (that state deliberately fast-fails + /// writes with "driver not connected"), and the host has to push the NodeId→driver reverse map that + /// resolves the write. Telling the write immediately therefore races the setup, which is why this + /// assertion failed roughly 1 run in 30 of the fully parallel assembly — and never once in 60 + /// consecutive runs of this class alone. + /// Why retrying does not weaken the assertion. Every rejection branch — the Primary + /// gate, an unresolved reverse map, "driver not running", and the child's own pre-Connected + /// fast-fail — replies without reaching the driver. A rejected attempt records nothing, so + /// the caller's Writes.Count.ShouldBe(1) still means exactly what it did before: the accepted + /// write reached the driver exactly once. + /// The failure message carries the last Reason so a genuine regression is diagnosable + /// rather than surfacing as a bare "expected True". + /// + /// The driver-host actor. + /// The ns-qualified NodeId string, as the node manager passes it. + /// The value to write. + /// The address-space realm the NodeId belongs to. + private void RouteWriteUntilAccepted( + IActorRef actor, string nodeId, object value, AddressSpaceRealm realm) + { + var lastReason = "(no reply)"; + AwaitAssert( + () => + { + var asker = CreateTestProbe(); + actor.Tell(new DriverHostActor.RouteNodeWrite(nodeId, value, realm), asker.Ref); + var result = asker.ExpectMsg(TimeSpan.FromSeconds(1)); + lastReason = result.Reason ?? "(none)"; + result.Success.ShouldBeTrue($"host kept rejecting the write; last reason: {lastReason}"); + }, + duration: Timeout, + interval: TimeSpan.FromMilliseconds(50)); + } + /// On a SECONDARY, RouteNodeWrite (via the UNS NodeId) replies "not primary" and the driver /// receives NO write — the primary gate fires before the reverse-map lookup. [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs index 381756e3..59794e08 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorTests.cs @@ -88,7 +88,7 @@ public sealed class DriverInstanceActorTests : RuntimeActorTestBase [Fact] public async Task Write_propagates_status_code_on_Bad_result() { - const uint badStatus = 0x80340000; // BadOutOfService — top severity bits = 10b + const uint badStatus = 0x80340000; // BadNodeIdUnknown — top severity bits = 10b var driver = new WritableStubDriver { NextStatusCode = badStatus }; var actor = Sys.ActorOf(DriverInstanceActor.Props(driver)); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs index f2fb1f81..be6fd400 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/RuntimeActorTestBase.cs @@ -11,6 +11,25 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; /// public abstract class RuntimeActorTestBase : TestKit { + /// + /// Shared upper bound for a presence wait (AwaitAssert / AwaitCondition). + /// + /// + /// A budget, not a delay. These helpers poll and return the instant the condition holds, + /// so the value is how long to wait before giving up. Raising it costs nothing on the happy path and + /// cannot make a genuinely failing assertion pass — it only changes how quickly a real + /// breakage is reported. That is the opposite of an absence window (ExpectNoMsg, + /// settle-then-assert), where the elapsed time IS the assertion and every millisecond is spent on + /// every run. Absence windows keep their own short, individually-calibrated literals and must not be + /// routed through here. + /// Why it exists (Gitea #500). This assembly runs 44 Akka test classes roughly 14-way + /// parallel, each with its own ActorSystem. Budgets of 300–500 ms sized on an + /// idle machine are comfortable in isolation and marginal under that contention: three consecutive + /// 30-run verification rounds each surfaced a different test failing on one, in three + /// different files. Fixing them one at a time was chasing a distribution rather than a defect. + /// + protected static readonly TimeSpan PresenceBudget = TimeSpan.FromSeconds(15); + /// Gets the Akka test HOCON configuration string for single-node cluster setup. protected static string AkkaTestHocon => @" akka { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Health/PeerProbeSupervisorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Health/PeerProbeSupervisorTests.cs index 4acd8e79..5937cd3e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Health/PeerProbeSupervisorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Health/PeerProbeSupervisorTests.cs @@ -57,7 +57,7 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase State(Adm, RedundancyRole.Detached))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies the child for a departed peer is stopped when the next snapshot omits it. @@ -71,11 +71,11 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase State(Local, RedundancyRole.Primary), State(Peer, RedundancyRole.Secondary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); sup.Tell(Snapshot(State(Local, RedundancyRole.Primary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies a single-node snapshot (just the local node) spawns no children. @@ -88,7 +88,7 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase sup.Tell(Snapshot(State(Local, RedundancyRole.Primary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies a previously-removed peer is respawned when it re-appears, without an @@ -103,17 +103,17 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase State(Local, RedundancyRole.Primary), State(Peer, RedundancyRole.Secondary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); sup.Tell(Snapshot(State(Local, RedundancyRole.Primary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); sup.Tell(Snapshot( State(Local, RedundancyRole.Primary), State(Peer, RedundancyRole.Secondary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Locks in the stale-Terminated guard: when an OLD (already-replaced) child's @@ -132,27 +132,27 @@ public sealed class PeerProbeSupervisorTests : RuntimeActorTestBase State(Local, RedundancyRole.Primary), State(Peer, RedundancyRole.Secondary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); - AwaitAssert(() => spawned.Count.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); + AwaitAssert(() => spawned.Count.ShouldBe(1), duration: PresenceBudget); var oldRef = spawned[0]; // Drop the peer -> child #0 stopped, ChildCount back to 0. sup.Tell(Snapshot(State(Local, RedundancyRole.Primary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); // Re-add the SAME peer -> a NEW child #1 (the FRESH ref) is spawned. sup.Tell(Snapshot( State(Local, RedundancyRole.Primary), State(Peer, RedundancyRole.Secondary))); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); - AwaitAssert(() => spawned.Count.ShouldBe(2), duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); + AwaitAssert(() => spawned.Count.ShouldBe(2), duration: PresenceBudget); // Now deliver a STALE Terminated for the OLD ref. The current child for Peer is the fresh // child #1, so ref-equality finds no match and the supervisor must leave ChildCount at 1. sup.Tell(new Terminated(oldRef, existenceConfirmed: true, addressTerminated: false)); AwaitAssert(() => sup.UnderlyingActor.ChildCount.ShouldBe(1), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs index 0a32804e..b665e221 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs @@ -295,10 +295,24 @@ public sealed class ContinuousHistorizationRecorderTests : TestKit // The first drain returns false (entry retained); after the backoff the retry drain succeeds // and acks, truncating the outbox to 0. - await AwaitAssertAsync(async () => - Assert.Equal(0, await outbox.CountAsync(default)), TimeSpan.FromSeconds(5)); - - Assert.True(writer.CallCount >= 2, "the writer must have been called at least twice (a retry happened)"); + // + // BOTH conditions are polled together, and the retry count is the load-bearing one. An empty + // outbox is NOT a distinguishing observation: it is equally true BEFORE the append lands, so + // waiting on it alone can be satisfied by the initial state and return before the recorder has + // done anything at all. That is exactly what happened intermittently under a fully parallel + // assembly run — the poll won the race against the first append, the wait returned immediately, + // and the CallCount check that used to sit outside the block then failed against a recorder that + // had not yet run. (The same trap is called out on the preceding test, which guards against it by + // pairing its count with a writer observation.) + await AwaitAssertAsync( + async () => + { + Assert.True( + writer.CallCount >= 2, + "the writer must have been called at least twice (a retry happened)"); + Assert.Equal(0, await outbox.CountAsync(default)); + }, + TimeSpan.FromSeconds(5)); } [Fact] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/HistorianAdapterActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/HistorianAdapterActorTests.cs index 4167c31d..d58ff74d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/HistorianAdapterActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/HistorianAdapterActorTests.cs @@ -23,9 +23,27 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase /// The local node id the gating tests construct the adapter with. private static readonly NodeId LocalNode = new("node-A"); - /// A short window we allow the fire-and-forget enqueue to land within. + /// + /// The window an absence assertion waits before concluding nothing arrived + /// (ExpectNoMsg). Its length is a calibration decision — long enough that a message which + /// was going to arrive would have — so it is deliberately NOT generous. + /// private static readonly TimeSpan Settle = TimeSpan.FromMilliseconds(500); + /// + /// The budget a presence assertion may take to become true (AwaitAssert). + /// + /// + /// Separate from on purpose, though they once shared its 500 ms. The two are + /// different quantities that merely had the same number: a presence budget is an upper bound before + /// giving up, and AwaitAssert returns the instant the condition holds, so a generous value + /// costs nothing in the passing case and can never make a genuinely failing assertion pass. An + /// absence window is the opposite — every millisecond is spent on every run. Conflating them meant + /// the enqueue assertions could only be given more headroom by slowing every ExpectNoMsg in + /// the class, so they kept a 500 ms budget that a fully parallel assembly run occasionally missed. + /// + private static readonly TimeSpan AssertTimeout = TimeSpan.FromSeconds(5); + /// Thread-safe fake sink that records every call. private sealed class RecordingSink : IAlarmHistorianSink { @@ -101,7 +119,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase actor.Tell(SampleEvent()); - AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), Settle); + AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), AssertTimeout); } /// Secondary suppression (T2): when the cached local role is Secondary, the adapter MUST @@ -145,7 +163,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase TellRedundancyRole(actor, RedundancyRole.Primary); actor.Tell(SampleEvent()); - AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), Settle); + AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), AssertTimeout); } /// Absent-node default-historize (T5): a snapshot that mentions only a DIFFERENT node @@ -174,7 +192,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase actor.Tell(SampleEvent()); // Local role is still unknown ⇒ default-historize path: sink must record exactly one enqueue. - AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), Settle); + AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), AssertTimeout); } /// Builds an (the shape published on the alerts @@ -222,7 +240,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase e.Severity.ShouldBe(AlarmSeverity.High); e.Comment.ShouldBe("note"); }, - Settle); + AssertTimeout); } /// Secondary suppression for alerts (T7): a Secondary node must NOT historize a transition @@ -252,7 +270,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase TellRedundancyRole(actor, RedundancyRole.Primary); actor.Tell(SampleTransition()); - AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), Settle); + AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), AssertTimeout); } /// Per-alarm opt-out (T8b): a Primary node must NOT historize a transition whose @@ -286,7 +304,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase TellRedundancyRole(actor, RedundancyRole.Primary); actor.Tell(SampleTransition(historizeToAveva: null)); - AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), Settle); + AwaitAssert(() => sink.EnqueueCount.ShouldBe(1), AssertTimeout); } /// Severity buckets (T9): the OPC UA 1–1000 numeric severity on the transition maps onto @@ -308,7 +326,7 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase AwaitAssert( () => sink.Events.ShouldHaveSingleItem().Severity.ShouldBe(expected), - Settle); + AssertTimeout); } /// Rolling-restart null default (T10): an old-format transition deserialized by Akka's JSON @@ -326,6 +344,6 @@ public sealed class HistorianAdapterActorTests : RuntimeActorTestBase AwaitAssert( () => sink.Events.ShouldHaveSingleItem().AlarmTypeName.ShouldBe("AlarmCondition"), - Settle); + AssertTimeout); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs index 33b4153c..4bd67911 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs @@ -48,11 +48,16 @@ public sealed class OpcUaPublishActorApplyFailureTests : RuntimeActorTestBase var dep2 = SeedEquipmentDeployment(db, ("eq-1", "eq-1-renamed")); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); + // A PRESENCE budget, so it is an upper bound before giving up rather than a wait: the helper polls + // and returns the instant the meter fires, so a generous value costs nothing on the happy path and + // can never make a genuinely failing assertion pass. Two seconds covered an idle machine but not a + // fully parallel assembly run, where this deploy→rebuild→throw chain (two DB-backed applies) has to + // share 14 cores with 40-odd other Akka test classes; it failed there roughly 1 run in 30. AwaitAssert(() => { recorder.Total.ShouldBeGreaterThanOrEqualTo(1); recorder.WithTag("kind", "rebuild").ShouldBeGreaterThanOrEqualTo(1); - }, duration: TimeSpan.FromSeconds(2)); + }, duration: TimeSpan.FromSeconds(15)); } /// A clean rebuild does NOT increment the apply-failed meter (Info-only happy path). diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index a51410b4..8a8c364f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -45,7 +45,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink.Calls.ShouldContain("EF:eq-1"); sink.Calls.ShouldContain("EF:eq-2"); sink.Calls.ShouldContain("NA:line-1"); - }, duration: TimeSpan.FromSeconds(2)); + }, duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); } @@ -85,7 +85,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); // PureAdd — no full rebuild var callsAfterFirst = sink.Calls.Count; @@ -107,7 +107,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500)); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); } /// @@ -132,7 +132,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), Artifact: artifact)); // The in-hand bytes drove the real diff-and-apply — the equipment folder was materialised… - AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: PresenceBudget); // …and the address-space-wiping raw-sink fallback was NOT taken. sink.RebuildCalls.ShouldBe(0); } @@ -176,7 +176,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500)); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); } /// @@ -197,7 +197,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep))); - AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); } @@ -230,7 +230,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase // PureAdd (equipment + tag) ⇒ no full rebuild; the materialise passes still run the cluster slice. // t-sa (EquipmentId "eq-sa", FolderPath "F", Name "S1") → folder-scoped variable "eq-sa/F/S1". - AwaitAssert(() => sinkA.Calls.ShouldContain("EV:eq-sa/F/S1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sinkA.Calls.ShouldContain("EV:eq-sa/F/S1"), duration: PresenceBudget); sinkA.RebuildCalls.ShouldBe(0); // t-main (MAIN cluster) must NOT leak onto the SITE-A node. sinkA.Calls.ShouldNotContain("EV:eq-main/F/M1"); @@ -249,7 +249,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase mainActor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sinkM.Calls.ShouldContain("EV:eq-main/F/M1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sinkM.Calls.ShouldContain("EV:eq-main/F/M1"), duration: PresenceBudget); sinkM.RebuildCalls.ShouldBe(0); sinkM.Calls.ShouldNotContain("EV:eq-sa/F/S1"); } @@ -331,7 +331,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.Calls.ShouldContain("NA:line-1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("NA:line-1"), duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); var calls = sink.Calls.ToList(); @@ -356,7 +356,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); - AwaitAssert(() => sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(1), duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); var naAfterFirst = sink.Calls.Count(c => c.StartsWith("NA:")); @@ -365,7 +365,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var dep2 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1-RENAMED"), ("eq-2", "Pump-2")); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); // No new NodeAdded announcement was raised for the rebuild-kind deploy. sink.Calls.Count(c => c.StartsWith("NA:")).ShouldBe(naAfterFirst); } @@ -388,7 +388,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); - AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: PresenceBudget); var callsAfterFirst = sink.Calls.Count; // The next deploy arrives while the ConfigDb is briefly unreachable. @@ -423,13 +423,13 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, dbFactory: db, applier: applier)); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); - AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-2"), duration: PresenceBudget); // eq-2 really is gone from the next artifact — that IS a configuration change, so it must apply. var dep2 = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1")); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep2))); - AwaitAssert(() => sink.Calls.ShouldContain("RE:eq-2"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("RE:eq-2"), duration: PresenceBudget); } /// An whose CreateDbContext can be made to @@ -552,7 +552,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase // First deploy: the area folder is materialised with the OLD name. R2-07 — this is now a PureAdd // (area + line + equipment all added), so NO full rebuild; the folder is materialised directly. actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep1))); - AwaitAssert(() => sink.Calls.ShouldContain("EF:area-1"), duration: TimeSpan.FromSeconds(2)); + AwaitAssert(() => sink.Calls.ShouldContain("EF:area-1"), duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); // Second deploy: ONLY the area Name changed — a rename. The actor must reach the apply path and @@ -563,7 +563,7 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase AwaitAssert(() => { sink.FolderRenameCalls.ShouldContain(("area-1", "Plant South")); - }, duration: TimeSpan.FromSeconds(2)); + }, duration: PresenceBudget); sink.RebuildCalls.ShouldBe(0); // the rename did NOT force a full rebuild } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs index e10e9329..a6f10ef3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs @@ -53,7 +53,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase sink.Values[0].Value.ShouldBe(3.14); sink.Values[0].Quality.ShouldBe(OpcUaQuality.Good); sink.Values[1].Quality.ShouldBe(OpcUaQuality.Uncertain); - }, duration: TimeSpan.FromMilliseconds(500)); + }, duration: PresenceBudget); } /// Verifies that AlarmStateUpdate routes to sink WriteAlarmCondition with the full snapshot. @@ -73,7 +73,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase sink.Alarms[0].State.Active.ShouldBeTrue(); sink.Alarms[0].State.Acknowledged.ShouldBeFalse(); sink.Alarms[0].State.Severity.ShouldBe((ushort)700); - }, duration: TimeSpan.FromMilliseconds(500)); + }, duration: PresenceBudget); } /// #477 — AlarmQualityUpdate routes to sink.WriteAlarmQuality with the quality + realm. @@ -93,7 +93,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase q.AlarmNodeId.ShouldBe("Plant/Modbus/dev1/temp_hi"); q.Quality.ShouldBe(OpcUaQuality.Bad); q.Realm.ShouldBe(AddressSpaceRealm.Raw); - }, duration: TimeSpan.FromMilliseconds(500)); + }, duration: PresenceBudget); } /// Builds a test with sensible defaults so each test @@ -117,7 +117,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500)); + AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: PresenceBudget); } /// Verifies that forwards to the @@ -146,7 +146,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase sink.Folders.ShouldContain(("EQ-1/Axes", "EQ-1", "Axes")); sink.Variables.ShouldContain(("EQ-1/Axes/X", "EQ-1/Axes", "X", "Double", false)); sink.ModelChanges.ShouldContain("EQ-1"); - }, duration: TimeSpan.FromMilliseconds(500)); + }, duration: PresenceBudget); } /// Verifies that ServiceLevelChanged publishes to IServiceLevelPublisher once per unique level. @@ -161,7 +161,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase actor.Tell(new OpcUaPublishActor.ServiceLevelChanged(100)); AwaitAssert(() => publisher.Levels.ShouldBe(new byte[] { 240, 100 }), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the very first computed ServiceLevel is always published even when it is @@ -186,7 +186,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldBe(new byte[] { 0 }), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that RedundancyStateChanged drives local ServiceLevel publish for primary leader. @@ -210,7 +210,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase actor.Tell(snapshot); AwaitAssert(() => publisher.Levels.ShouldBe(new byte[] { 240 }), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that RedundancyStateChanged for secondary publishes 100. @@ -231,7 +231,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase actor.Tell(snapshot); AwaitAssert(() => publisher.Levels.ShouldBe(new byte[] { 100 }), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the calculator path computes 250 for a healthy primary role-leader @@ -256,7 +256,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the calculator path computes 240 for a healthy non-leader secondary @@ -282,7 +282,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the calculator path computes 100 when the DB is unreachable @@ -307,7 +307,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)100), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the calculator path computes 200 for a stale snapshot when the DB is @@ -335,7 +335,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)200), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that a detached local node publishes 0 (the calculator does not model @@ -362,7 +362,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase }, CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); // Now detach — expect the guard to drive ServiceLevel down to 0. actor.Tell(new RedundancyStateChanged( @@ -374,7 +374,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that an actively-observed, recent peer probe of MY endpoint that came back @@ -403,7 +403,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies branch (3) of OpcUaProbeOk(): a peer's NEGATIVE verdict about this node @@ -447,7 +447,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase // Healthy (250), NOT 0 — proves an aged negative verdict does not demote. AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that with no peer probe result ever received, OpcUaProbeOk() defaults @@ -474,7 +474,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that a later Ok==true peer probe supersedes an earlier Ok==false @@ -503,7 +503,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that a peer probe result about a DIFFERENT node is ignored — it does not @@ -531,7 +531,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies the legacy back-compat seam: with no DB-health probe wired, the handler @@ -553,7 +553,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Verifies that the periodic HealthTick Asks the local @@ -607,7 +607,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// DB-less, member Up, snapshot fresh, Primary → 250 (basis 240 + 10 Primary bonus). @@ -629,7 +629,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// DB-less, snapshot stale (entry.AsOfUtc older than the stale window) → 200. Staleness @@ -654,7 +654,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)200), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// DB-less, Detached → 0. The Detached / missing-entry guard runs ABOVE the DB-less @@ -677,7 +677,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase }, CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); actor.Tell(new RedundancyStateChanged( Nodes: new[] @@ -688,7 +688,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)0), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Regression: a DB-BACKED node (probe wired + DbHealthStatus reachable, dbLess default @@ -714,7 +714,7 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase CorrelationId.NewId())); AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), - duration: TimeSpan.FromMilliseconds(500)); + duration: PresenceBudget); } /// Stub DB-health probe actor that answers diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ScriptedAlarms/ScriptedAlarmHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ScriptedAlarms/ScriptedAlarmHostActorTests.cs index 86686e86..27861788 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ScriptedAlarms/ScriptedAlarmHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ScriptedAlarms/ScriptedAlarmHostActorTests.cs @@ -28,7 +28,23 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.ScriptedAlarms; /// public sealed class ScriptedAlarmHostActorTests : RuntimeActorTestBase { - private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(8); + /// + /// Upper bound for the presence waits in this class (ExpectMsg / FishForMessage). + /// + /// + /// Sized for a real Roslyn compilation, not for message passing. + /// ApplyScriptedAlarms drives ScriptedAlarmEngine.LoadAsync, which compiles every + /// alarm predicate through ScriptEvaluator.Compile; the RegisterInterest these tests + /// wait on is only sent once that finishes. A cold C# script compile is slow and highly variable — + /// far more so than anything else in this assembly — and Roslyn's caches are per-process, so the + /// first class to compile pays the worst of it. Under a fully parallel assembly run, sharing 14 + /// cores with 40-odd other Akka test classes, 8 s was occasionally missed (Gitea #500). + /// Raising it is free on the happy path: these waits return the instant the message arrives, + /// so the value is an upper bound before giving up, not a delay, and it cannot make a genuinely + /// failing expectation pass. The absence assertions in this class deliberately keep their own + /// short literals — there the elapsed time is the point. + /// + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(30); /// Plan whose predicate compares the single tag "M.T" against 90 — enabled by default. private static EquipmentScriptedAlarmPlan Plan( diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs index a63d9ed5..38fb64bd 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs @@ -79,7 +79,7 @@ public sealed class VirtualTagActorTests : RuntimeActorTestBase entry.Message.ShouldContain("syntax error"); entry.ScriptId.ShouldBe("script-7"); entry.VirtualTagId.ShouldBe("vt-1"); - }, duration: TimeSpan.FromMilliseconds(500)); + }, duration: PresenceBudget); // 02/S13: the failure now ALSO degrades the node — with no declared dependencyRefs the // inputs-ready gate is vacuously satisfied, so a Bad EvaluationResult reaches the parent (this diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs index d74f7276..e61de11a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs @@ -260,7 +260,15 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase // The old child is stopped (PostStop ⇒ UnregisterInterest) and a new one spawned // (PreStart ⇒ RegisterInterest on "B"). Both messages arrive at the mux probe; order between // the dying child's PostStop and the new child's PreStart is not guaranteed, so accept either. + // + // Because the order is genuinely undefined, the SENDER of the RegisterInterest must be captured + // as that message is received. Reading mux.LastSender after the loop instead would read the + // sender of whichever message happened to arrive SECOND — the dying child when the interleaving + // is [Register, Unregister] — so the identity assertion below silently depended on the very + // ordering this loop exists to tolerate. That made it fail roughly 10% of the time under a + // fully parallel assembly run, and never in isolation. DependencyMuxActor.RegisterInterest? reg2 = null; + IActorRef? secondChild = null; var sawUnregister = false; for (var i = 0; i < 2; i++) { @@ -269,6 +277,7 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase { case DependencyMuxActor.RegisterInterest r: reg2 = r; + secondChild = mux.LastSender; // sender OF THIS message, not of the last one seen break; case DependencyMuxActor.UnregisterInterest: sawUnregister = true; @@ -282,7 +291,8 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase reg2.TagRefs.ShouldNotContain("A"); // The replacement is a different actor ref than the original (auto-named, so no collision). - mux.LastSender.ShouldNotBe(firstChild); + secondChild.ShouldNotBeNull(); + secondChild.ShouldNotBe(firstChild); } ///