FIX: Only consider the file exists if size > 0. Only condider donwload complete if size > 0

Author: amilcarlucas

ARCHITECTURE.md

@@ -94,6 +94,8 @@ the following system design requirements were derived:
 - The software must provide feedback to the user, such as success or error messages, after each action.
 - The software must handle errors gracefully and provide clear error messages to the user.
 - The software must log events and errors for debugging and auditing purposes to the console.
+- if files are empty flag them as non-existing [PR #135](https://github.com/ArduPilot/MethodicConfigurator/pull/135)
+- if a downloaded file is empty flag it as download failed [PR #135](https://github.com/ArduPilot/MethodicConfigurator/pull/135)
 
 #### 6. Parameter File Management
 

ardupilot_methodic_configurator/backend_filesystem.py

@@ -292,12 +292,11 @@ def vehicle_configuration_file_exists(self, filename: str) -> bool:
           filename (str): The name of the file to check.
 
         Returns:
-          bool: True if the file exists and is a file (not a directory), False otherwise.
+          bool: True if the file exists and is a file (not a directory) and is not empty, False otherwise.
 
         """
-        return os_path.exists(os_path.join(self.vehicle_dir, filename)) and os_path.isfile(
-            os_path.join(self.vehicle_dir, filename)
-        )
+        file_path = os_path.join(self.vehicle_dir, filename)
+        return os_path.exists(file_path) and os_path.isfile(file_path) and os_path.getsize(file_path) > 0
 
     def __all_intermediate_parameter_file_comments(self) -> dict[str, str]:
         """

ardupilot_methodic_configurator/backend_internet.py

@@ -76,7 +76,7 @@ def download_file_from_url(
 
         if progress_callback:
             progress_callback(100.0, _("Download complete"))
-        return True
+        return bool(downloaded > 0)
 
     except requests_Timeout:
         logging_error(_("Download timed out"))

tests/test_backend_filesystem.py

@@ -110,15 +110,43 @@ def test_rename_parameter_files(self) -> None:
             expected_new = os_path.join("/mock/dir", "new_file.param")
             mock_rename.assert_called_once_with(expected_old, expected_new)
 
-    def test_vehicle_configuration_file_exists(self) -> None:
-        """Test checking if a specific configuration file exists."""
+    def test_vehicle_configuration_file_exists_comprehensive(self) -> None:
+        """Test checking if a specific configuration file exists with comprehensive path and size checks."""
         mock_vehicle_dir = "/mock/dir"
-        filesystem = LocalFilesystem(mock_vehicle_dir, "ArduCopter", "4.3.0", False)  # noqa: FBT003
+        filesystem = LocalFilesystem(mock_vehicle_dir, "ArduCopter", "4.3.0", allow_editing_template_files=False)
 
-        with patch("os.path.exists", return_value=True), patch("os.path.isfile", return_value=True):
+        # Test with all conditions met (file exists, is a file, and has size > 0)
+        with (
+            patch("os.path.exists", return_value=True) as mock_exists,
+            patch("os.path.isfile", return_value=True) as mock_isfile,
+            patch("os.path.getsize", return_value=100) as mock_getsize,
+            patch("os.path.join", side_effect=os_path.join) as mock_join,
+        ):
             assert filesystem.vehicle_configuration_file_exists("test.param")
+            mock_exists.assert_called_once()
+            mock_isfile.assert_called_once()
+            mock_getsize.assert_called_once()
+            mock_join.assert_called_once_with(mock_vehicle_dir, "test.param")
+
+        # Test with file that exists but is empty
+        with (
+            patch("os.path.exists", return_value=True),
+            patch("os.path.isfile", return_value=True),
+            patch("os.path.getsize", return_value=0),
+            patch("os.path.join", side_effect=os_path.join),
+        ):
+            assert not filesystem.vehicle_configuration_file_exists("empty.param")
+
+        # Test with directory instead of file
+        with (
+            patch("os.path.exists", return_value=True),
+            patch("os.path.isfile", return_value=False),
+            patch("os.path.join", side_effect=os_path.join),
+        ):
+            assert not filesystem.vehicle_configuration_file_exists("dir.param")
 
-        with patch("os.path.exists", return_value=False):
+        # Test with nonexistent file
+        with patch("os.path.exists", return_value=False), patch("os.path.join", side_effect=os_path.join):
             assert not filesystem.vehicle_configuration_file_exists("nonexistent.param")
 
     @patch("os.path.exists")

tests/test_backend_internet.py

@@ -93,7 +93,7 @@ def test_download_file_empty_response(mock_get) -> None:
     mock_response.headers = {"content-length": "0"}
     mock_response.iter_content.return_value = []
     mock_get.return_value = mock_response
-    assert download_file_from_url("http://test.com", "test.txt")
+    assert not download_file_from_url("http://test.com", "test.txt")
 
 
 @patch("ardupilot_methodic_configurator.backend_internet.requests_get")