From e14920ff38dce5851da2863a5a25ba4dabf90c62 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 9 Jul 2024 22:55:50 +0200 Subject: [PATCH 1/6] Tests: harden TestNetwork even more - take into account that payloads with DATA1 may be in flight when we update the task - test task.stop() - use more lenient timeout - use threading.Condition for improved readability - wait for periodicity to be established before validation; some platforms are a little bit flaky --- test/test_network.py | 46 ++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/test/test_network.py b/test/test_network.py index d488a2bc..f7acfbee 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -1,5 +1,5 @@ import unittest -from threading import Event +import threading import canopen import can @@ -57,6 +57,7 @@ def test_send_periodic(self): DATA2 = bytes([4, 5, 6]) COB_ID = 0x123 PERIOD = 0.1 + TIMEOUT = PERIOD * 10 self.network.connect( interface="virtual", channel=1, @@ -65,11 +66,12 @@ def test_send_periodic(self): self.addCleanup(self.network.disconnect) acc = [] - event = Event() + condition = threading.Condition() def hook(_, data, ts): - acc.append((data, ts)) - event.set() + item = data, ts + acc.append(item) + condition.notify() self.network.subscribe(COB_ID, hook) self.addCleanup(self.network.unsubscribe, COB_ID) @@ -77,18 +79,34 @@ def hook(_, data, ts): task = self.network.send_periodic(COB_ID, DATA1, PERIOD) self.addCleanup(task.stop) - event.wait(PERIOD*2) - - # Update task data. + def periodicity(): + # Check if periodicity is established; flakiness have been observed + # on macOS. + if len(acc) >= 2: + delta = acc[-1][1] - acc[-2][1] + return round(delta, ndigits=1) == PERIOD + return False + + # Wait for frames to arrive; then check the result. + with condition: + condition.wait_for(periodicity, TIMEOUT) + self.assertTrue(all(v[0] == DATA1 for v in acc)) + + # Update task data, which implicitly restarts the timer. + # Wait for frames to arrive; then check the result. task.update(DATA2) - event.clear() - event.wait(PERIOD*2) - task.stop() - + with condition: + acc.clear() + condition.wait_for(periodicity, TIMEOUT) + # Find the first message with new data, and verify that all subsequent + # messages also carry the new payload. data = [v[0] for v in acc] - self.assertEqual(data, [DATA1, DATA2]) - ts = [v[1] for v in acc] - self.assertAlmostEqual(ts[1]-ts[0], PERIOD, places=1) + idx = data.index(DATA2) + self.assertTrue(all(v[0] == DATA2 for v in acc[idx:])) + + # Stop the task. + task.stop() + self.assertIsNone(self.network.bus.recv(TIMEOUT)) class TestScanner(unittest.TestCase): From 15e5ad2769195470892429014f3aea97ffc671a3 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 10 Jul 2024 11:32:38 +0200 Subject: [PATCH 2/6] Allow one spurious message post disabling timers --- test/test_network.py | 7 ++++++- test/test_nmt.py | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/test_network.py b/test/test_network.py index f7acfbee..3578d66b 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -106,7 +106,12 @@ def periodicity(): # Stop the task. task.stop() - self.assertIsNone(self.network.bus.recv(TIMEOUT)) + # A message may have been in flight when we stopped the timer, + # so allow a single failure. + bus = self.network.bus + msg = bus.recv(TIMEOUT) + if msg: + self.assertIsNone(bus.recv(TIMEOUT)) class TestScanner(unittest.TestCase): diff --git a/test/test_nmt.py b/test/test_nmt.py index b6892cff..ca40ab59 100644 --- a/test/test_nmt.py +++ b/test/test_nmt.py @@ -117,7 +117,11 @@ def test_nmt_master_node_guarding(self): self.assertEqual(msg.dlc, 0) self.node.nmt.stop_node_guarding() - self.assertIsNone(self.bus.recv(self.TIMEOUT)) + # A message may have been in flight when we stopped the timer, + # so allow a single failure. + msg = self.bus.recv(self.TIMEOUT) + if msg: + self.assertIsNone(self.bus.recv(self.TIMEOUT)) class TestNmtSlave(unittest.TestCase): From 0945c8767a7ceee9ab15bf32819747e1e3bfca34 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 10 Jul 2024 12:56:51 +0200 Subject: [PATCH 3/6] Wording --- test/test_network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_network.py b/test/test_network.py index 3578d66b..26b67120 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -80,7 +80,7 @@ def hook(_, data, ts): self.addCleanup(task.stop) def periodicity(): - # Check if periodicity is established; flakiness have been observed + # Check if periodicity is established; flakiness has been observed # on macOS. if len(acc) >= 2: delta = acc[-1][1] - acc[-2][1] From bf50a22d34ee00de170dd98173ea89b91cdff85e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 10 Jul 2024 12:59:14 +0200 Subject: [PATCH 4/6] Be slightly more specific --- test/test_network.py | 2 +- test/test_nmt.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_network.py b/test/test_network.py index 26b67120..b646cfd1 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -110,7 +110,7 @@ def periodicity(): # so allow a single failure. bus = self.network.bus msg = bus.recv(TIMEOUT) - if msg: + if msg is not None: self.assertIsNone(bus.recv(TIMEOUT)) diff --git a/test/test_nmt.py b/test/test_nmt.py index ca40ab59..059e1982 100644 --- a/test/test_nmt.py +++ b/test/test_nmt.py @@ -120,7 +120,7 @@ def test_nmt_master_node_guarding(self): # A message may have been in flight when we stopped the timer, # so allow a single failure. msg = self.bus.recv(self.TIMEOUT) - if msg: + if msg is not None: self.assertIsNone(self.bus.recv(self.TIMEOUT)) From e4a2135a1722c07951692d3f1f28240cce89a428 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 12 Jul 2024 12:58:08 +0200 Subject: [PATCH 5/6] Make sure condition is locked before notifying --- test/test_network.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test_network.py b/test/test_network.py index b646cfd1..56c292fc 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -69,9 +69,10 @@ def test_send_periodic(self): condition = threading.Condition() def hook(_, data, ts): - item = data, ts - acc.append(item) - condition.notify() + with condition: + item = data, ts + acc.append(item) + condition.notify_all() self.network.subscribe(COB_ID, hook) self.addCleanup(self.network.unsubscribe, COB_ID) From 0363ecea7cca679cb3ff0f2cacec13d3845c2a4e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 12 Jul 2024 13:12:08 +0200 Subject: [PATCH 6/6] Amend comment --- test/test_network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_network.py b/test/test_network.py index 56c292fc..315c613b 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -93,7 +93,7 @@ def periodicity(): condition.wait_for(periodicity, TIMEOUT) self.assertTrue(all(v[0] == DATA1 for v in acc)) - # Update task data, which implicitly restarts the timer. + # Update task data, which may implicitly restart the timer. # Wait for frames to arrive; then check the result. task.update(DATA2) with condition: