From b1f3e09661395f4e20f0f226aeb8898823e24174 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 23 May 2026 19:45:57 -0400 Subject: [PATCH] test(modbus, abcip): align failing integration tests with fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real bugs uncovered by re-running with the new fixture defaults pointing at the shared docker host. Both are test-side, not driver-side. AbCip — Driver_reads_seeded_DInt_from_ab_server (4 parametrized rows): Hardcoded 'ab://127.0.0.1:{port}/1,0' in the deviceUri instead of the resolved fixture.Host. The new 10.100.0.35 default (and any AB_SERVER_ENDPOINT override) silently couldn't reach this test — the driver tried to connect to a non-existent localhost:44818 and returned BadCommunicationError on all 4 profile rows. The sibling Emulate tests already use the fixture's resolved endpoint; this smoke test was missed in the original migration. Fix: deviceUri = $"ab://{fixture.Host}:{fixture.Port}/1,0". Modbus — Float32_With_CDAB_Roundtrips_Through_Wire: Test wrote a Float32 to HR 100 (2 consecutive registers: 100+101). standard.json's writable HR range declares [100,100] only — a single-cell auto-incrementing register, not a 2-register pair. The write to register 101 was rejected with Illegal Data Address (BadOutOfRange). Fix: moved the tag from HR 100 to HR 200 (in standard.json's '[200, 209]' scratch range — 10 consecutive writable HRs). The Float32+CDAB semantic the test exercises is unchanged. Modbus — Block_Read_Coalescing_Reduces_PDU_Count_End_To_End: Test read HR 300, 302, 304 — outside both the writable ranges and the uint16 seed list. pymodbus rejects reads to unseeded HRs even though 'hr size' is 2048. BadOutOfRange on every read. Fix: moved the tags from 300/302/304 to 200/202/204 (within the scratch range). The non-contiguous coalescing semantic (3 tags inside a 5-register window with MaxReadGap=5) is preserved. After this commit: - Modbus.IntegrationTests: 6/38 pass / 32 skip / 0 fail (was 4 pass / 32 skip / 2 fail; 32 skips are profile-gated ExceptionInjectionTests — they need MODBUS_SIM_PROFILE= exception_injection and a different container, intentional gating) - AbCip.IntegrationTests: 10/12 pass / 2 skip / 0 fail (was 6 pass / 2 skip / 4 fail; 2 skips are Emulate tests that need the fixture for separate scenarios) No driver code changed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AbCipReadSmokeTests.cs | 6 +++++- .../AddressingGrammarTests.cs | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipReadSmokeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipReadSmokeTests.cs index 2d6457f..150cfdb 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipReadSmokeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/AbCipReadSmokeTests.cs @@ -26,7 +26,11 @@ public sealed class AbCipReadSmokeTests await fixture.InitializeAsync(); try { - var deviceUri = $"ab://127.0.0.1:{fixture.Port}/1,0"; + // Use fixture.Host (not hardcoded 127.0.0.1) so the docker-host migration + // (10.100.0.35) and AB_SERVER_ENDPOINT overrides reach this test too. The + // sibling Emulate tests already use the fixture's resolved endpoint; this + // smoke test was missed. + var deviceUri = $"ab://{fixture.Host}:{fixture.Port}/1,0"; var drv = new AbCipDriver(new AbCipDriverOptions { Devices = [new AbCipDeviceOptions(deviceUri, profile.Family)], diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/AddressingGrammarTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/AddressingGrammarTests.cs index 8655644..e0cbaff 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/AddressingGrammarTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.IntegrationTests/AddressingGrammarTests.cs @@ -54,7 +54,12 @@ public sealed class AddressingGrammarTests { if (_sim.SkipReason is not null) Assert.Skip(_sim.SkipReason); - var tag = new ModbusTagDefinition("Tank", ModbusRegion.HoldingRegisters, 100, ModbusDataType.Float32, + // HR 200 lives in pymodbus's scratch range (`write: [200, 209]` per standard.json), + // which gives us two consecutive writable HRs for the Float32 round-trip. The earlier + // version used HR 100 — but standard.json declares HR 100 as a single-cell auto- + // incrementing register (`write: [100, 100]`) so the second register of the Float32 + // write was rejected with Illegal Data Address. + var tag = new ModbusTagDefinition("Tank", ModbusRegion.HoldingRegisters, 200, ModbusDataType.Float32, ByteOrder: ModbusByteOrder.WordSwap); var drv = await NewDriverAsync(tag); @@ -87,9 +92,13 @@ public sealed class AddressingGrammarTests if (_sim.SkipReason is not null) Assert.Skip(_sim.SkipReason); // Sanity check that the simulator accepts the larger PDU coalescing produces. - var t1 = new ModbusTagDefinition("T1", ModbusRegion.HoldingRegisters, 300, ModbusDataType.Int16); - var t2 = new ModbusTagDefinition("T2", ModbusRegion.HoldingRegisters, 302, ModbusDataType.Int16); - var t3 = new ModbusTagDefinition("T3", ModbusRegion.HoldingRegisters, 304, ModbusDataType.Int16); + // Using HR 200/202/204 in the scratch range (standard.json's `uint16: 200..209`), + // not 300/302/304 — pymodbus rejects reads outside the seeded uint16 list with + // Illegal Data Address (= BadOutOfRange). The coalescing semantic the test + // exercises is identical with the scratch addresses. + var t1 = new ModbusTagDefinition("T1", ModbusRegion.HoldingRegisters, 200, ModbusDataType.Int16); + var t2 = new ModbusTagDefinition("T2", ModbusRegion.HoldingRegisters, 202, ModbusDataType.Int16); + var t3 = new ModbusTagDefinition("T3", ModbusRegion.HoldingRegisters, 204, ModbusDataType.Int16); var opts = new ModbusDriverOptions { Host = _sim.Host, Port = _sim.Port, Tags = [t1, t2, t3], MaxReadGap = 5,