Browse Source

use os.remove and improve tests

Divyansh Singh 2 years ago
parent
commit
675e54ba9f
2 changed files with 34 additions and 11 deletions
  1. 11 7
      borgmatic/hooks/sqlite.py
  2. 23 4
      tests/unit/hooks/test_sqlite.py

+ 11 - 7
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',

+ 23 - 4
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