diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index 572b147f..c57abf7a 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -48,8 +48,8 @@ def make_defaults_file_pipe(username=None, password=None): return None fields_message = ' and '.join( - field_name for field_name in - ( + field_name + for field_name in ( (f'username ({username})' if username is not None else None), ('password' if password is not None else None), ) @@ -58,9 +58,7 @@ def make_defaults_file_pipe(username=None, password=None): logger.debug(f'Writing database {fields_message} to defaults extra file pipe') read_file_descriptor, write_file_descriptor = os.pipe() - os.write( - write_file_descriptor, f'[client]\n{values}'.encode('utf-8') - ) + os.write(write_file_descriptor, f'[client]\n{values}'.encode('utf-8')) os.close(write_file_descriptor) # This plus subprocess.Popen(..., close_fds=False) in execute.py is necessary for the database @@ -88,7 +86,11 @@ def database_names_to_dump(database, config, username, password, environment, dr defaults_file_descriptor = make_defaults_file_pipe(username, password) show_command = ( mariadb_show_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -109,7 +111,15 @@ def database_names_to_dump(database, config, username, password, environment, dr def execute_dump_command( - database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label + database, + config, + username, + password, + dump_path, + database_names, + environment, + dry_run, + dry_run_label, ): ''' Kick off a dump for the given MariaDB database (provided as a configuration dict) to a named @@ -139,7 +149,11 @@ def execute_dump_command( defaults_file_descriptor = make_defaults_file_pipe(username, password) dump_command = ( mariadb_dump_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) @@ -200,10 +214,16 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) - password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + username = borgmatic.hooks.credential.parse.resolve_credential( + database.get('username'), config + ) + password = borgmatic.hooks.credential.parse.resolve_credential( + database.get('password'), config + ) environment = dict(os.environ) - dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) + dump_database_names = database_names_to_dump( + database, config, username, password, environment, dry_run + ) if not dump_database_names: if dry_run: @@ -329,7 +349,11 @@ def restore_data_source_dump( defaults_file_descriptor = make_defaults_file_pipe(username, password) restore_command = ( mariadb_restore_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 23175bd6..a31f7a54 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -42,10 +42,16 @@ def database_names_to_dump(database, config, username, password, environment, dr mysql_show_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) show_command = ( mysql_show_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['list_options'].split(' ')) if 'list_options' in database else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) @@ -66,7 +72,15 @@ def database_names_to_dump(database, config, username, password, environment, dr def execute_dump_command( - database, config, username, password, dump_path, database_names, environment, dry_run, dry_run_label + database, + config, + username, + password, + dump_path, + database_names, + environment, + dry_run, + dry_run_label, ): ''' Kick off a dump for the given MySQL/MariaDB database (provided as a configuration dict) to a @@ -92,10 +106,16 @@ def execute_dump_command( mysql_dump_command = tuple( shlex.quote(part) for part in shlex.split(database.get('mysql_dump_command') or 'mysqldump') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) dump_command = ( mysql_dump_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + (tuple(database['options'].split(' ')) if 'options' in database else ()) + (('--add-drop-database',) if database.get('add_drop_database', True) else ()) + (('--host', database['hostname']) if 'hostname' in database else ()) @@ -156,10 +176,16 @@ def dump_data_sources( for database in databases: dump_path = make_dump_path(borgmatic_runtime_directory) - username = borgmatic.hooks.credential.parse.resolve_credential(database.get('username'), config) - password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config) + username = borgmatic.hooks.credential.parse.resolve_credential( + database.get('username'), config + ) + password = borgmatic.hooks.credential.parse.resolve_credential( + database.get('password'), config + ) environment = dict(os.environ) - dump_database_names = database_names_to_dump(database, config, username, password, environment, dry_run) + dump_database_names = database_names_to_dump( + database, config, username, password, environment, dry_run + ) if not dump_database_names: if dry_run: @@ -282,10 +308,16 @@ def restore_data_source_dump( mysql_restore_command = tuple( shlex.quote(part) for part in shlex.split(data_source.get('mysql_command') or 'mysql') ) - defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe(username, password) + defaults_file_descriptor = borgmatic.hooks.data_source.mariadb.make_defaults_file_pipe( + username, password + ) restore_command = ( mysql_restore_command - + ((f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) if defaults_file_descriptor else ()) + + ( + (f'--defaults-extra-file=/dev/fd/{defaults_file_descriptor}',) + if defaults_file_descriptor + else () + ) + ('--batch',) + ( tuple(data_source['restore_options'].split(' ')) diff --git a/tests/unit/hooks/data_source/test_mariadb.py b/tests/unit/hooks/data_source/test_mariadb.py index 474c7d6d..6db363b8 100644 --- a/tests/unit/hooks/data_source/test_mariadb.py +++ b/tests/unit/hooks/data_source/test_mariadb.py @@ -9,7 +9,9 @@ from borgmatic.hooks.data_source import mariadb as module def test_database_names_to_dump_passes_through_name(): environment = flexmock() - names = module.database_names_to_dump({'name': 'foo'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'foo'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo',) @@ -18,7 +20,9 @@ def test_database_names_to_dump_bails_for_dry_run(): environment = flexmock() flexmock(module).should_receive('execute_command_and_capture_output').never() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=True) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=True + ) assert names == () @@ -28,12 +32,24 @@ def test_database_names_to_dump_queries_mariadb_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( - ('mariadb', '--skip-column-names', '--batch', '--execute', 'show schemas'), + ( + 'mariadb', + '--defaults-extra-file=/dev/fd/99', + '--skip-column-names', + '--batch', + '--execute', + 'show schemas', + ), environment=environment, ).and_return('foo\nbar\nmysql\n').once() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo', 'bar') @@ -53,6 +69,9 @@ def test_dump_data_sources_dumps_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' @@ -65,6 +84,8 @@ def test_dump_data_sources_dumps_each_database(): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -89,10 +110,10 @@ def test_dump_data_sources_dumps_with_password(): database = {'name': 'foo', 'username': 'root', 'password': 'trustsome1'} process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) ) @@ -100,9 +121,11 @@ def test_dump_data_sources_dumps_with_password(): flexmock(module).should_receive('execute_dump_command').with_args( database=database, config={}, + username='root', + password='trustsome1', dump_path=object, database_names=('foo',), - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, dry_run=object, dry_run_label=object, ).and_return(process).once() @@ -121,14 +144,16 @@ def test_dump_data_sources_dumps_all_databases_at_once(): databases = [{'name': 'all'}] process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) flexmock(module).should_receive('execute_dump_command').with_args( database={'name': 'all'}, config={}, + username=None, + password=None, dump_path=object, database_names=('foo', 'bar'), environment={'USER': 'root'}, @@ -150,16 +175,18 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured databases = [{'name': 'all', 'format': 'sql'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' - ).replace_with(lambda value, config: value) + ).and_return(None) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) for name, process in zip(('foo', 'bar'), processes): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name, 'format': 'sql'}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -181,11 +208,15 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mariadb_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-extra-file=mariadb.cnf'} + database = {'name': 'all', 'list_options': '--defaults-file=mariadb.cnf'} + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mariadb', - '--defaults-extra-file=mariadb.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=mariadb.cnf', '--skip-column-names', '--batch', '--execute', @@ -194,20 +225,27 @@ def test_database_names_to_dump_runs_mariadb_with_list_options(): environment=None, ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-extra-file=mariadb.cnf', + 'list_options': '--defaults-file=mariadb.cnf', 'mariadb_command': 'custom_mariadb', } + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mariadb', # Custom MariaDB command - '--defaults-extra-file=mariadb.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=mariadb.cnf', '--skip-column-names', '--batch', '--execute', @@ -215,7 +253,10 @@ def test_database_names_to_dump_runs_non_default_mariadb_with_list_options(): ), ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_execute_dump_command_runs_mariadb_dump(): @@ -225,11 +266,15 @@ def test_execute_dump_command_runs_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -244,6 +289,8 @@ def test_execute_dump_command_runs_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -261,11 +308,15 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--databases', 'foo', '--result-file', @@ -279,6 +330,8 @@ def test_execute_dump_command_runs_mariadb_dump_without_add_drop_database(): module.execute_dump_command( database={'name': 'foo', 'add_drop_database': False}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -296,11 +349,15 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--host', 'database.example.org', @@ -321,6 +378,8 @@ def test_execute_dump_command_runs_mariadb_dump_with_hostname_and_port(): module.execute_dump_command( database={'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -338,20 +397,22 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', - '--user', - 'root', '--databases', 'foo', '--result-file', 'dump', ), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, run_to_completion=False, ).and_return(process).once() @@ -359,9 +420,11 @@ def test_execute_dump_command_runs_mariadb_dump_with_username_and_password(): module.execute_dump_command( database={'name': 'foo', 'username': 'root', 'password': 'trustsome1'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, dry_run=False, dry_run_label='', ) @@ -376,11 +439,15 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mariadb-dump', + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -396,6 +463,8 @@ def test_execute_dump_command_runs_mariadb_dump_with_options(): module.execute_dump_command( database={'name': 'foo', 'options': '--stuff=such'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -413,11 +482,15 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'custom_mariadb_dump', # Custom MariaDB dump command + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -437,6 +510,8 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): 'options': '--stuff=such', }, # Custom MariaDB dump command specified config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -450,6 +525,9 @@ def test_execute_dump_command_runs_non_default_mariadb_dump_with_options(): def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -457,6 +535,8 @@ def test_execute_dump_command_with_duplicate_dump_skips_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -473,6 +553,9 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -481,6 +564,8 @@ def test_execute_dump_command_with_dry_run_skips_mariadb_dump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -685,13 +770,16 @@ def test_restore_data_source_dump_runs_mariadb_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'root', 'trustsome1' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mariadb', '--batch', '--user', 'root'), + ('mariadb', '--defaults-extra-file=/dev/fd/99', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -727,10 +815,14 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'cliusername', 'clipassword' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mariadb', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'clihost', @@ -738,13 +830,11 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ 'cliport', '--protocol', 'tcp', - '--user', - 'cliusername', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'clipassword'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -782,10 +872,14 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module).should_receive('make_defaults_file_pipe').with_args( + 'restoreuser', 'restorepass' + ).and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mariadb', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'restorehost', @@ -793,13 +887,11 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ 'restoreport', '--protocol', 'tcp', - '--user', - 'restoreuser', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'restorepass'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( diff --git a/tests/unit/hooks/data_source/test_mysql.py b/tests/unit/hooks/data_source/test_mysql.py index 5b4cbcc9..0fb3f432 100644 --- a/tests/unit/hooks/data_source/test_mysql.py +++ b/tests/unit/hooks/data_source/test_mysql.py @@ -9,7 +9,9 @@ from borgmatic.hooks.data_source import mysql as module def test_database_names_to_dump_passes_through_name(): environment = flexmock() - names = module.database_names_to_dump({'name': 'foo'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'foo'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo',) @@ -21,7 +23,9 @@ def test_database_names_to_dump_bails_for_dry_run(): ).replace_with(lambda value, config: value) flexmock(module).should_receive('execute_command_and_capture_output').never() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=True) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=True + ) assert names == () @@ -31,12 +35,24 @@ def test_database_names_to_dump_queries_mysql_for_database_names(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( - ('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'), + ( + 'mysql', + '--defaults-extra-file=/dev/fd/99', + '--skip-column-names', + '--batch', + '--execute', + 'show schemas', + ), environment=environment, ).and_return('foo\nbar\nmysql\n').once() - names = module.database_names_to_dump({'name': 'all'}, {}, environment, dry_run=False) + names = module.database_names_to_dump( + {'name': 'all'}, {}, 'root', 'trustsome1', environment, dry_run=False + ) assert names == ('foo', 'bar') @@ -56,6 +72,9 @@ def test_dump_data_sources_dumps_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) @@ -65,6 +84,8 @@ def test_dump_data_sources_dumps_each_database(): flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -89,10 +110,10 @@ def test_dump_data_sources_dumps_with_password(): database = {'name': 'foo', 'username': 'root', 'password': 'trustsome1'} process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') - flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( ('bar',) ) @@ -100,9 +121,11 @@ def test_dump_data_sources_dumps_with_password(): flexmock(module).should_receive('execute_dump_command').with_args( database=database, config={}, + username='root', + password='trustsome1', dump_path=object, database_names=('foo',), - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, dry_run=object, dry_run_label=object, ).and_return(process).once() @@ -121,11 +144,16 @@ def test_dump_data_sources_dumps_all_databases_at_once(): databases = [{'name': 'all'}] process = flexmock() flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) flexmock(module).should_receive('execute_dump_command').with_args( database={'name': 'all'}, config={}, + username=None, + password=None, dump_path=object, database_names=('foo', 'bar'), environment={'USER': 'root'}, @@ -147,6 +175,9 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured databases = [{'name': 'all', 'format': 'sql'}] processes = [flexmock(), flexmock()] flexmock(module).should_receive('make_dump_path').and_return('') + flexmock(module.borgmatic.hooks.credential.parse).should_receive( + 'resolve_credential' + ).and_return(None) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) @@ -154,6 +185,8 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured flexmock(module).should_receive('execute_dump_command').with_args( database={'name': name, 'format': 'sql'}, config={}, + username=None, + password=None, dump_path=object, database_names=(name,), environment={'USER': 'root'}, @@ -175,11 +208,15 @@ def test_dump_data_sources_dumps_all_databases_separately_when_format_configured def test_database_names_to_dump_runs_mysql_with_list_options(): - database = {'name': 'all', 'list_options': '--defaults-extra-file=my.cnf'} + database = {'name': 'all', 'list_options': '--defaults-file=my.cnf'} + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( ( 'mysql', - '--defaults-extra-file=my.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=my.cnf', '--skip-column-names', '--batch', '--execute', @@ -188,20 +225,27 @@ def test_database_names_to_dump_runs_mysql_with_list_options(): environment=None, ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_database_names_to_dump_runs_non_default_mysql_with_list_options(): database = { 'name': 'all', - 'list_options': '--defaults-extra-file=my.cnf', + 'list_options': '--defaults-file=my.cnf', 'mysql_command': 'custom_mysql', } + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module).should_receive('execute_command_and_capture_output').with_args( environment=None, full_command=( 'custom_mysql', # Custom MySQL command - '--defaults-extra-file=my.cnf', + '--defaults-extra-file=/dev/fd/99', + '--defaults-file=my.cnf', '--skip-column-names', '--batch', '--execute', @@ -209,7 +253,10 @@ def test_database_names_to_dump_runs_non_default_mysql_with_list_options(): ), ).and_return(('foo\nbar')).once() - assert module.database_names_to_dump(database, {}, None, '') == ('foo', 'bar') + assert module.database_names_to_dump(database, {}, 'root', 'trustsome1', None, '') == ( + 'foo', + 'bar', + ) def test_execute_dump_command_runs_mysqldump(): @@ -219,11 +266,15 @@ def test_execute_dump_command_runs_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -238,6 +289,8 @@ def test_execute_dump_command_runs_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -255,11 +308,15 @@ def test_execute_dump_command_runs_mysqldump_without_add_drop_database(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--databases', 'foo', '--result-file', @@ -273,6 +330,8 @@ def test_execute_dump_command_runs_mysqldump_without_add_drop_database(): module.execute_dump_command( database={'name': 'foo', 'add_drop_database': False}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -290,11 +349,15 @@ def test_execute_dump_command_runs_mysqldump_with_hostname_and_port(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--host', 'database.example.org', @@ -315,6 +378,8 @@ def test_execute_dump_command_runs_mysqldump_with_hostname_and_port(): module.execute_dump_command( database={'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -332,20 +397,22 @@ def test_execute_dump_command_runs_mysqldump_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', - '--user', - 'root', '--databases', 'foo', '--result-file', 'dump', ), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, run_to_completion=False, ).and_return(process).once() @@ -353,9 +420,11 @@ def test_execute_dump_command_runs_mysqldump_with_username_and_password(): module.execute_dump_command( database={'name': 'foo', 'username': 'root', 'password': 'trustsome1'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), - environment={'MYSQL_PWD': 'trustsome1'}, + environment={}, dry_run=False, dry_run_label='', ) @@ -370,11 +439,15 @@ def test_execute_dump_command_runs_mysqldump_with_options(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'mysqldump', + '--defaults-extra-file=/dev/fd/99', '--stuff=such', '--add-drop-database', '--databases', @@ -390,6 +463,8 @@ def test_execute_dump_command_runs_mysqldump_with_options(): module.execute_dump_command( database={'name': 'foo', 'options': '--stuff=such'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -407,11 +482,15 @@ def test_execute_dump_command_runs_non_default_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( ( 'custom_mysqldump', # Custom MySQL dump command + '--defaults-extra-file=/dev/fd/99', '--add-drop-database', '--databases', 'foo', @@ -429,6 +508,8 @@ def test_execute_dump_command_runs_non_default_mysqldump(): 'mysql_dump_command': 'custom_mysqldump', }, # Custom MySQL dump command specified config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -442,6 +523,9 @@ def test_execute_dump_command_runs_non_default_mysqldump(): def test_execute_dump_command_with_duplicate_dump_skips_mysqldump(): flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return('dump') flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump').never() flexmock(module).should_receive('execute_command').never() @@ -449,6 +533,8 @@ def test_execute_dump_command_with_duplicate_dump_skips_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -465,6 +551,9 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').never() @@ -473,6 +562,8 @@ def test_execute_dump_command_with_dry_run_skips_mysqldump(): module.execute_dump_command( database={'name': 'foo'}, config={}, + username='root', + password='trustsome1', dump_path=flexmock(), database_names=('foo',), environment=None, @@ -675,13 +766,16 @@ def test_restore_data_source_dump_runs_mysql_with_username_and_password(): flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('root', 'trustsome1').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( - ('mysql', '--batch', '--user', 'root'), + ('mysql', '--defaults-extra-file=/dev/fd/99', '--batch'), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'trustsome1'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -717,10 +811,14 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('cliusername', 'clipassword').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mysql', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'clihost', @@ -728,13 +826,11 @@ def test_restore_data_source_dump_with_connection_params_uses_connection_params_ 'cliport', '--protocol', 'tcp', - '--user', - 'cliusername', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'clipassword'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( @@ -772,10 +868,14 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ flexmock(module.borgmatic.hooks.credential.parse).should_receive( 'resolve_credential' ).replace_with(lambda value, config: value) + flexmock(module.borgmatic.hooks.data_source.mariadb).should_receive( + 'make_defaults_file_pipe' + ).with_args('restoreuser', 'restorepass').and_return(99) flexmock(module.os).should_receive('environ').and_return({'USER': 'root'}) flexmock(module).should_receive('execute_command_with_processes').with_args( ( 'mysql', + '--defaults-extra-file=/dev/fd/99', '--batch', '--host', 'restorehost', @@ -783,13 +883,11 @@ def test_restore_data_source_dump_without_connection_params_uses_restore_params_ 'restoreport', '--protocol', 'tcp', - '--user', - 'restoreuser', ), processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, - environment={'USER': 'root', 'MYSQL_PWD': 'restorepass'}, + environment={'USER': 'root'}, ).once() module.restore_data_source_dump( diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index 4d39fe3b..f3ef9413 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -191,7 +191,7 @@ def test_execute_command_calls_full_command(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -213,7 +213,7 @@ def test_execute_command_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stderr=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -234,7 +234,7 @@ def test_execute_command_calls_full_command_without_capturing_output(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) @@ -257,7 +257,7 @@ def test_execute_command_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -278,7 +278,7 @@ def test_execute_command_calls_full_command_with_shell(): shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -299,7 +299,7 @@ def test_execute_command_calls_full_command_with_environment(): shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -320,33 +320,12 @@ def test_execute_command_calls_full_command_with_working_directory(): shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(stdout=None)).once() - flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) - flexmock(module).should_receive('log_outputs') - - output = module.execute_command(full_command, working_directory='/working') - - assert output is None - - -def test_execute_command_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, - stdin=None, - stdout=module.subprocess.PIPE, - stderr=module.subprocess.STDOUT, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') - output = module.execute_command(full_command, environment={'BORG_PASSPHRASE_FD': '4'}) + output = module.execute_command(full_command, working_directory='/working') assert output is None @@ -363,7 +342,7 @@ def test_execute_command_without_run_to_completion_returns_process(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(process).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -377,11 +356,12 @@ def test_execute_command_and_capture_output_returns_stdout(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command) @@ -395,11 +375,12 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr() flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, capture_stderr=True) @@ -414,11 +395,12 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(1, full_command, err_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.SUCCESS @@ -435,11 +417,12 @@ def test_execute_command_and_capture_output_raises_when_command_errors(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(2, full_command, expected_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.ERROR @@ -455,11 +438,12 @@ def test_execute_command_and_capture_output_returns_output_with_shell(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( 'foo bar', + stdin=None, stderr=None, shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, shell=True) @@ -473,11 +457,12 @@ def test_execute_command_and_capture_output_returns_output_with_environment(): flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( @@ -493,37 +478,16 @@ def test_execute_command_and_capture_output_returns_output_with_working_director flexmock(module).should_receive('log_command') flexmock(module.subprocess).should_receive('check_output').with_args( full_command, + stdin=None, stderr=None, shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(decode=lambda: expected_output)).once() - - output = module.execute_command_and_capture_output( - full_command, shell=False, working_directory='/working' - ) - - assert output == expected_output - - -def test_execute_command_and_capture_output_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - expected_output = '[]' - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, - stderr=None, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( - full_command, - shell=False, - environment={'BORG_PASSPHRASE_FD': '4'}, + full_command, shell=False, working_directory='/working' ) assert output == expected_output @@ -541,7 +505,7 @@ def test_execute_command_with_processes_calls_full_command(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -564,7 +528,7 @@ def test_execute_command_with_processes_returns_output_with_output_log_level_non shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(process).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs').and_return({process: 'out'}) @@ -587,7 +551,7 @@ def test_execute_command_with_processes_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stderr=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -609,7 +573,7 @@ def test_execute_command_with_processes_calls_full_command_without_capturing_out shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) @@ -635,7 +599,7 @@ def test_execute_command_with_processes_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -657,7 +621,7 @@ def test_execute_command_with_processes_calls_full_command_with_shell(): shell=True, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -679,7 +643,7 @@ def test_execute_command_with_processes_calls_full_command_with_environment(): shell=False, env={'a': 'b'}, cwd=None, - close_fds=True, + close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -701,39 +665,13 @@ def test_execute_command_with_processes_calls_full_command_with_working_director shell=False, env=None, cwd='/working', - close_fds=True, - ).and_return(flexmock(stdout=None)).once() - flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) - flexmock(module).should_receive('log_outputs') - - output = module.execute_command_with_processes( - full_command, processes, working_directory='/working' - ) - - assert output is None - - -def test_execute_command_with_processes_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): - full_command = ['foo', 'bar'] - processes = (flexmock(),) - flexmock(module).should_receive('log_command') - flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, - stdin=None, - stdout=module.subprocess.PIPE, - stderr=module.subprocess.STDOUT, - shell=False, - env={'BORG_PASSPHRASE_FD': '4'}, - cwd=None, close_fds=False, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') output = module.execute_command_with_processes( - full_command, - processes, - environment={'BORG_PASSPHRASE_FD': '4'}, + full_command, processes, working_directory='/working' ) assert output is None @@ -754,7 +692,7 @@ def test_execute_command_with_processes_kills_processes_on_error(): shell=False, env=None, cwd=None, - close_fds=True, + close_fds=False, ).and_raise(subprocess.CalledProcessError(1, full_command, 'error')).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs').never()