Skip to content

Commit aceb8ca

Browse files
committed
fix(tests): update test mocks to use safe_write instead of builtins.open
safe_write uses tempfile.mkstemp + os.fdopen internally, so tests that mock builtins.open to inject errors (PermissionError, FileNotFoundError, etc.) were bypassed. Update all save-related tests across three test files to patch safe_write directly. Also fix pyright type error by changing write_func signature from Callable[[IO[str]], None] to Callable[[IO[str]], object] since f.write() returns int. Signed-off-by: Yash Goel <yashgoel892@gmail.com>
1 parent e2f8f9f commit aceb8ca

File tree

5 files changed

+100
-100
lines changed

5 files changed

+100
-100
lines changed

ardupilot_methodic_configurator/backend_safe_file_io.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from typing import IO, Callable
1616

1717

18-
def safe_write(filepath: str, write_func: Callable[[IO[str]], None]) -> None:
18+
def safe_write(filepath: str, write_func: Callable[[IO[str]], object]) -> None:
1919
"""
2020
Write to a temporary file, then atomically replace the target.
2121

tests/test_backend_filesystem_json_with_schema.py

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,9 @@ def test_user_can_save_valid_data_successfully(self, json_manager_with_schema) -
432432
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
433433
return_value="/output/directory/output.json",
434434
),
435-
patch("builtins.open", mock_open()) as mock_file,
435+
patch(
436+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
437+
) as mock_safe_write,
436438
patch(
437439
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.json_dumps",
438440
return_value='{"name": "Test Output", "count": 42}',
@@ -444,7 +446,7 @@ def test_user_can_save_valid_data_successfully(self, json_manager_with_schema) -
444446
# Assert: Data saved successfully
445447
assert error_occurred is False
446448
assert error_message == ""
447-
mock_file.assert_called_once_with("/output/directory/output.json", "w", encoding="utf-8", newline="\n")
449+
mock_safe_write.assert_called_once()
448450
mock_dumps.assert_called_once_with(valid_data, indent=4)
449451
# Assert: Internal cache updated immediately (regression test for commit ccc53bb)
450452
assert json_manager_with_schema.data == valid_data
@@ -501,7 +503,10 @@ def test_user_handles_missing_directory_error(self, json_manager_with_schema) ->
501503
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
502504
return_value="/nonexistent/directory/output.json",
503505
),
504-
patch("builtins.open", side_effect=FileNotFoundError),
506+
patch(
507+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
508+
side_effect=FileNotFoundError,
509+
),
505510
patch("ardupilot_methodic_configurator.backend_filesystem_json_with_schema.logging_error") as mock_error,
506511
):
507512
# Act: User attempts to save to missing directory
@@ -530,7 +535,10 @@ def test_user_handles_permission_denied_error(self, json_manager_with_schema) ->
530535
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
531536
return_value="/protected/directory/output.json",
532537
),
533-
patch("builtins.open", side_effect=PermissionError),
538+
patch(
539+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
540+
side_effect=PermissionError,
541+
),
534542
patch("ardupilot_methodic_configurator.backend_filesystem_json_with_schema.logging_error") as mock_error,
535543
):
536544
# Act: User attempts to save without permissions
@@ -559,7 +567,10 @@ def test_user_handles_directory_instead_of_file_error(self, json_manager_with_sc
559567
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
560568
return_value="/test/directory/output.json",
561569
),
562-
patch("builtins.open", side_effect=IsADirectoryError),
570+
patch(
571+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
572+
side_effect=IsADirectoryError,
573+
),
563574
patch("ardupilot_methodic_configurator.backend_filesystem_json_with_schema.logging_error") as mock_error,
564575
):
565576
# Act: User attempts to save to directory path
@@ -589,7 +600,10 @@ def test_user_handles_os_error_gracefully(self, json_manager_with_schema) -> Non
589600
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
590601
return_value="/test/directory/output.json",
591602
),
592-
patch("builtins.open", side_effect=os_error),
603+
patch(
604+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
605+
side_effect=os_error,
606+
),
593607
patch("ardupilot_methodic_configurator.backend_filesystem_json_with_schema.logging_error") as mock_error,
594608
):
595609
# Act: User encounters OS error
@@ -619,7 +633,6 @@ def test_user_handles_json_serialization_errors(self, json_manager_with_schema)
619633
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
620634
return_value="/test/directory/output.json",
621635
),
622-
patch("builtins.open", mock_open()),
623636
patch(
624637
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.json_dumps",
625638
side_effect=TypeError("Object not serializable"),
@@ -652,7 +665,6 @@ def test_user_handles_json_value_errors(self, json_manager_with_schema) -> None:
652665
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
653666
return_value="/test/directory/output.json",
654667
),
655-
patch("builtins.open", mock_open()),
656668
patch(
657669
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.json_dumps",
658670
side_effect=ValueError("Circular reference in data"),
@@ -687,7 +699,10 @@ def test_user_handles_unexpected_errors_gracefully(self, json_manager_with_schem
687699
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.os_path.join",
688700
return_value="/test/directory/output.json",
689701
),
690-
patch("builtins.open", side_effect=unexpected_error),
702+
patch(
703+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
704+
side_effect=unexpected_error,
705+
),
691706
patch("ardupilot_methodic_configurator.backend_filesystem_json_with_schema.logging_error") as mock_error,
692707
):
693708
# Act: User encounters unexpected error
@@ -731,10 +746,10 @@ def test_user_completes_full_workflow_successfully(self) -> None:
731746
),
732747
patch(
733748
"builtins.open",
734-
side_effect=[
735-
mock_open(read_data=json.dumps(schema_data)).return_value, # Schema file
736-
mock_open().return_value, # Data file for saving
737-
],
749+
return_value=mock_open(read_data=json.dumps(schema_data)).return_value, # Schema file
750+
),
751+
patch(
752+
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.safe_write",
738753
),
739754
patch(
740755
"ardupilot_methodic_configurator.backend_filesystem_json_with_schema.json_dumps",

tests/test_backend_filesystem_program_settings.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -578,14 +578,16 @@ def test_user_can_save_settings_to_file(self, mock_user_config) -> None:
578578
with (
579579
patch.object(ProgramSettings, "_user_config_dir", return_value=mock_user_config["config_dir"]),
580580
patch("os.path.join", return_value=mock_user_config["settings_file"]),
581-
patch("builtins.open", mock_open()) as mock_file,
581+
patch(
582+
"ardupilot_methodic_configurator.backend_filesystem_program_settings.safe_write",
583+
) as mock_safe_write,
582584
):
583585
# Act: Save settings to file
584586
ProgramSettings._set_settings_from_dict(mock_settings)
585587

586-
# Assert: File is opened correctly and data is written
587-
mock_file.assert_called_once_with(mock_user_config["settings_file"], "w", encoding="utf-8", newline="\n")
588-
mock_file().write.assert_called()
588+
# Assert: safe_write is called with the correct file path
589+
mock_safe_write.assert_called_once()
590+
assert mock_safe_write.call_args[0][0] == mock_user_config["settings_file"]
589591

590592

591593
class TestUsagePopupSettings:

0 commit comments

Comments
 (0)