From 675e54ba9f2b8001c11550aa6f46a7ab48f2de43 Mon Sep 17 00:00:00 2001
From: Divyansh Singh <divyanshanandsingh@gmail.com>
Date: Sat, 4 Mar 2023 12:43:07 +0530
Subject: [PATCH] use os.remove and improve tests

---
 borgmatic/hooks/sqlite.py       | 18 +++++++++++-------
 tests/unit/hooks/test_sqlite.py | 27 +++++++++++++++++++++++----
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/borgmatic/hooks/sqlite.py b/borgmatic/hooks/sqlite.py
index d31e331f..b4937e10 100644
--- a/borgmatic/hooks/sqlite.py
+++ b/borgmatic/hooks/sqlite.py
@@ -1,6 +1,5 @@
 import logging
 import os
-from subprocess import CalledProcessError
 
 from borgmatic.execute import execute_command, execute_command_with_processes
 from borgmatic.hooks import dump
@@ -29,11 +28,16 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
 
     logger.info('{}: Dumping SQLite databases{}'.format(log_prefix, dry_run_label))
 
-    if databases[0]['name'] == 'all':
-        logger.warning('The "all" database name has no meaning for SQLite3 databases')
-
     for database in databases:
         database_path = database['path']
+
+        if database['name'] == 'all':
+            logger.warning('The "all" database name has no meaning for SQLite3 databases')
+        if not os.path.exists(database_path):
+            logger.warning(
+                f'{log_prefix}: No SQLite database at {database_path}; An empty database will be created and dumped'
+            )
+
         dump_path = make_dump_path(location_config)
         dump_filename = dump.make_database_dump_filename(dump_path, database['name'])
         if os.path.exists(dump_filename):
@@ -101,9 +105,9 @@ def restore_database_dump(database_config, log_prefix, location_config, dry_run,
         return
 
     try:
-        execute_command(('rm', database_path), shell=True)
-    except CalledProcessError:  # pragma: no cover
-        logger.info(f'{log_prefix}: Database does not exist at {database_path}, skipping removal')
+        os.remove(database_path)
+    except FileNotFoundError:  # pragma: no cover
+        pass
 
     restore_command = (
         'sqlite3',
diff --git a/tests/unit/hooks/test_sqlite.py b/tests/unit/hooks/test_sqlite.py
index f9e09e66..d5050fe6 100644
--- a/tests/unit/hooks/test_sqlite.py
+++ b/tests/unit/hooks/test_sqlite.py
@@ -1,5 +1,3 @@
-import logging
-
 import pytest
 from flexmock import flexmock
 
@@ -14,6 +12,8 @@ def test_dump_databases_logs_and_skips_if_dump_already_exists():
         '/path/to/dump/database'
     )
     flexmock(module.os.path).should_receive('exists').and_return(True)
+    flexmock(module.dump).should_receive('create_parent_directory_for_dump').never()
+    flexmock(module).should_receive('execute_command').never()
 
     assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == []
 
@@ -38,6 +38,24 @@ def test_dump_databases_dumps_each_database():
     assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == processes
 
 
+def test_dumping_database_with_non_existent_path_warns_and_dumps_database():
+    databases = [
+        {'path': '/path/to/database1', 'name': 'database1'},
+    ]
+    processes = [flexmock()]
+
+    flexmock(module).should_receive('make_dump_path').and_return('/path/to/dump')
+    flexmock(module.logger).should_receive('warning').once()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        '/path/to/dump/database'
+    )
+    flexmock(module.os.path).should_receive('exists').and_return(False)
+    flexmock(module.dump).should_receive('create_parent_directory_for_dump')
+    flexmock(module).should_receive('execute_command').and_return(processes[0])
+
+    assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == processes
+
+
 def test_dumping_database_with_name_all_warns_and_dumps_all_databases():
     databases = [
         {'path': '/path/to/database1', 'name': 'all'},
@@ -45,7 +63,9 @@ def test_dumping_database_with_name_all_warns_and_dumps_all_databases():
     processes = [flexmock()]
 
     flexmock(module).should_receive('make_dump_path').and_return('/path/to/dump')
-    flexmock(logging).should_receive('warning')
+    flexmock(module.logger).should_receive(
+        'warning'
+    ).twice()  # once for the name=all, once for the non-existent path
     flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         '/path/to/dump/database'
     )
@@ -75,7 +95,6 @@ def test_restore_database_dump_restores_database():
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').once()
-    flexmock(module).should_receive('execute_command').once()
 
     module.restore_database_dump(
         database_config, 'test.yaml', {}, dry_run=False, extract_process=extract_process