Skip to content

Commit a9617ea

Browse files
yashhzdamilcarlucas
authored andcommitted
refactor(tests): deduplicate capture_safe_write into backend_safe_file_io.py
Move the capture_safe_write helper that was defined 6 times in test_backend_filesystem_vehicle_components.py into a single make_capture_safe_write() factory in backend_safe_file_io.py. Address Copilot review comments: - Use precise type annotations (dict[str, Any], Callable[[IO[str]], object]) in make_capture_safe_write for pyright/mypy compatibility - Restore captured_data in test_save_component_templates_empty_input to verify that the written JSON is actually an empty dict use implicit booleaness for empty dict check (pylint C1803) Follow-up to PR #1437 as requested by @amilcarlucas. Signed-off-by: Yash Goel <yashhzd@users.noreply.github.com>
1 parent c64993a commit a9617ea

File tree

2 files changed

+53
-56
lines changed

2 files changed

+53
-56
lines changed

ardupilot_methodic_configurator/backend_safe_file_io.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
SPDX-License-Identifier: GPL-3.0-or-later
1010
"""
1111

12+
import io
13+
import json
1214
import os
1315
import tempfile
1416
from contextlib import suppress
15-
from typing import IO, Callable
17+
from typing import IO, Any, Callable
1618

1719

1820
def safe_write(filepath: str, write_func: Callable[[IO[str]], object]) -> None:
@@ -73,3 +75,38 @@ def safe_write(filepath: str, write_func: Callable[[IO[str]], object]) -> None:
7375
if not replaced:
7476
with suppress(OSError):
7577
os.unlink(tmp_path)
78+
79+
80+
# ==================== SAFE-WRITE TEST HELPERS ====================
81+
82+
83+
def make_capture_safe_write() -> tuple[ # pragma: no cover
84+
dict[str, Any], list[bool], Callable[[str, Callable[[IO[str]], object]], None]
85+
]:
86+
"""
87+
Create a ``safe_write`` side-effect that captures written JSON data.
88+
89+
Returns a (captured_data, called, side_effect) triple:
90+
- ``captured_data``: dict updated with the JSON that the write_func produces.
91+
- ``called``: single-element list (``[False]``) flipped to ``True`` on invocation.
92+
- ``side_effect``: function to assign to ``mock_safe_write.side_effect``.
93+
94+
Usage::
95+
96+
captured_data, called, side_effect = make_capture_safe_write()
97+
mock_safe_write.side_effect = side_effect
98+
...
99+
assert called[0]
100+
assert captured_data == expected
101+
102+
"""
103+
captured_data: dict[str, Any] = {}
104+
called: list[bool] = [False]
105+
106+
def _capture(_filepath: str, write_func: Callable[[IO[str]], object]) -> None:
107+
fake_file = io.StringIO()
108+
write_func(fake_file)
109+
captured_data.update(json.loads(fake_file.getvalue()))
110+
called[0] = True
111+
112+
return captured_data, called, _capture

tests/test_backend_filesystem_vehicle_components.py

Lines changed: 15 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,14 @@
1010
SPDX-License-Identifier: GPL-3.0-or-later
1111
"""
1212

13-
import io
14-
import json as json_mod
1513
import os.path
1614
from json.decoder import JSONDecodeError as RealJSONDecodeError
1715
from unittest.mock import mock_open, patch
1816

1917
import pytest
2018

2119
from ardupilot_methodic_configurator.backend_filesystem_vehicle_components import VehicleComponents
20+
from ardupilot_methodic_configurator.backend_safe_file_io import make_capture_safe_write
2221
from ardupilot_methodic_configurator.data_model_template_overview import TemplateOverview
2322

2423
# pylint: disable=too-many-lines,too-many-public-methods,attribute-defined-outside-init
@@ -654,14 +653,8 @@ def test_save_component_templates_basic( # type: ignore[misc] # pylint: disable
654653
mock_load_user.return_value = {}
655654

656655
# Capture written data via safe_write callback
657-
captured_data = {}
658-
659-
def capture_safe_write(_filepath, write_func) -> None:
660-
fake_file = io.StringIO()
661-
write_func(fake_file)
662-
captured_data.update(json_mod.loads(fake_file.getvalue()))
663-
664-
mock_safe_write.side_effect = capture_safe_write
656+
captured_data, _, side_effect = make_capture_safe_write()
657+
mock_safe_write.side_effect = side_effect
665658

666659
templates = {"Component1": [{"name": "Test Template", "data": {"param": "value"}, "is_user_modified": True}]}
667660

@@ -725,14 +718,8 @@ def test_save_component_templates_only_modified( # type: ignore[misc] # pylint:
725718
mock_load_user.return_value = {}
726719

727720
# Capture and verify written data
728-
captured_data = {}
729-
730-
def capture_safe_write(_filepath, write_func) -> None:
731-
fake_file = io.StringIO()
732-
write_func(fake_file)
733-
captured_data.update(json_mod.loads(fake_file.getvalue()))
734-
735-
mock_safe_write.side_effect = capture_safe_write
721+
captured_data, _, side_effect = make_capture_safe_write()
722+
mock_safe_write.side_effect = side_effect
736723

737724
# Call method
738725
result, msg = self.vehicle_components.save_component_templates(templates)
@@ -780,14 +767,8 @@ def test_save_component_templates_different_data( # type: ignore[misc] # pylint
780767
mock_load_user.return_value = {}
781768

782769
# Capture and verify written data
783-
captured_data = {}
784-
785-
def capture_safe_write(_filepath, write_func) -> None:
786-
fake_file = io.StringIO()
787-
write_func(fake_file)
788-
captured_data.update(json_mod.loads(fake_file.getvalue()))
789-
790-
mock_safe_write.side_effect = capture_safe_write
770+
captured_data, _, side_effect = make_capture_safe_write()
771+
mock_safe_write.side_effect = side_effect
791772

792773
# Call method
793774
result, msg = self.vehicle_components.save_component_templates(templates)
@@ -836,14 +817,8 @@ def test_save_component_templates_not_in_system( # type: ignore[misc] # pylint:
836817
mock_load_user.return_value = {}
837818

838819
# Capture and verify written data
839-
captured_data = {}
840-
841-
def capture_safe_write(_filepath, write_func) -> None:
842-
fake_file = io.StringIO()
843-
write_func(fake_file)
844-
captured_data.update(json_mod.loads(fake_file.getvalue()))
845-
846-
mock_safe_write.side_effect = capture_safe_write
820+
captured_data, _, side_effect = make_capture_safe_write()
821+
mock_safe_write.side_effect = side_effect
847822

848823
# Call method
849824
result, msg = self.vehicle_components.save_component_templates(templates)
@@ -977,18 +952,8 @@ def test_save_component_templates_empty_input( # type: ignore[misc] # pylint: d
977952
mock_load_user.return_value = {}
978953

979954
# Capture and verify written data
980-
captured_data: dict = {}
981-
wrote_empty = [False]
982-
983-
def capture_safe_write(_filepath, write_func) -> None:
984-
fake_file = io.StringIO()
985-
write_func(fake_file)
986-
value = fake_file.getvalue()
987-
captured_data.update(json_mod.loads(value))
988-
if json_mod.loads(value) == {}:
989-
wrote_empty[0] = True
990-
991-
mock_safe_write.side_effect = capture_safe_write
955+
captured_data, called, side_effect = make_capture_safe_write()
956+
mock_safe_write.side_effect = side_effect
992957

993958
# Call method with empty dictionary
994959
result, msg = self.vehicle_components.save_component_templates({})
@@ -997,7 +962,8 @@ def capture_safe_write(_filepath, write_func) -> None:
997962
assert not result # False means success
998963
assert msg == "/templates/user_vehicle_components_template.json"
999964
# Should save empty dict
1000-
assert wrote_empty[0], "Expected empty dict to be written"
965+
assert called[0], "Expected empty dict to be written"
966+
assert not captured_data, f"Expected empty dict, got {captured_data}"
1001967

1002968
@patch.object(VehicleComponents, "_load_system_templates")
1003969
@patch.object(VehicleComponents, "_load_user_templates")
@@ -1111,14 +1077,8 @@ def test_save_component_templates_preserves_existing_user_components( # type: i
11111077
mock_load_user.return_value = existing_user_templates
11121078

11131079
# Capture the write_func callback passed to safe_write to verify data
1114-
captured_data = {}
1115-
1116-
def capture_safe_write(_filepath, write_func) -> None:
1117-
fake_file = io.StringIO()
1118-
write_func(fake_file)
1119-
captured_data.update(json_mod.loads(fake_file.getvalue()))
1120-
1121-
mock_safe_write.side_effect = capture_safe_write
1080+
captured_data, _, side_effect = make_capture_safe_write()
1081+
mock_safe_write.side_effect = side_effect
11221082

11231083
# Save a new ESC template (Battery is NOT included in this call)
11241084
templates_to_save = {"ESC": [{"name": "BLHeli32 60A", "data": {"protocol": "DSHOT600"}, "is_user_modified": True}]}

0 commit comments

Comments
 (0)