Skip to content

Commit 7d4e617

Browse files
[PATCH] Applied changes as discussed in review (- WIP #175 -)
Changes in file .coderabbit.yaml: - mention code coverage for tests Changes in file Makefile: - tweak MANIFEST.in to ignore .yaml Changes in file multicast/exceptions.py: - refactor to avoid shadowing vars Changes in file multicast/hear.py: - minor refactor for readability Changes in file tests/context.py: - implemented managed_process context manager Changes in file tests/test_fuzz.py: - Refactored to use managed_process context manager Changes in file tests/test_usage.py: - minor fixes
1 parent 830114d commit 7d4e617

File tree

7 files changed

+96
-46
lines changed

7 files changed

+96
-46
lines changed

.coderabbit.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ reviews:
3838
# Test Code Review Instructions
3939
- Ensure that test code is automated, comprehensive, and follows testing best practices.
4040
- Verify that all critical functionality is covered by tests.
41+
- Ensure that the test coverage meets or exceeds the project's required threshold
42+
(e.g., aiming for 100% coverage as per Issue #53).
4143
- For **test** code, *also* follow
4244
[CEP-9](https://gist.github.com/reactive-firewall/d840ee9990e65f302ce2a8d78ebe73f6)
4345

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ MANIFEST.in: init
173173
$(QUIET)$(ECHO) "exclude .deepsource.toml" >>"$@" ;
174174
$(QUIET)$(ECHO) "exclude .*.ini" >>"$@" ;
175175
$(QUIET)$(ECHO) "exclude .*.yml" >>"$@" ;
176+
$(QUIET)$(ECHO) "exclude .*.yaml" >>"$@" ;
176177
$(QUIET)$(ECHO) "global-exclude .git" >>"$@" ;
177178
$(QUIET)$(ECHO) "global-exclude codecov_env" >>"$@" ;
178179
$(QUIET)$(ECHO) "global-exclude .DS_Store" >>"$@" ;

multicast/exceptions.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,12 +243,13 @@ def __init__(self, *args, **kwargs):
243243
else:
244244
exit_code = kwargs.pop("exit_code", 1)
245245
if len(args) > 0 and isinstance(args[0], BaseException):
246-
__cause__ = args[0]
246+
cause = args[0]
247247
args = args[1:]
248248
else:
249-
__cause__ = kwargs.pop("__cause__", None)
249+
cause = kwargs.pop("__cause__", None)
250250
super().__init__(*args, **kwargs)
251-
self.__cause__ = __cause__
251+
if cause is not None:
252+
self.__cause__ = cause
252253
self.message = args[0] if args else kwargs.get("message", "An error occurred")
253254
self.exit_code = exit_code
254255

multicast/hear.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
True
8888
>>> int(test_fixture) < int(256)
8989
True
90-
>>> (int(test_fixture) >= int(0)) and (int(test_fixture) < int(156))
90+
>>> (int(test_fixture) >= int(0)) and (int(test_fixture) < int(256))
9191
True
9292
>>>
9393
@@ -518,18 +518,22 @@ def doStep(self, *args, **kwargs):
518518
"""
519519
HOST = kwargs.get("group", multicast._MCAST_DEFAULT_GROUP) # skipcq: PYL-W0212 - module ok
520520
PORT = kwargs.get("port", multicast._MCAST_DEFAULT_PORT) # skipcq: PYL-W0212 - module ok
521-
_result = False
521+
server_initialized = False
522+
server = None
522523
try:
523524
with McastServer((HOST, PORT), HearUDPHandler) as server:
524-
_result = True
525+
server_initialized = True
525526
server.serve_forever()
526527
except KeyboardInterrupt as userInterrupt:
527528
try:
528-
old_sock = server.socket
529-
if old_sock:
529+
if server and server.socket:
530+
old_sock = server.socket
530531
multicast.endSocket(old_sock)
531532
finally:
532-
raise KeyboardInterrupt("HEAR ended after interruption was signaled.") from userInterrupt
533+
raise KeyboardInterrupt(
534+
f"HEAR has stopped due to interruption signal (was previously listening on ({HOST}, {PORT}))."
535+
) from userInterrupt
533536
finally:
534-
server.shutdown()
535-
return (_result, None)
537+
if server:
538+
server.shutdown()
539+
return (server_initialized, None)

tests/context.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,24 @@
9494
except ImportError as err: # pragma: no branch
9595
raise ModuleNotFoundError("[CWE-440] unittest Failed to import.") from err
9696

97+
try:
98+
if 'contextlib' not in sys.modules:
99+
import contextlib
100+
else: # pragma: no branch
101+
contextlib = sys.modules["""contextlib"""]
102+
except ImportError as err: # pragma: no branch
103+
raise ModuleNotFoundError("[CWE-440] contextlib Failed to import.") from err
104+
105+
106+
try:
107+
if 'contextlib.contextmanager' not in sys.modules:
108+
from contextlib import contextmanager
109+
else: # pragma: no branch
110+
contextlib.contextmanager = sys.modules["""contextlib.contextmanager"""]
111+
contextmanager = contextlib.contextmanager
112+
except ImportError as err: # pragma: no branch
113+
raise ModuleNotFoundError("[CWE-440] contextlib.contextmanager Failed to import.") from err
114+
97115

98116
try:
99117
if 'Process' not in sys.modules:
@@ -832,6 +850,32 @@ def debugUnexpectedOutput(expectedOutput, actualOutput, thepython):
832850
print(__BLANK)
833851

834852

853+
@contextmanager
854+
def managed_process(process):
855+
"""
856+
Context manager for safely handling multiprocessing processes.
857+
858+
Ensures that the given process is properly terminated and cleaned up,
859+
even if exceptions occur during execution. This includes terminating,
860+
joining, and closing the process to prevent resource leaks.
861+
862+
Args:
863+
process (multiprocessing.Process): The process to manage.
864+
865+
Yields:
866+
multiprocessing.Process: The managed process within the context.
867+
"""
868+
try:
869+
yield process
870+
finally:
871+
if process.is_alive():
872+
process.terminate()
873+
process.join(timeout=3)
874+
if process.is_alive():
875+
process.kill()
876+
process.close()
877+
878+
835879
class BasicUsageTestSuite(unittest.TestCase):
836880
"""
837881
Basic functional test cases.

tests/test_fuzz.py

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,14 @@ def test_say_works_WHEN_using_stdin_GIVEN_alnum_of_any_size_fuzz_input(self, tex
174174
"""
175175
Test the multicast send response to valid alnum input.
176176
177-
???
178-
179177
Args:
180178
text (str): A randomly generated string of ASCII letters and digits,
181179
with length between 56 and 2048 characters.
182180
183181
Assertions:
184-
- FIX ME
182+
- Verifies that the multicast sender can handle input from stdin
183+
- Confirms the process exits successfully after sending the message
184+
- Validates the receiver process terminates cleanly
185185
"""
186186
theResult = False
187187
fail_fixture = str(f"stdin({text.__sizeof__()}) --> SAY == error")
@@ -201,34 +201,31 @@ def test_say_works_WHEN_using_stdin_GIVEN_alnum_of_any_size_fuzz_input(self, tex
201201
)
202202
p.daemon = True
203203
p.start()
204-
self.assertIsNotNone(p)
205-
self.assertTrue(p.is_alive())
206-
try:
207-
sender = multicast.send.McastSAY()
208-
self.assertIsNotNone(sender)
209-
test_input = str(text)
210-
# ESSENTIAL PART OF THIS TEST
211-
self.assertIsNotNone(test_input)
212-
with patch('sys.stdin', io.StringIO(test_input)):
213-
self.assertIsNotNone(
214-
sender.doStep(data=['-'], group='224.0.0.1', port=_fixture_port_num)
215-
)
216-
self.assertIsNotNone(p)
217-
self.assertTrue(p.is_alive())
218-
while p.is_alive():
219-
sender(group="224.0.0.1", port=_fixture_port_num, data=["STOP", "Test"])
220-
p.join(1)
221-
self.assertFalse(p.is_alive())
222-
except Exception as _cause:
223-
p.join(3)
224-
if p.is_alive():
225-
p.terminate()
226-
p.close()
227-
raise unittest.SkipTest(sub_fail_fixture) from _cause
228-
p.join(5)
229-
self.assertIsNotNone(p.exitcode)
230-
self.assertEqual(int(p.exitcode), int(0))
231-
theResult = (int(p.exitcode) <= int(0))
204+
with context.managed_process(p) as managed_p:
205+
self.assertIsNotNone(managed_p)
206+
self.assertTrue(managed_p.is_alive())
207+
try:
208+
sender = multicast.send.McastSAY()
209+
self.assertIsNotNone(sender)
210+
test_input = str(text)
211+
# ESSENTIAL PART OF THIS TEST
212+
self.assertIsNotNone(test_input)
213+
with patch('sys.stdin', io.StringIO(test_input)):
214+
self.assertIsNotNone(
215+
sender.doStep(data=['-'], group='224.0.0.1', port=_fixture_port_num)
216+
)
217+
self.assertIsNotNone(p)
218+
self.assertTrue(p.is_alive())
219+
while p.is_alive():
220+
sender(group="224.0.0.1", port=_fixture_port_num, data=["STOP", "Test"])
221+
managed_p.join(1)
222+
self.assertFalse(managed_p.is_alive())
223+
except Exception as _cause:
224+
raise unittest.SkipTest(sub_fail_fixture) from _cause
225+
self.assertFalse(managed_p.is_alive(), "RESOURCE LEAK")
226+
self.assertIsNotNone(managed_p.exitcode)
227+
self.assertEqual(int(managed_p.exitcode), int(0))
228+
theResult = (int(managed_p.exitcode) <= int(0))
232229
except unittest.SkipTest as _skip_not_invalid:
233230
raise unittest.SkipTest(fail_fixture) from _skip_not_invalid
234231
except Exception as err:

tests/test_usage.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,14 +357,15 @@ def test_hear_works_WHEN_say_works(self):
357357
)
358358
p.start()
359359
try:
360+
tst_fixture_sendDispatch = multicast.__main__.McastDispatch()
360361
self.assertIsNotNone(
361-
multicast.__main__.McastDispatch().doStep(["""SAY""", _fixture_SAY_args])
362+
tst_fixture_sendDispatch.doStep(["""SAY""", _fixture_SAY_args])
362363
)
363364
self.assertIsNotNone(
364-
multicast.__main__.McastDispatch().doStep(["""SAY""", _fixture_SAY_args])
365+
tst_fixture_sendDispatch.doStep(["""SAY""", _fixture_SAY_args])
365366
)
366367
self.assertIsNotNone(
367-
multicast.__main__.McastDispatch().doStep(["""SAY""", _fixture_SAY_args])
368+
tst_fixture_sendDispatch.doStep(["""SAY""", _fixture_SAY_args])
368369
)
369370
except Exception as _cause:
370371
p.join()
@@ -442,7 +443,7 @@ def test_say_works_WHEN_using_stdin(self):
442443
test_input = "Test message from stdin"
443444
self.assertIsNotNone(test_input)
444445
with patch('sys.stdin', io.StringIO(test_input)):
445-
self.assertIsNone(say.doStep(data=['-'], group='224.0.0.1', port=_fixture_port_num))
446+
self.assertIsNotNone(say.doStep(data=['-'], group='224.0.0.1', port=_fixture_port_num))
446447
# Assert that the result is as expected
447448
# You might need to modify this based on what _sayStep returns
448449
theResult = True # or whatever the expected output is

0 commit comments

Comments
 (0)