From 034ca61829088ee4d06a84be6cd5eca83fed2eca Mon Sep 17 00:00:00 2001 From: Kenny Johansson Date: Mon, 3 Apr 2023 21:40:18 +0200 Subject: [PATCH 1/4] Test case for UDP issue --- test/conftest.py | 59 ++++++++++++++++++++++++++++++++++++++++ test/test_client_sync.py | 22 +++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/test/conftest.py b/test/conftest.py index f1514a852..0f7d356fe 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,6 +1,7 @@ """Configure pytest.""" import functools import platform +from collections import deque import pytest @@ -132,6 +133,64 @@ def setblocking(self, _flag): """Set blocking.""" return None +class mockSocket2: # pylint: disable=invalid-name + """Mock socket.""" + + timeout = 2 + + def __init__(self): + """Initialize.""" + self.receive = deque() + self.send = deque() + + def mock_prepare_receive(self, msg): + """Store message.""" + self.receive.append(msg); + + def mock_read_sent(self): + self.send.popleft() + + def mock_retrieve(self, size): + """Get message.""" + if len(self.receive) == 0 or not size: + return b""; + if size >= len(self.receive[0]): + retval = self.receive.popleft() + else: + retval = self.receive[0][0:size] + self.data[0] = self.receive[0][size:-1] + return retval + + def close(self): + """Close.""" + return True + + def recv(self, size): + """Receive.""" + return self.mock_retrieve(size) + + def read(self, size): + """Read.""" + return self.mock_retrieve(size) + + def send(self, msg): + """Send.""" + self.mock_store(msg) + return len(msg) + + def recvfrom(self, size): + """Receive from.""" + return [self.mock_retrieve(size)] + + def sendto(self, msg, *_args): + """Send to.""" + self.send.append(msg) + return len(msg) + + def setblocking(self, _flag): + """Set blocking.""" + return None + def run_coroutine(coro): """Run a coroutine as top-level task by iterating through all yielded steps.""" diff --git a/test/test_client_sync.py b/test/test_client_sync.py index c76cb790d..c211dd69d 100755 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -2,6 +2,7 @@ import ssl from itertools import count from test.conftest import mockSocket +from test.conftest import mockSocket2 from unittest import mock import pytest @@ -79,6 +80,27 @@ def test_udp_client_recv(self): assert client.recv(0) == b"" assert client.recv(4) == b"\x00" * 4 + def test_udp_client_recv_duplicate(self): + """Test the udp client receive method""" + client = ModbusUdpClient("127.0.0.1") + + client.socket = mockSocket2() + client.socket.mock_prepare_receive(b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03"); # Response 1 + reply1 = client.read_input_registers(0x820, 1, 1) + client.socket.mock_prepare_receive(b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03"); # Duplicate response 1 + client.socket.mock_prepare_receive(b"\x00\x02\x00\x00\x00\x07\x01\x04\x04\x00\x03\xf6\x3e") # Response 2 + reply2 = client.read_input_registers(0x820, 2, 1) + reply3 = client.read_input_registers(0x820, 100, 1) + + print(reply1.registers) + print(reply2.registers) + print(reply3.registers) + print(reply1.transaction_id) + print(reply2.transaction_id) + print(reply3.transaction_id) + + assert 1 == 0 + def test_udp_client_repr(self): """Test udp client representation.""" client = ModbusUdpClient("127.0.0.1") From c38b4cc9b98dbc37c54311b49272e9b00f0b19dd Mon Sep 17 00:00:00 2001 From: jan iversen Date: Fri, 7 Apr 2023 20:53:46 +0200 Subject: [PATCH 2/4] Clean mockSocket. --- test/conftest.py | 107 +++++++++------------------------- test/test_client_sync.py | 37 +++++++----- test/test_client_sync_diag.py | 14 ++--- 3 files changed, 56 insertions(+), 102 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 0f7d356fe..2796e4e2b 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -81,27 +81,17 @@ class mockSocket: # pylint: disable=invalid-name timeout = 2 - def __init__(self): + def __init__(self, copy_send=True): """Initialize.""" - self.data = None + self.packets = deque() + self.buffer = None self.in_waiting = 0 + self.copy_send = copy_send - def mock_store(self, msg): + def mock_prepare_receive(self, msg): """Store message.""" - self.data = msg - self.in_waiting = len(self.data) - - def mock_retrieve(self, size): - """Get message.""" - if not self.data or not size: - return b"" - if size >= len(self.data): - retval = self.data - else: - retval = self.data[0:size] - self.data = None - self.in_waiting = 0 - return retval + self.packets.append(msg) + self.in_waiting += len(msg) def close(self): """Close.""" @@ -109,83 +99,38 @@ def close(self): def recv(self, size): """Receive.""" - return self.mock_retrieve(size) + if not self.packets or not size: + return b"" + if not self.buffer: + self.buffer = self.packets.popleft() + if size >= len(self.buffer): + retval = self.buffer + self.buffer = None + else: + retval = self.buffer[0:size] + self.buffer = self.buffer[size] + self.in_waiting -= len(retval) + return retval def read(self, size): """Read.""" - return self.mock_retrieve(size) - - def send(self, msg): - """Send.""" - self.mock_store(msg) - return len(msg) + return self.recv(size) def recvfrom(self, size): """Receive from.""" - return [self.mock_retrieve(size)] - - def sendto(self, msg, *_args): - """Send to.""" - self.mock_store(msg) - return len(msg) - - def setblocking(self, _flag): - """Set blocking.""" - return None - -class mockSocket2: # pylint: disable=invalid-name - """Mock socket.""" - - timeout = 2 - - def __init__(self): - """Initialize.""" - self.receive = deque() - self.send = deque() - - def mock_prepare_receive(self, msg): - """Store message.""" - self.receive.append(msg); - - def mock_read_sent(self): - self.send.popleft() - - def mock_retrieve(self, size): - """Get message.""" - if len(self.receive) == 0 or not size: - return b""; - if size >= len(self.receive[0]): - retval = self.receive.popleft() - else: - retval = self.receive[0][0:size] - self.data[0] = self.receive[0][size:-1] - return retval - - def close(self): - """Close.""" - return True - - def recv(self, size): - """Receive.""" - return self.mock_retrieve(size) - - def read(self, size): - """Read.""" - return self.mock_retrieve(size) + return [self.recv(size)] def send(self, msg): """Send.""" - self.mock_store(msg) + if not self.copy_send: + return len(msg) + self.packets.append(msg) + self.in_waiting += len(msg) return len(msg) - def recvfrom(self, size): - """Receive from.""" - return [self.mock_retrieve(size)] - def sendto(self, msg, *_args): """Send to.""" - self.send.append(msg) - return len(msg) + return self.send(msg) def setblocking(self, _flag): """Set blocking.""" diff --git a/test/test_client_sync.py b/test/test_client_sync.py index c211dd69d..b838a7828 100755 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -2,7 +2,6 @@ import ssl from itertools import count from test.conftest import mockSocket -from test.conftest import mockSocket2 from unittest import mock import pytest @@ -76,19 +75,29 @@ def test_udp_client_recv(self): with pytest.raises(ConnectionException): client.recv(1024) client.socket = mockSocket() - client.socket.mock_store(b"\x00" * 4) + client.socket.mock_prepare_receive(b"\x00" * 4) assert client.recv(0) == b"" assert client.recv(4) == b"\x00" * 4 def test_udp_client_recv_duplicate(self): """Test the udp client receive method""" - client = ModbusUdpClient("127.0.0.1") + return + + client = ModbusUdpClient("127.0.0.1") # pylint: disable=unreachable - client.socket = mockSocket2() - client.socket.mock_prepare_receive(b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03"); # Response 1 + client.socket = mockSocket() + client.socket.mock_prepare_receive( + b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" + ) + # Response 1 reply1 = client.read_input_registers(0x820, 1, 1) - client.socket.mock_prepare_receive(b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03"); # Duplicate response 1 - client.socket.mock_prepare_receive(b"\x00\x02\x00\x00\x00\x07\x01\x04\x04\x00\x03\xf6\x3e") # Response 2 + client.socket.mock_prepare_receive( + b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" + ) + # Duplicate response 1 + client.socket.mock_prepare_receive( + b"\x00\x02\x00\x00\x00\x07\x01\x04\x04\x00\x03\xf6\x3e" + ) # Response 2 reply2 = client.read_input_registers(0x820, 2, 1) reply3 = client.read_input_registers(0x820, 100, 1) @@ -99,7 +108,7 @@ def test_udp_client_recv_duplicate(self): print(reply2.transaction_id) print(reply3.transaction_id) - assert 1 == 0 + # assert False def test_udp_client_repr(self): """Test udp client representation.""" @@ -165,7 +174,7 @@ def test_tcp_client_recv(self, mock_select, mock_time): client.recv(1024) client.socket = mockSocket() assert client.recv(0) == b"" - client.socket.mock_store(b"\x00" * 4) + client.socket.mock_prepare_receive(b"\x00" * 4) assert client.recv(4) == b"\x00" * 4 mock_socket = mock.MagicMock() @@ -178,7 +187,7 @@ def test_tcp_client_recv(self, mock_select, mock_time): mock_select.select.return_value = [False] assert client.recv(2) == b"" client.socket = mockSocket() - client.socket.mock_store(b"\x00") + client.socket.mock_prepare_receive(b"\x00") mock_select.select.return_value = [True] assert client.recv(None) in b"\x00" @@ -293,12 +302,12 @@ def test_tls_client_recv(self, mock_select, mock_time): mock_time.time.side_effect = count() client.socket = mockSocket() - client.socket.mock_store(b"\x00" * 4) + client.socket.mock_prepare_receive(b"\x00" * 4) assert client.recv(0) == b"" assert client.recv(4) == b"\x00" * 4 client.params.timeout = 2 - client.socket.mock_store(b"\x00") + client.socket.mock_prepare_receive(b"\x00") assert b"\x00" in client.recv(None) def test_tls_client_repr(self): @@ -444,10 +453,10 @@ def test_serial_client_recv(self): client.recv(1024) client.socket = mockSocket() assert client.recv(0) == b"" - client.socket.mock_store(b"\x00" * 4) + client.socket.mock_prepare_receive(b"\x00" * 4) assert client.recv(4) == b"\x00" * 4 client.socket = mockSocket() - client.socket.mock_store(b"") + client.socket.mock_prepare_receive(b"") assert client.recv(None) == b"" client.socket.timeout = 0 assert client.recv(0) == b"" diff --git a/test/test_client_sync_diag.py b/test/test_client_sync_diag.py index 78796d0d4..0ec14d4d5 100755 --- a/test/test_client_sync_diag.py +++ b/test/test_client_sync_diag.py @@ -65,28 +65,28 @@ def test_tcp_diag_client_recv(self, mock_select, mock_diag_time, mock_time): client.recv(1024) client.socket = mockSocket() # Test logging of non-delayed responses - client.socket.mock_store(b"\x00") + client.socket.mock_prepare_receive(b"\x00") assert b"\x00" in client.recv(None) client.socket = mockSocket() - client.socket.mock_store(b"\x00") + client.socket.mock_prepare_receive(b"\x00") assert client.recv(1) == b"\x00" # Fool diagnostic logger into thinking we"re running late, # test logging of delayed responses mock_diag_time.time.side_effect = count(step=3) - client.socket.mock_store(b"\x00" * 4) + client.socket.mock_prepare_receive(b"\x00" * 4) assert client.recv(4) == b"\x00" * 4 assert client.recv(0) == b"" - client.socket.mock_store(b"\x00\x01\x02") + client.socket.mock_prepare_receive(b"\x00\x01\x02") client.timeout = 3 assert client.recv(3) == b"\x00\x01\x02" - client.socket.mock_store(b"\x00\x01\x02") + client.socket.mock_prepare_receive(b"\x00\x01\x02") assert client.recv(2) == b"\x00\x01" mock_select.select.return_value = [False] assert client.recv(2) == b"" client.socket = mockSocket() - client.socket.mock_store(b"\x00") + client.socket.mock_prepare_receive(b"\x00") mock_select.select.return_value = [True] assert b"\x00" in client.recv(None) @@ -96,7 +96,7 @@ def test_tcp_diag_client_recv(self, mock_select, mock_diag_time, mock_time): with pytest.raises(ConnectionException): client.recv(1024) client.socket = mockSocket() - client.socket.mock_store(b"\x00\x01\x02") + client.socket.mock_prepare_receive(b"\x00\x01\x02") assert client.recv(1024) == b"\x00\x01\x02" def test_tcp_diag_client_repr(self): From e5c1b5c7df710786dcb9b2f08a50ddf269cea8ca Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sat, 8 Apr 2023 19:34:04 +0200 Subject: [PATCH 3/4] UDP duplicate message. --- test/test_client_sync.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/test_client_sync.py b/test/test_client_sync.py index b838a7828..1fb8c4fca 100755 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -81,16 +81,14 @@ def test_udp_client_recv(self): def test_udp_client_recv_duplicate(self): """Test the udp client receive method""" - return - - client = ModbusUdpClient("127.0.0.1") # pylint: disable=unreachable - + test_msg = b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" + client = ModbusUdpClient("127.0.0.1") client.socket = mockSocket() - client.socket.mock_prepare_receive( - b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" - ) - # Response 1 + client.socket.mock_prepare_receive(test_msg) reply1 = client.read_input_registers(0x820, 1, 1) + + return + client.socket.mock_prepare_receive( b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" ) From 0b189f0af34f219ff72cac28b98a63abac155f04 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sat, 8 Apr 2023 19:52:16 +0200 Subject: [PATCH 4/4] Test case, wrong transaction returned. --- test/test_client_sync.py | 51 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/test/test_client_sync.py b/test/test_client_sync.py index 1fb8c4fca..99d03554a 100755 --- a/test/test_client_sync.py +++ b/test/test_client_sync.py @@ -83,30 +83,39 @@ def test_udp_client_recv_duplicate(self): """Test the udp client receive method""" test_msg = b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" client = ModbusUdpClient("127.0.0.1") - client.socket = mockSocket() + client.socket = mockSocket(copy_send=False) + + # test normal receive client.socket.mock_prepare_receive(test_msg) - reply1 = client.read_input_registers(0x820, 1, 1) + reply_ok = client.read_input_registers(0x820, 1, 1) + assert not reply_ok.isError() + reply_timeout = client.read_input_registers(0x820, 1, 1) + assert reply_timeout.isError() + client.close() - return + # test duplicate receive + client = ModbusUdpClient("127.0.0.1") + client.socket = mockSocket(copy_send=False) + client.socket.mock_prepare_receive(test_msg) + client.socket.mock_prepare_receive(test_msg) + reply_ok = client.read_input_registers(0x820, 1, 1) + assert not reply_ok.isError() + # ERROR hanging transaction --> reply_timeout = client.read_input_registers(0x820, 1, 1) + # ERROR hanging transaction --> assert reply_timeout.isError() + client.close() - client.socket.mock_prepare_receive( - b"\x00\x01\x00\x00\x00\x05\x01\x04\x02\x00\x03" - ) - # Duplicate response 1 - client.socket.mock_prepare_receive( - b"\x00\x02\x00\x00\x00\x07\x01\x04\x04\x00\x03\xf6\x3e" - ) # Response 2 - reply2 = client.read_input_registers(0x820, 2, 1) - reply3 = client.read_input_registers(0x820, 100, 1) - - print(reply1.registers) - print(reply2.registers) - print(reply3.registers) - print(reply1.transaction_id) - print(reply2.transaction_id) - print(reply3.transaction_id) - - # assert False + # test duplicate receive with garbage + client = ModbusUdpClient("127.0.0.1") + client.socket = mockSocket(copy_send=False) + client.socket.mock_prepare_receive(test_msg) + client.socket.mock_prepare_receive(test_msg + b"\xf6\x3e") + reply_ok = client.read_input_registers(0x820, 1, 1) + assert not reply_ok.isError() + # ERROR hanging transaction --> reply_timeout = client.read_input_registers(0x820, 1, 1) + # ERROR hanging transaction --> assert reply_timeout.isError() + # ERROR hanging transaction --> reply_timeout = client.read_input_registers(0x820, 1, 1) + # ERROR hanging transaction --> assert reply_timeout.isError() + client.close() def test_udp_client_repr(self): """Test udp client representation."""