Skip to content

Commit 3d35038

Browse files
committed
fix(backend filesystem): fixes test_backend_filesystem.py pylint
1 parent 47c87e7 commit 3d35038

File tree

1 file changed

+21
-14
lines changed

1 file changed

+21
-14
lines changed

tests/test_backend_filesystem.py

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
from subprocess import SubprocessError
1717
from unittest.mock import MagicMock, mock_open, patch
1818

19-
import pytest
20-
2119
from ardupilot_methodic_configurator.annotate_params import Par
2220
from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem, is_within_tolerance
2321

@@ -1082,13 +1080,13 @@ def test_user_can_skip_copying_vehicle_image_from_template(
10821080
@patch("ardupilot_methodic_configurator.backend_filesystem.os_path.join")
10831081
@patch("ardupilot_methodic_configurator.backend_filesystem.shutil_copy2")
10841082
@patch("ardupilot_methodic_configurator.backend_filesystem.os_path.isdir")
1085-
def test_copy_vehicle_image_defaults_to_false(self, mock_isdir, mock_copy2, mock_join, mock_listdir, mock_exists) -> None: # pylint: disable=unused-argument
1083+
def test_copy_vehicle_image_defaults_to_false(self, mock_isdir, mock_copy2, mock_join, mock_listdir, mock_exists) -> None:
10861084
"""
10871085
Vehicle image copying defaults to disabled.
10881086
10891087
GIVEN: A template directory containing a vehicle.jpg file
1090-
WHEN: The user creates a new vehicle without specifying copy_vehicle_image parameter
1091-
THEN: The vehicle.jpg file should not be copied by default
1088+
WHEN: The user creates a new vehicle with copy_vehicle_image=False
1089+
THEN: The vehicle.jpg file should not be copied
10921090
"""
10931091
# Arrange: Set up template directory with vehicle.jpg
10941092
mock_exists.side_effect = lambda path: path in ["template_dir", "new_vehicle_dir"]
@@ -1100,12 +1098,21 @@ def test_copy_vehicle_image_defaults_to_false(self, mock_isdir, mock_copy2, mock
11001098
"vehicle_dir", "vehicle_type", "", allow_editing_template_files=False, save_component_to_system_templates=False
11011099
)
11021100

1103-
# Act: Copy template files without specifying copy_vehicle_image
1104-
# (this should cause an error since parameter is required)
1105-
with pytest.raises(TypeError):
1106-
lfs.copy_template_files_to_new_vehicle_dir(
1107-
"template_dir", "new_vehicle_dir", blank_change_reason=False, copy_vehicle_image=False
1108-
)
1101+
# Act: Copy template files with copy_vehicle_image=False
1102+
result = lfs.copy_template_files_to_new_vehicle_dir(
1103+
"template_dir", "new_vehicle_dir", blank_change_reason=False, copy_vehicle_image=False
1104+
)
1105+
1106+
# Assert: No error and vehicle image should not be copied
1107+
assert result == "" # No error
1108+
1109+
# Verify vehicle.jpg was NOT copied
1110+
copy_calls = [call.args for call in mock_copy2.call_args_list]
1111+
vehicle_jpg_calls = [call for call in copy_calls if "vehicle.jpg" in str(call)]
1112+
assert len(vehicle_jpg_calls) == 0, "vehicle.jpg should not be copied when copy_vehicle_image=False"
1113+
1114+
# Verify other files were copied
1115+
mock_copy2.assert_any_call("template_dir/config.param", "new_vehicle_dir/config.param")
11091116

11101117
@patch("ardupilot_methodic_configurator.backend_filesystem.os_path.exists")
11111118
@patch("ardupilot_methodic_configurator.backend_filesystem.os_listdir")
@@ -1129,7 +1136,7 @@ def test_copy_vehicle_image_with_no_image_file_present(
11291136
mock_isdir.return_value = False
11301137

11311138
lfs = LocalFilesystem(
1132-
"vehicle_dir", "vehicle_type", None, allow_editing_template_files=False, save_component_to_system_templates=False
1139+
"vehicle_dir", "vehicle_type", "", allow_editing_template_files=False, save_component_to_system_templates=False
11331140
)
11341141

11351142
# Act: Copy template files with copy_vehicle_image=True (but no vehicle.jpg exists)
@@ -1146,7 +1153,7 @@ def test_copy_vehicle_image_with_no_image_file_present(
11461153
def test_merge_forced_or_derived_parameters_comprehensive() -> None:
11471154
"""Test merge_forced_or_derived_parameters with various scenarios."""
11481155
lfs = LocalFilesystem(
1149-
"vehicle_dir", "vehicle_type", None, allow_editing_template_files=False, save_component_to_system_templates=False
1156+
"vehicle_dir", "vehicle_type", "", allow_editing_template_files=False, save_component_to_system_templates=False
11501157
)
11511158
test_file = "test_file"
11521159

@@ -1214,7 +1221,7 @@ def test_merge_forced_or_derived_parameters_comprehensive() -> None:
12141221
def test_merge_forced_or_derived_parameters_none_parameters() -> None:
12151222
"""Test merge_forced_or_derived_parameters handles None parameters."""
12161223
lfs = LocalFilesystem(
1217-
"vehicle_dir", "vehicle_type", None, allow_editing_template_files=False, save_component_to_system_templates=False
1224+
"vehicle_dir", "vehicle_type", "", allow_editing_template_files=False, save_component_to_system_templates=False
12181225
)
12191226
test_file = "test.json"
12201227
lfs.file_parameters = {test_file: {}} # Test with empty dict instead of None (None is not valid type)

0 commit comments

Comments
 (0)