From 766bcddedeb773c004dc9023ff72d13e979f93e6 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 12:40:51 +0100 Subject: [PATCH 01/20] Using tmp_path and mock the remote path using tmp_path_factory --- tests/transports/test_all_plugins.py | 138 +++++++++------------------ 1 file changed, 44 insertions(+), 94 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 0a25add0a7..d697af10f5 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -33,8 +33,14 @@ # TODO : silly cases of copy/put/get from self to self +@pytest.fixture(scope='function') +def remote_tmp_path(tmp_path_factory): + """Mock the remote tmp path using tmp_path_factory to create folder start with 'remote'""" + return tmp_path_factory.mktemp('remote') + + @pytest.fixture(scope='function', params=entry_point.get_entry_point_names('aiida.transports')) -def custom_transport(request, tmp_path, monkeypatch) -> Transport: +def custom_transport(request, tmp_path_factory, monkeypatch) -> Transport: """Fixture that parametrizes over all the registered implementations of the ``CommonRelaxWorkChain``.""" plugin = TransportFactory(request.param) @@ -42,7 +48,7 @@ def custom_transport(request, tmp_path, monkeypatch) -> Transport: kwargs = {'machine': 'localhost', 'timeout': 30, 'load_system_host_keys': True, 'key_policy': 'AutoAddPolicy'} elif request.param == 'core.ssh_auto': kwargs = {'machine': 'localhost'} - filepath_config = tmp_path / 'config' + filepath_config = tmp_path_factory.mktemp('transport') / 'config' monkeypatch.setattr(plugin, 'FILEPATH_CONFIG', filepath_config) if not filepath_config.exists(): filepath_config.write_text('Host localhost') @@ -62,26 +68,24 @@ def test_is_open(custom_transport): assert not custom_transport.is_open -def test_makedirs(custom_transport): - """Verify the functioning of makedirs command""" +def test_deprecate_chdir_and_getcwd(custom_transport, remote_tmp_path): + """Test to be deprecated ``chdir``/``getcwd`` methods still work.""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' + location = str(remote_tmp_path) transport.chdir(location) assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) - transport.chdir(directory) + +def test_makedirs(custom_transport, remote_tmp_path): + """Verify the functioning of makedirs command""" + with custom_transport as transport: # define folder structure - dir_tree = os.path.join('1', '2') + dir_tree = str(remote_tmp_path / '1' / '2') # I create the tree transport.makedirs(dir_tree) # verify the existence - assert transport.isdir('1') + assert transport.isdir(str(remote_tmp_path / '1')) assert dir_tree # try to recreate the same folder @@ -92,92 +96,57 @@ def test_makedirs(custom_transport): transport.makedirs(dir_tree, True) transport.rmdir(dir_tree) - transport.rmdir('1') - - transport.chdir('..') - transport.rmdir(directory) + transport.rmdir(str(remote_tmp_path / '1')) -def test_rmtree(custom_transport): +def test_rmtree(custom_transport, remote_tmp_path): """Verify the functioning of rmtree command""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) - transport.chdir(directory) - # define folder structure - dir_tree = os.path.join('1', '2') + dir_tree = str(remote_tmp_path / '1' / '2') # I create the tree transport.makedirs(dir_tree) # remove it - transport.rmtree('1') + transport.rmtree(str(remote_tmp_path / '1')) # verify the removal - assert not transport.isdir('1') + assert not transport.isdir(str(remote_tmp_path / '1')) # also tests that it works with a single file # create file local_file_name = 'file.txt' text = 'Viva Verdi\n' - with open(os.path.join(transport.getcwd(), local_file_name), 'w', encoding='utf8') as fhandle: + single_file_path = str(remote_tmp_path / local_file_name) + with open(single_file_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) # remove it - transport.rmtree(local_file_name) + transport.rmtree(single_file_path) # verify the removal - assert not transport.isfile(local_file_name) + assert not transport.isfile(single_file_path) - transport.chdir('..') - transport.rmdir(directory) - -def test_listdir(custom_transport): +def test_listdir(custom_transport, remote_tmp_path): """Create directories, verify listdir, delete a folder with subfolders""" - with custom_transport as trans: - # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - trans.chdir(location) - - assert location == trans.getcwd() - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + with custom_transport as transport: list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - trans.mkdir(this_dir) + transport.mkdir(str(remote_tmp_path / this_dir)) + for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(remote_tmp_path / fname)) - list_found = trans.listdir('.') + list_found = transport.listdir(str(remote_tmp_path)) assert sorted(list_found) == sorted(list_of_dir + list_of_files) - assert sorted(trans.listdir('.', 'a*')), sorted(['as', 'a2', 'a4f']) - assert sorted(trans.listdir('.', 'a?')), sorted(['as', 'a2']) - assert sorted(trans.listdir('.', 'a[2-4]*')), sorted(['a2', 'a4f']) + assert sorted(transport.listdir(str(remote_tmp_path), 'a*')), sorted(['as', 'a2', 'a4f']) + assert sorted(transport.listdir(str(remote_tmp_path), 'a?')), sorted(['as', 'a2']) + assert sorted(transport.listdir(str(remote_tmp_path), 'a[2-4]*')), sorted(['a2', 'a4f']) - for this_dir in list_of_dir: - trans.rmdir(this_dir) - - for this_file in list_of_files: - trans.remove(this_file) - - trans.chdir('..') - trans.rmdir(directory) - -def test_listdir_withattributes(custom_transport): +def test_listdir_withattributes(custom_transport, remote_tmp_path): """Create directories, verify listdir_withattributes, delete a folder with subfolders""" def simplify_attributes(data): @@ -189,49 +158,30 @@ def simplify_attributes(data): """ return {_['name']: _['isdir'] for _ in data} - with custom_transport as trans: - # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - trans.chdir(location) - - assert location == trans.getcwd() - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + with custom_transport as transport: list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - trans.mkdir(this_dir) + transport.mkdir(str(remote_tmp_path / this_dir)) + for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(remote_tmp_path / fname)) comparison_list = {k: True for k in list_of_dir} for k in list_of_files: comparison_list[k] = False - assert simplify_attributes(trans.listdir_withattributes('.')), comparison_list - assert simplify_attributes(trans.listdir_withattributes('.', 'a*')), { + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path))), comparison_list + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a*')), { 'as': True, 'a2': True, 'a4f': True, 'a': False, } - assert simplify_attributes(trans.listdir_withattributes('.', 'a?')), {'as': True, 'a2': True} - assert simplify_attributes(trans.listdir_withattributes('.', 'a[2-4]*')), {'a2': True, 'a4f': True} - - for this_dir in list_of_dir: - trans.rmdir(this_dir) - - for this_file in list_of_files: - trans.remove(this_file) - - trans.chdir('..') - trans.rmdir(directory) + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a?')), {'as': True, 'a2': True} + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a[2-4]*')), {'a2': True, 'a4f': True} def test_dir_creation_deletion(custom_transport): From c626f75624317a6ff05cd25913b20870ce8632cf Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 12:50:38 +0100 Subject: [PATCH 02/20] test_dir_copy --- tests/transports/test_all_plugins.py | 54 ++++++++++++---------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index d697af10f5..3f73fbe169 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -180,57 +180,47 @@ def simplify_attributes(data): 'a4f': True, 'a': False, } - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a?')), {'as': True, 'a2': True} - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a[2-4]*')), {'a2': True, 'a4f': True} + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a?')), { + 'as': True, + 'a2': True, + } + assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a[2-4]*')), { + 'a2': True, + 'a4f': True, + } -def test_dir_creation_deletion(custom_transport): +def test_dir_creation_deletion(custom_transport, remote_tmp_path): """Test creating and deleting directories.""" with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - assert location == transport.getcwd() - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) + new_dir = str(remote_tmp_path / 'new') + transport.mkdir(new_dir) with pytest.raises(OSError): # I create twice the same directory - transport.mkdir(directory) + transport.mkdir(new_dir) - transport.isdir(directory) - assert not transport.isfile(directory) - transport.rmdir(directory) + transport.isdir(new_dir) + assert not transport.isfile(new_dir) -def test_dir_copy(custom_transport): +def test_dir_copy(custom_transport, remote_tmp_path): """Verify if in the copy of a directory also the protection bits are carried over """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) + # Create a src dir + src_dir = str(remote_tmp_path / 'copy_src') + transport.mkdir(src_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - transport.mkdir(directory) - - dest_directory = f'{directory}_copy' - transport.copy(directory, dest_directory) + dst_dir = str(remote_tmp_path / 'copy_dst') + transport.copy(src_dir, dst_dir) with pytest.raises(ValueError): - transport.copy(directory, '') + transport.copy(src_dir, '') with pytest.raises(ValueError): - transport.copy('', directory) - - transport.rmdir(directory) - transport.rmdir(dest_directory) + transport.copy('', dst_dir) def test_dir_permissions_creation_modification(custom_transport): From 77e0074eb773a2c084c225d4a6604de03f6c5ca9 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 16:04:35 +0100 Subject: [PATCH 03/20] more checkpoint More checkpoint --- tests/transports/test_all_plugins.py | 110 +++++++++++---------------- 1 file changed, 44 insertions(+), 66 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 3f73fbe169..756d938036 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -77,6 +77,18 @@ def test_deprecate_chdir_and_getcwd(custom_transport, remote_tmp_path): assert location == transport.getcwd() +def test_chdir_to_empty_string(custom_transport, remote_tmp_path): + """I check that if I pass an empty string to chdir, the cwd does + not change (this is a paramiko default behavior), but getcwd() + is still correctly defined. + """ + with custom_transport as transport: + new_dir = str(remote_tmp_path) + transport.chdir(new_dir) + transport.chdir('') + assert new_dir == transport.getcwd() + + def test_makedirs(custom_transport, remote_tmp_path): """Verify the functioning of makedirs command""" with custom_transport as transport: @@ -223,21 +235,14 @@ def test_dir_copy(custom_transport, remote_tmp_path): transport.copy('', dst_dir) -def test_dir_permissions_creation_modification(custom_transport): +def test_dir_permissions_creation_modification(custom_transport, remote_tmp_path): """Verify if chmod raises OSError when trying to change bits on a non-existing folder """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) + directory = str(remote_tmp_path / 'test') - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - - # create directory with non default permissions - transport.mkdir(directory) + transport.makedirs(directory) # change permissions transport.chmod(directory, 0o777) @@ -274,18 +279,12 @@ def test_dir_permissions_creation_modification(custom_transport): transport.rmdir(directory) -def test_dir_reading_permissions(custom_transport): +def test_dir_reading_permissions(custom_transport, remote_tmp_path): """Try to enter a directory with no read permissions. Verify that the cwd has not changed after failed try. """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - directory = 'temp_dir_test' - transport.chdir(location) - - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + directory = str(remote_tmp_path / 'test') # create directory with non default permissions transport.mkdir(directory) @@ -305,92 +304,71 @@ def test_dir_reading_permissions(custom_transport): assert old_cwd == new_cwd - # TODO : the test leaves a directory even if it is successful - # The bug is in paramiko. After lowering the permissions, - # I cannot restore them to higher values - # transport.rmdir(directory) - def test_isfile_isdir_to_empty_string(custom_transport): """I check that isdir or isfile return False when executed on an empty string """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(location) assert not transport.isdir('') assert not transport.isfile('') -def test_isfile_isdir_to_non_existing_string(custom_transport): +def test_isfile_isdir_to_non_existing_string(custom_transport, remote_tmp_path): """I check that isdir or isfile return False when executed on an empty string """ with custom_transport as transport: - location = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(location) - fake_folder = 'pippo' + fake_folder = str(remote_tmp_path / 'pippo') assert not transport.isfile(fake_folder) assert not transport.isdir(fake_folder) with pytest.raises(OSError): transport.chdir(fake_folder) -def test_chdir_to_empty_string(custom_transport): - """I check that if I pass an empty string to chdir, the cwd does - not change (this is a paramiko default behavior), but getcwd() - is still correctly defined. - """ - with custom_transport as transport: - new_dir = transport.normalize(os.path.join('/', 'tmp')) - transport.chdir(new_dir) - transport.chdir('') - assert new_dir == transport.getcwd() - - -def test_put_and_get_file(custom_transport): +def test_put_and_get_file(custom_transport, tmp_path_factory): """Test putting and getting files.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + # transport.chdir(remote_dir) + # while transport.isdir(directory): + # # I append a random letter/number until it is unique + # directory += random.choice(string.ascii_uppercase + string.digits) + # + # transport.chdir(directory) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - local_file_name = os.path.join(local_dir, directory, 'file.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') + + # here use full path in src and dst + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # here use full path in src and dst - transport.put(local_file_name, remote_file_name) - transport.get(remote_file_name, retrieved_file_name) - transport.putfile(local_file_name, remote_file_name) - transport.getfile(remote_file_name, retrieved_file_name) + transport.put(local_file_abs_path, remote_file_abs_path) + transport.get(remote_file_abs_path, retrieved_file_abs_path) + # transport.putfile(local_file_abs_path, remote_file_abs_path) + # transport.getfile(remote_file_name, retrieved_file_name) - list_of_files = transport.listdir('.') + list_of_files = transport.listdir(str(remote_dir / directory)) # it is False because local_file_name has the full path, # while list_of_files has not assert local_file_name not in list_of_files assert remote_file_name in list_of_files assert retrieved_file_name not in list_of_files - os.remove(local_file_name) - transport.remove(remote_file_name) - os.remove(retrieved_file_name) - - transport.chdir('..') - transport.rmdir(directory) - def test_put_get_abs_path_file(custom_transport): """Test of exception for non existing files and abs path""" From 5b6a82d54e6f61dc287c0bc55a9c13fb94093bad Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 16:38:45 +0100 Subject: [PATCH 04/20] Seperate put/get and putfile/getfile --- tests/transports/test_all_plugins.py | 47 ++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 756d938036..c4d2d3a87f 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -326,7 +326,7 @@ def test_isfile_isdir_to_non_existing_string(custom_transport, remote_tmp_path): transport.chdir(fake_folder) -def test_put_and_get_file(custom_transport, tmp_path_factory): +def test_put_and_get(custom_transport, tmp_path_factory): """Test putting and getting files.""" local_dir = tmp_path_factory.mktemp('local') remote_dir = tmp_path_factory.mktemp('remote') @@ -334,12 +334,6 @@ def test_put_and_get_file(custom_transport, tmp_path_factory): directory = 'tmp_try' with custom_transport as transport: - # transport.chdir(remote_dir) - # while transport.isdir(directory): - # # I append a random letter/number until it is unique - # directory += random.choice(string.ascii_uppercase + string.digits) - # - # transport.chdir(directory) (local_dir / directory).mkdir() transport.mkdir(str(remote_dir / directory)) @@ -359,8 +353,41 @@ def test_put_and_get_file(custom_transport, tmp_path_factory): transport.put(local_file_abs_path, remote_file_abs_path) transport.get(remote_file_abs_path, retrieved_file_abs_path) - # transport.putfile(local_file_abs_path, remote_file_abs_path) - # transport.getfile(remote_file_name, retrieved_file_name) + + list_of_files = transport.listdir(str(remote_dir / directory)) + # it is False because local_file_name has the full path, + # while list_of_files has not + assert local_file_name not in list_of_files + assert remote_file_name in list_of_files + assert retrieved_file_name not in list_of_files + +def test_putfile_and_getfile(custom_transport, tmp_path_factory): + """Test putting and getting files.""" + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') + + directory = 'tmp_try' + + with custom_transport as transport: + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) + + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' + + remote_file_name = 'file_remote.txt' + + # here use full path in src and dst + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) + + text = 'Viva Verdi\n' + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: + fhandle.write(text) + + transport.putfile(local_file_abs_path, remote_file_abs_path) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) list_of_files = transport.listdir(str(remote_dir / directory)) # it is False because local_file_name has the full path, @@ -370,7 +397,7 @@ def test_put_and_get_file(custom_transport, tmp_path_factory): assert retrieved_file_name not in list_of_files -def test_put_get_abs_path_file(custom_transport): +def test_put_get_abs_path_file(custom_transport, tmp_path_factory): """Test of exception for non existing files and abs path""" local_dir = os.path.join('/', 'tmp') remote_dir = local_dir From c64bb05eb2ab4b948919ca4a65715d1cc6b6146c Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 16:58:35 +0100 Subject: [PATCH 05/20] Fix the exception type and docstring of local plugin --- src/aiida/transports/plugins/local.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/aiida/transports/plugins/local.py b/src/aiida/transports/plugins/local.py index 755476a066..8de49838e3 100644 --- a/src/aiida/transports/plugins/local.py +++ b/src/aiida/transports/plugins/local.py @@ -453,8 +453,8 @@ def getfile(self, remotepath, localpath, *args, **kwargs): """Copies a file recursively from 'remote' remotepath to 'local' localpath. - :param remotepath: path to local file - :param localpath: absolute path to remote file + :param remotepath: absolute path to remote file + :param localpath: path to local file :param overwrite: if True overwrites localpath. Default = False @@ -462,6 +462,9 @@ def getfile(self, remotepath, localpath, *args, **kwargs): :raise ValueError: if 'local' localpath is not valid :raise OSError: if unintentionally overwriting """ + if not os.path.isabs(localpath): + raise ValueError('localpath must be an absolute path') + overwrite = kwargs.get('overwrite', args[0] if args else True) if not localpath: raise ValueError('Input localpath to get function must be a non empty string') From f19067cea002ff782cb1ae5d482b1ac07afdc03f Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 16:58:44 +0100 Subject: [PATCH 06/20] more checkpoints --- tests/transports/test_all_plugins.py | 116 ++++++++++++--------------- 1 file changed, 51 insertions(+), 65 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index c4d2d3a87f..2403b8b5b6 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -361,6 +361,7 @@ def test_put_and_get(custom_transport, tmp_path_factory): assert remote_file_name in list_of_files assert retrieved_file_name not in list_of_files + def test_putfile_and_getfile(custom_transport, tmp_path_factory): """Test putting and getting files.""" local_dir = tmp_path_factory.mktemp('local') @@ -399,129 +400,114 @@ def test_putfile_and_getfile(custom_transport, tmp_path_factory): def test_put_get_abs_path_file(custom_transport, tmp_path_factory): """Test of exception for non existing files and abs path""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - partial_file_name = 'file.txt' - local_file_name = os.path.join(local_dir, directory, 'file.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') - pathlib.Path(local_file_name).touch() + local_file_rel_path = local_file_name + local_file_abs_path = str(local_dir / directory / local_file_name) + + remote_file_rel_path = remote_file_name + + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) # partial_file_name is not an abs path with pytest.raises(ValueError): - transport.put(partial_file_name, remote_file_name) + transport.put(local_file_rel_path, remote_file_abs_path) with pytest.raises(ValueError): - transport.putfile(partial_file_name, remote_file_name) + transport.putfile(local_file_rel_path, remote_file_abs_path) # retrieved_file_name does not exist with pytest.raises(OSError): - transport.put(retrieved_file_name, remote_file_name) + transport.put(retrieved_file_abs_path, remote_file_abs_path) with pytest.raises(OSError): - transport.putfile(retrieved_file_name, remote_file_name) + transport.putfile(retrieved_file_abs_path, remote_file_abs_path) # remote_file_name does not exist with pytest.raises(OSError): - transport.get(remote_file_name, retrieved_file_name) + transport.get(remote_file_abs_path, retrieved_file_abs_path) with pytest.raises(OSError): - transport.getfile(remote_file_name, retrieved_file_name) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) - transport.put(local_file_name, remote_file_name) - transport.putfile(local_file_name, remote_file_name) - - # local filename is not an abs path + # remote filename is not an abs path with pytest.raises(ValueError): - transport.get(remote_file_name, 'delete_me.txt') + transport.get(remote_file_rel_path, 'delete_me.txt') with pytest.raises(ValueError): - transport.getfile(remote_file_name, 'delete_me.txt') + transport.getfile(remote_file_rel_path, 'delete_me.txt') - transport.remove(remote_file_name) - os.remove(local_file_name) - transport.chdir('..') - transport.rmdir(directory) - - -def test_put_get_empty_string_file(custom_transport): +def test_put_get_empty_string_file(custom_transport, tmp_path_factory): """Test of exception put/get of empty strings""" - # TODO : verify the correctness of \n at the end of a file - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - while transport.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + (local_dir / directory).mkdir() + transport.mkdir(str(remote_dir / directory)) - transport.mkdir(directory) - transport.chdir(directory) + local_file_name = 'file.txt' + retrieved_file_name = 'file_retrieved.txt' - local_file_name = os.path.join(local_dir, directory, 'file_local.txt') remote_file_name = 'file_remote.txt' - retrieved_file_name = os.path.join(local_dir, directory, 'file_retrieved.txt') + + # here use full path in src and dst + local_file_abs_path = str(local_dir / directory / local_file_name) + retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) + remote_file_abs_path = str(remote_dir / directory / remote_file_name) text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) # localpath is an empty string # ValueError because it is not an abs path with pytest.raises(ValueError): - transport.put('', remote_file_name) + transport.put('', remote_file_abs_path) with pytest.raises(ValueError): - transport.putfile('', remote_file_name) + transport.putfile('', remote_file_abs_path) # remote path is an empty string with pytest.raises(OSError): - transport.put(local_file_name, '') + transport.put(local_file_abs_path, '') with pytest.raises(OSError): - transport.putfile(local_file_name, '') + transport.putfile(local_file_abs_path, '') - transport.put(local_file_name, remote_file_name) + transport.put(local_file_abs_path, remote_file_abs_path) # overwrite the remote_file_name - transport.putfile(local_file_name, remote_file_name) + transport.putfile(local_file_abs_path, remote_file_abs_path) # remote path is an empty string with pytest.raises(OSError): - transport.get('', retrieved_file_name) + transport.get('', retrieved_file_abs_path) with pytest.raises(OSError): - transport.getfile('', retrieved_file_name) + transport.getfile('', retrieved_file_abs_path) # local path is an empty string # ValueError because it is not an abs path with pytest.raises(ValueError): - transport.get(remote_file_name, '') + transport.get(remote_file_abs_path, '') with pytest.raises(ValueError): - transport.getfile(remote_file_name, '') + transport.getfile(remote_file_abs_path, '') # TODO : get doesn't retrieve empty files. # Is it what we want? - transport.get(remote_file_name, retrieved_file_name) + transport.get(remote_file_abs_path, retrieved_file_abs_path) # overwrite retrieved_file_name - transport.getfile(remote_file_name, retrieved_file_name) - - os.remove(local_file_name) - transport.remove(remote_file_name) - # If it couldn't end the copy, it leaves what he did on - # local file - assert 'file_retrieved.txt' in transport.listdir('.') - os.remove(retrieved_file_name) - - transport.chdir('..') - transport.rmdir(directory) + transport.getfile(remote_file_abs_path, retrieved_file_abs_path) def test_put_and_get_tree(custom_transport): From 633d263bc2096d1f2a35e911deafead1c96c2273 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 17:08:46 +0100 Subject: [PATCH 07/20] test_exec_pwd --- tests/transports/test_all_plugins.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 2403b8b5b6..1848400c2f 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -1008,9 +1008,9 @@ def test_gettree_nested_directory(custom_transport): transport.gettree(os.path.join(dir_remote, 'sub/path'), os.path.join(dir_local, 'sub/path')) -def test_exec_pwd(custom_transport): +def test_exec_pwd(custom_transport, remote_tmp_path): """I create a strange subfolder with a complicated name and - then see if I can run pwd. This also checks the correct + then see if I can run ``ls``. This also checks the correct escaping of funny characters, both in the directory creation (which should be done by paramiko) and in the command execution (done in this module, in the _exec_command_internal function). @@ -1022,29 +1022,21 @@ def test_exec_pwd(custom_transport): # To compare with: getcwd uses the normalized ('realpath') path location = transport.normalize('/tmp') subfolder = """_'s f"#""" # A folder with characters to escape - subfolder_fullpath = os.path.join(location, subfolder) + subfolder_fullpath = str(remote_tmp_path / subfolder) - transport.chdir(location) - if not transport.isdir(subfolder): + if not transport.isdir(subfolder_fullpath): # Since I created the folder, I will remember to # delete it at the end of this test delete_at_end = True - transport.mkdir(subfolder) + transport.mkdir(subfolder_fullpath) - assert transport.isdir(subfolder) - transport.chdir(subfolder) + assert transport.isdir(subfolder_fullpath) - assert subfolder_fullpath == transport.getcwd() - retcode, stdout, stderr = transport.exec_command_wait('pwd') + retcode, stdout, stderr = transport.exec_command_wait(f'ls {str(remote_tmp_path)}') assert retcode == 0 - # I have to strip it because 'pwd' returns a trailing \n - assert stdout.strip() == subfolder_fullpath + assert stdout.strip() in subfolder_fullpath assert stderr == '' - if delete_at_end: - transport.chdir(location) - transport.rmdir(subfolder) - def test_exec_with_stdin_string(custom_transport): """Test command execution with a stdin string.""" From eedd3901ad97ea4b04bbdd5af713ba60724055cd Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 18:31:10 +0100 Subject: [PATCH 08/20] test_transfer_big_stdout --- tests/transports/test_all_plugins.py | 38 +++++++++++----------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 1848400c2f..f54a659c0b 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -1117,7 +1117,7 @@ def test_exec_with_wrong_stdin(custom_transport): transport.exec_command_wait('cat', stdin=1) -def test_transfer_big_stdout(custom_transport): +def test_transfer_big_stdout(custom_transport, tmp_path): """Test the transfer of a large amount of data on stdout.""" # Create a "big" file of > 2MB (10MB here; in general, larger than the buffer size) min_file_size_bytes = 5 * 1024 * 1024 @@ -1132,25 +1132,19 @@ def test_transfer_big_stdout(custom_transport): line_repetitions = min_file_size_bytes // len(file_line_binary) + 1 fcontent = (file_line_binary * line_repetitions).decode('utf8') - with custom_transport as trans: + with custom_transport as transport: + transport: Transport # We cannot use tempfile.mkdtemp because we're on a remote folder - location = trans.normalize(os.path.join('/', 'tmp')) - trans.chdir(location) - assert location == trans.getcwd() - - directory = 'temp_dir_test_transfer_big_stdout' - while trans.isdir(directory): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - trans.mkdir(directory) - trans.chdir(directory) + directory_name = 'temp_dir_test_transfer_big_stdout' + directory_path = tmp_path / directory_name + transport.mkdir(str(directory_path)) with tempfile.NamedTemporaryFile(mode='wb') as tmpf: tmpf.write(fcontent.encode('utf8')) tmpf.flush() # I put a file with specific content there at the right file name - trans.putfile(tmpf.name, fname) + transport.putfile(tmpf.name, str(directory_path / fname)) python_code = r"""import sys @@ -1174,16 +1168,20 @@ def test_transfer_big_stdout(custom_transport): tmpf.flush() # I put a file with specific content there at the right file name - trans.putfile(tmpf.name, script_fname) + transport.putfile(tmpf.name, str(directory_path / script_fname)) # I get its content via the stdout; emulate also network slowness (note I cat twice) - retcode, stdout, stderr = trans.exec_command_wait(f'cat {fname} ; sleep 1 ; cat {fname}') + retcode, stdout, stderr = transport.exec_command_wait( + f'cat {fname} ; sleep 1 ; cat {fname}', workdir=str(directory_path) + ) assert stderr == '' assert stdout == fcontent + fcontent assert retcode == 0 # I get its content via the stderr; emulate also network slowness (note I cat twice) - retcode, stdout, stderr = trans.exec_command_wait(f'cat {fname} >&2 ; sleep 1 ; cat {fname} >&2') + retcode, stdout, stderr = transport.exec_command_wait( + f'cat {fname} >&2 ; sleep 1 ; cat {fname} >&2', workdir=str(directory_path) + ) assert stderr == fcontent + fcontent assert stdout == '' assert retcode == 0 @@ -1196,18 +1194,12 @@ def test_transfer_big_stdout(custom_transport): # line_repetitions, file_line, file_line)) # However this is pretty slow (and using 'cat' of a file containing only one line is even slower) - retcode, stdout, stderr = trans.exec_command_wait(f'python3 {script_fname}') + retcode, stdout, stderr = transport.exec_command_wait(f'python3 {script_fname}', workdir=str(directory_path)) assert stderr == fcontent assert stdout == fcontent assert retcode == 0 - # Clean-up - trans.remove(fname) - trans.remove(script_fname) - trans.chdir('..') - trans.rmdir(directory) - def test_asynchronous_execution(custom_transport): """Test that the execution of a long(ish) command via the direct scheduler does not block. From db729762b18f436908a889c65db5898c2ed94b34 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 18:40:58 +0100 Subject: [PATCH 09/20] test_asynchronuos_execution --- tests/transports/test_all_plugins.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index f54a659c0b..8abc6fef4b 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -1201,7 +1201,7 @@ def test_transfer_big_stdout(custom_transport, tmp_path): assert retcode == 0 -def test_asynchronous_execution(custom_transport): +def test_asynchronous_execution(custom_transport, tmp_path): """Test that the execution of a long(ish) command via the direct scheduler does not block. This is a regression test for #3094, where running a long job on the direct scheduler @@ -1220,10 +1220,10 @@ def test_asynchronous_execution(custom_transport): tmpf.write(b'#!/bin/bash\nsleep 10\n') tmpf.flush() - transport.putfile(tmpf.name, os.path.join('/tmp', script_fname)) + transport.putfile(tmpf.name, str(tmp_path / script_fname)) timestamp_before = time.time() - job_id_string = scheduler.submit_job('/tmp', script_fname) + job_id_string = scheduler.submit_job(str(tmp_path), script_fname) elapsed_time = time.time() - timestamp_before # We want to get back control. If it takes < 5 seconds, it means that it is not blocking @@ -1259,9 +1259,3 @@ def test_asynchronous_execution(custom_transport): # If the process is already dead (or has never run), I just ignore the error pass - # Also remove the script - try: - transport.remove(f'/tmp/{script_fname}') - except FileNotFoundError: - # If the file wasn't even created, I just ignore this error - pass From f9792b84bee0ade9cb8020d2a99d51039f0ba239 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 21 Nov 2024 23:30:36 +0100 Subject: [PATCH 10/20] checkpoint --- tests/transports/test_all_plugins.py | 98 ++++++++++++---------------- 1 file changed, 42 insertions(+), 56 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 8abc6fef4b..c55259cd84 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -24,6 +24,7 @@ import psutil import pytest +from pathlib import Path from aiida.plugins import SchedulerFactory, TransportFactory, entry_point from aiida.transports import Transport @@ -510,64 +511,42 @@ def test_put_get_empty_string_file(custom_transport, tmp_path_factory): transport.getfile(remote_file_abs_path, retrieved_file_abs_path) -def test_put_and_get_tree(custom_transport): +def test_put_and_get_tree(custom_transport, tmp_path_factory): """Test putting and getting files.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + local_subfolder = str(local_dir / directory / 'tmp1') + remote_subfolder = str(remote_dir / 'tmp2') + retrieved_subfolder = str(local_dir / directory / 'tmp3') - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + (local_dir / directory / local_subfolder).mkdir(parents=True) - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) - - transport.chdir(directory) - - local_file_name = os.path.join(local_subfolder, 'file.txt') + local_file_name = Path(local_subfolder) / 'file.txt' text = 'Viva Verdi\n' with open(local_file_name, 'w', encoding='utf8') as fhandle: fhandle.write(text) # here use full path in src and dst - for i in range(2): - if i == 0: - transport.put(local_subfolder, remote_subfolder) - transport.get(remote_subfolder, retrieved_subfolder) - else: - transport.puttree(local_subfolder, remote_subfolder) - transport.gettree(remote_subfolder, retrieved_subfolder) - - # Here I am mixing the local with the remote fold - list_of_dirs = transport.listdir('.') - # # it is False because local_file_name has the full path, - # # while list_of_files has not - assert local_subfolder not in list_of_dirs - assert remote_subfolder in list_of_dirs - assert retrieved_subfolder not in list_of_dirs - assert 'tmp1' in list_of_dirs - assert 'tmp3' in list_of_dirs - - list_pushed_file = transport.listdir('tmp2') - list_retrieved_file = transport.listdir('tmp3') - assert 'file.txt' in list_pushed_file - assert 'file.txt' in list_retrieved_file - - shutil.rmtree(local_subfolder) - shutil.rmtree(retrieved_subfolder) - transport.rmtree(remote_subfolder) + transport.puttree(local_subfolder, remote_subfolder) + transport.gettree(remote_subfolder, retrieved_subfolder) - transport.chdir('..') - transport.rmdir(directory) + list_of_dirs = transport.listdir(str(local_dir / directory)) + + assert local_subfolder not in list_of_dirs + assert remote_subfolder not in list_of_dirs + assert retrieved_subfolder not in list_of_dirs + assert 'tmp1' in list_of_dirs + assert 'tmp3' in list_of_dirs + + list_pushed_file = transport.listdir(remote_subfolder) + list_retrieved_file = transport.listdir(retrieved_subfolder) + assert 'file.txt' in list_pushed_file + assert 'file.txt' in list_retrieved_file @pytest.mark.parametrize( @@ -928,11 +907,14 @@ def test_put_get_abs_path_tree(custom_transport): transport.rmdir(directory) -def test_put_get_empty_string_tree(custom_transport): +def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): """Test of exception put/get of empty strings""" # TODO : verify the correctness of \n at the end of a file local_dir = os.path.join('/', 'tmp') remote_dir = local_dir + # local_dir = tmp_path_factory.mktemp('local') + # remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: @@ -994,18 +976,23 @@ def test_put_get_empty_string_tree(custom_transport): transport.rmdir(directory) -def test_gettree_nested_directory(custom_transport): +def test_gettree_nested_directory(custom_transport, remote_tmp_path: Path, tmp_path: Path): """Test `gettree` for a nested directory.""" - with tempfile.TemporaryDirectory() as dir_remote, tempfile.TemporaryDirectory() as dir_local: - content = b'dummy\ncontent' - filepath = os.path.join(dir_remote, 'sub', 'path', 'filename.txt') - os.makedirs(os.path.dirname(filepath)) + # with tempfile.TemporaryDirectory() as dir_remote, tempfile.TemporaryDirectory() as dir_local: + content = b'dummy\ncontent' + # filepath = os.path.join(dir_remote, 'sub', 'path', 'filename.txt') + dir_path = remote_tmp_path / 'sub' / 'path' + dir_path.mkdir(parents=True) - with open(filepath, 'wb') as handle: - handle.write(content) + file_path = str(dir_path / 'filename.txt') - with custom_transport as transport: - transport.gettree(os.path.join(dir_remote, 'sub/path'), os.path.join(dir_local, 'sub/path')) + with open(file_path, 'wb') as handle: + handle.write(content) + + with custom_transport as transport: + transport.gettree(str(remote_tmp_path / 'sub' / 'path'), str(tmp_path / 'sub' / 'path')) + + assert (tmp_path / 'sub' / 'path' / 'filename.txt').is_file def test_exec_pwd(custom_transport, remote_tmp_path): @@ -1258,4 +1245,3 @@ def test_asynchronous_execution(custom_transport, tmp_path): except ProcessLookupError: # If the process is already dead (or has never run), I just ignore the error pass - From f9bfea95fd4758aaf41bbd9e73fc8e71f56aa68d Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Fri, 22 Nov 2024 00:24:45 +0100 Subject: [PATCH 11/20] test_copy --- tests/transports/test_all_plugins.py | 83 ++++++++++++++-------------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index c55259cd84..24617dc360 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -620,72 +620,73 @@ def test_put_and_get_overwrite( ) -def test_copy(custom_transport): +def test_copy(custom_transport, tmp_path_factory): """Test copying.""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + # local_dir = os.path.join('/', 'tmp') + # remote_dir = local_dir + remote_dir = tmp_path_factory.mktemp('remote') + directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + workdir = remote_dir / directory - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - - transport.mkdir(directory) - transport.chdir(directory) + transport.mkdir(str(workdir)) - local_base_dir = os.path.join(local_dir, directory, 'local') - os.mkdir(local_base_dir) + base_dir = workdir / 'origin' + base_dir.mkdir() # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + file_1 = base_dir / 'a.txt' + file_2 = base_dir / 'b.tmp' + file_3 = base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) # first test the copy. Copy of two files matching patterns, into a folder - transport.copy(os.path.join('local', '*.txt'), '.') - assert set(['a.txt', 'c.txt', 'local']) == set(transport.listdir('.')) - transport.remove('a.txt') - transport.remove('c.txt') + transport.copy(str(base_dir / '*.txt'), str(workdir)) + assert set(['a.txt', 'c.txt', 'origin']) == set(transport.listdir(str(workdir))) + transport.remove(str(workdir / 'a.txt')) + transport.remove(str(workdir / 'c.txt')) + # second test copy. Copy of two folders - transport.copy('local', 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir('prova')) - transport.rmtree('prova') + transport.copy(str(base_dir), str(workdir / 'prova')) + assert set(['prova', 'origin']) == set(transport.listdir(str(workdir))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(workdir / 'prova'))) + transport.rmtree(str(workdir / 'prova')) + # third test copy. Can copy one file into a new file - transport.copy(os.path.join('local', '*.tmp'), 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - transport.remove('prova') + transport.copy(str(base_dir / '*.tmp'), str(workdir / 'prova')) + assert transport.isfile(str(workdir / 'prova')) + transport.remove(str(workdir / 'prova')) + # fourth test copy: can't copy more than one file on the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.copy(os.path.join('local', '*.txt'), 'prova') + transport.copy(str(base_dir / '*.txt'), str(workdir / 'prova')) + # fifth test, copying one file into a folder - transport.mkdir('prova') - transport.copy(os.path.join('local', 'a.txt'), 'prova') - assert set(transport.listdir('prova')) == set(['a.txt']) - transport.rmtree('prova') + transport.mkdir(str(workdir / 'prova')) + transport.copy(str(base_dir / 'a.txt'), str(workdir / 'prova')) + assert set(transport.listdir(str(workdir / 'prova'))) == set(['a.txt']) + transport.rmtree(str(workdir / 'prova')) + # sixth test, copying one file into a file - transport.copy(os.path.join('local', 'a.txt'), 'prova') - assert transport.isfile('prova') - transport.remove('prova') + transport.copy(str(base_dir / 'a.txt'), str(workdir / 'prova')) + assert transport.isfile(str(workdir / 'prova')) + transport.remove(str(workdir / 'prova')) # copy of folder into an existing folder # NOTE: the command cp has a different behavior on Mac vs Ubuntu # tests performed locally on a Mac may result in a failure. - transport.mkdir('prova') - transport.copy('local', 'prova') - assert set(['local']) == set(transport.listdir('prova')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(os.path.join('prova', 'local'))) - transport.rmtree('prova') + transport.mkdir(str(workdir / 'prova')) + transport.copy(str(base_dir), str(workdir / 'prova')) + assert set(['origin']) == set(transport.listdir(str(workdir / 'prova'))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(workdir / 'prova' / 'origin'))) + transport.rmtree(str(workdir / 'prova')) # exit - transport.chdir('..') - transport.rmtree(directory) + transport.rmtree(str(workdir)) def test_put(custom_transport): From 58d238739a6d80fc9a380884b9db2cfc38b45f28 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 21 Nov 2024 23:24:49 +0000 Subject: [PATCH 12/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/transports/test_all_plugins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 24617dc360..a0bfa7a396 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -21,10 +21,10 @@ import tempfile import time import uuid +from pathlib import Path import psutil import pytest -from pathlib import Path from aiida.plugins import SchedulerFactory, TransportFactory, entry_point from aiida.transports import Transport @@ -1020,7 +1020,7 @@ def test_exec_pwd(custom_transport, remote_tmp_path): assert transport.isdir(subfolder_fullpath) - retcode, stdout, stderr = transport.exec_command_wait(f'ls {str(remote_tmp_path)}') + retcode, stdout, stderr = transport.exec_command_wait(f'ls {remote_tmp_path!s}') assert retcode == 0 assert stdout.strip() in subfolder_fullpath assert stderr == '' From eb6f979097b824adda01b0ecf9a5874c880ca43a Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Sun, 24 Nov 2024 13:44:41 +0100 Subject: [PATCH 13/20] test_put --- tests/transports/test_all_plugins.py | 114 +++++++++++++-------------- 1 file changed, 54 insertions(+), 60 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index a0bfa7a396..ad15653acc 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -621,9 +621,7 @@ def test_put_and_get_overwrite( def test_copy(custom_transport, tmp_path_factory): - """Test copying.""" - # local_dir = os.path.join('/', 'tmp') - # remote_dir = local_dir + """Test copying from a remote src to remote dst""" remote_dir = tmp_path_factory.mktemp('remote') directory = 'tmp_try' @@ -636,7 +634,7 @@ def test_copy(custom_transport, tmp_path_factory): base_dir = workdir / 'origin' base_dir.mkdir() - # first test put: I create three files in local + # first create three files file_1 = base_dir / 'a.txt' file_2 = base_dir / 'b.tmp' file_3 = base_dir / 'c.txt' @@ -689,80 +687,76 @@ def test_copy(custom_transport, tmp_path_factory): transport.rmtree(str(workdir)) -def test_put(custom_transport): - """Test putting files.""" - # exactly the same tests of copy, just with the put function - # and therefore the local path must be absolute - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir +def test_put(custom_transport, tmp_path_factory): + """Test putting files. + Those are similar tests of copy, just with the put function which copy from mocked local to mocked remote + and therefore the local path must be absolute + """ + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + local_workdir = local_dir / directory + remote_workdir = remote_dir / directory - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + transport.mkdir(str(remote_workdir)) - transport.mkdir(directory) - transport.chdir(directory) - - local_base_dir = os.path.join(local_dir, directory, 'local') - os.mkdir(local_base_dir) + local_base_dir: Path = local_workdir / 'origin' + local_base_dir.mkdir(parents=True) # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + file_1 = local_base_dir / 'a.txt' + file_2 = local_base_dir / 'b.tmp' + file_3 = local_base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # first test putransport. Copy of two files matching patterns, into a folder - transport.put(os.path.join(local_base_dir, '*.txt'), '.') - assert set(['a.txt', 'c.txt', 'local']) == set(transport.listdir('.')) - transport.remove('a.txt') - transport.remove('c.txt') - # second. Copy of folder into a non existing folder - transport.put(local_base_dir, 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir('prova')) - transport.rmtree('prova') - # third. copy of folder into an existing folder - transport.mkdir('prova') - transport.put(local_base_dir, 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - assert set(['local']) == set(transport.listdir('prova')) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(os.path.join('prova', 'local'))) - transport.rmtree('prova') - # third test copy. Can copy one file into a new file - transport.put(os.path.join(local_base_dir, '*.tmp'), 'prova') - assert set(['prova', 'local']) == set(transport.listdir('.')) - transport.remove('prova') - # fourth test copy: can't copy more than one file on the same file, + # first test the put. Copy of two files matching patterns, into a folder + transport.put(str(local_base_dir / '*.txt'), str(remote_workdir)) + assert set(['a.txt', 'c.txt']) == set(transport.listdir(str(remote_workdir))) + transport.remove(str(remote_workdir / 'a.txt')) + transport.remove(str(remote_workdir / 'c.txt')) + + # second test put. Put of two folders + transport.put(str(local_base_dir), str(remote_workdir / 'prova')) + assert set(['prova']) == set(transport.listdir(str(remote_workdir))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(remote_workdir / 'prova'))) + transport.rmtree(str(remote_workdir / 'prova')) + + # third test put. Can copy one file into a new file + transport.put(str(local_base_dir / '*.tmp'), str(remote_workdir / 'prova')) + assert transport.isfile(str(remote_workdir / 'prova')) + transport.remove(str(remote_workdir / 'prova')) + + # fourth test put: can't copy more than one file on the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.put(os.path.join(local_base_dir, '*.txt'), 'prova') - # copy of folder into file - with open(os.path.join(local_dir, directory, 'existing.txt'), 'w', encoding='utf8') as fhandle: - fhandle.write(text) - with pytest.raises(OSError): - transport.put(os.path.join(local_base_dir), 'existing.txt') - transport.remove('existing.txt') + transport.put(str(local_base_dir / '*.txt'), str(remote_workdir / 'prova')) + # fifth test, copying one file into a folder - transport.mkdir('prova') - transport.put(os.path.join(local_base_dir, 'a.txt'), 'prova') - assert set(transport.listdir('prova')) == set(['a.txt']) - transport.rmtree('prova') + transport.mkdir(str(remote_workdir / 'prova')) + transport.put(str(local_base_dir / 'a.txt'), str(remote_workdir / 'prova')) + assert set(transport.listdir(str(remote_workdir / 'prova'))) == set(['a.txt']) + transport.rmtree(str(remote_workdir / 'prova')) + # sixth test, copying one file into a file - transport.put(os.path.join(local_base_dir, 'a.txt'), 'prova') - assert transport.isfile('prova') - transport.remove('prova') + transport.put(str(local_base_dir / 'a.txt'), str(remote_workdir / 'prova')) + assert transport.isfile(str(remote_workdir / 'prova')) + transport.remove(str(remote_workdir / 'prova')) + # put of folder into an existing folder + # NOTE: the command cp has a different behavior on Mac vs Ubuntu + # tests performed locally on a Mac may result in a failure. + transport.mkdir(str(remote_workdir / 'prova')) + transport.put(str(local_base_dir), str(remote_workdir / 'prova')) + assert set(['origin']) == set(transport.listdir(str(remote_workdir / 'prova'))) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set(transport.listdir(str(remote_workdir / 'prova' / 'origin'))) + transport.rmtree(str(remote_workdir / 'prova')) # exit - transport.chdir('..') - transport.rmtree(directory) + transport.rmtree(str(remote_workdir)) def test_get(custom_transport): From 67d32cc9f9eae9102844cb38d0f65b926e4923ec Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Sun, 24 Nov 2024 19:14:37 +0100 Subject: [PATCH 14/20] test_get --- tests/transports/test_all_plugins.py | 102 +++++++++++++-------------- 1 file changed, 49 insertions(+), 53 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index ad15653acc..3a7f3921b7 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -759,81 +759,77 @@ def test_put(custom_transport, tmp_path_factory): transport.rmtree(str(remote_workdir)) -def test_get(custom_transport): +def test_get(custom_transport, tmp_path_factory): """Test getting files.""" - # exactly the same tests of copy, just with the put function - # and therefore the local path must be absolute - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) + local_workdir: Path = local_dir / directory + remote_workdir: Path = remote_dir / directory - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) - - transport.mkdir(directory) - transport.chdir(directory) + local_workdir.mkdir() - local_base_dir = os.path.join(local_dir, directory, 'local') - local_destination = os.path.join(local_dir, directory) - os.mkdir(local_base_dir) + remote_base_dir: Path = remote_workdir / 'origin' + remote_base_dir.mkdir(parents=True) - # first test put: I create three files in local - file_1 = os.path.join(local_base_dir, 'a.txt') - file_2 = os.path.join(local_base_dir, 'b.tmp') - file_3 = os.path.join(local_base_dir, 'c.txt') + # first test put: I create three files in remote + file_1 = remote_base_dir / 'a.txt' + file_2 = remote_base_dir / 'b.tmp' + file_3 = remote_base_dir / 'c.txt' text = 'Viva Verdi\n' for filename in [file_1, file_2, file_3]: with open(filename, 'w', encoding='utf8') as fhandle: fhandle.write(text) - # first test put. Copy of two files matching patterns, into a folder - transport.get(os.path.join('local', '*.txt'), local_destination) - assert set(['a.txt', 'c.txt', 'local']) == set(os.listdir(local_destination)) - os.remove(os.path.join(local_destination, 'a.txt')) - os.remove(os.path.join(local_destination, 'c.txt')) + # first test get. Get two files matching patterns, from mocked remote folder into a local folder + transport.get(str(remote_base_dir / '*.txt'), str(local_workdir)) + assert set(['a.txt', 'c.txt']) == set(os.listdir(local_workdir)) + (local_workdir / 'a.txt').unlink() + (local_workdir / 'c.txt').unlink() + # second. Copy of folder into a non existing folder - transport.get('local', os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(os.listdir(os.path.join(local_destination, 'prova'))) - shutil.rmtree(os.path.join(local_destination, 'prova')) + transport.get(str(remote_base_dir), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + shutil.rmtree(local_workdir / 'prova') + # third. copy of folder into an existing folder - os.mkdir(os.path.join(local_destination, 'prova')) - transport.get('local', os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - assert set(['local']) == set(os.listdir(os.path.join(local_destination, 'prova'))) - assert set(['a.txt', 'b.tmp', 'c.txt']) == set(os.listdir(os.path.join(local_destination, 'prova', 'local'))) - shutil.rmtree(os.path.join(local_destination, 'prova')) - # third test copy. Can copy one file into a new file - transport.get(os.path.join('local', '*.tmp'), os.path.join(local_destination, 'prova')) - assert set(['prova', 'local']) == set(os.listdir(local_destination)) - os.remove(os.path.join(local_destination, 'prova')) + (local_workdir / 'prova').mkdir() + transport.get(str(remote_base_dir), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert set(['origin']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + assert set(['a.txt', 'b.tmp', 'c.txt']) == set([p.name for p in (local_workdir / 'prova' / 'origin').iterdir()]) + shutil.rmtree(local_workdir / 'prova') + + # test get one file into a new file prova + transport.get(str(remote_base_dir / '*.tmp'), str(local_workdir / 'prova')) + assert set(['prova']) == set([p.name for p in local_workdir.iterdir()]) + assert (local_workdir / 'prova').is_file() + (local_workdir / 'prova').unlink() + # fourth test copy: can't copy more than one file on the same file, # i.e., the destination should be a folder with pytest.raises(OSError): - transport.get(os.path.join('local', '*.txt'), os.path.join(local_destination, 'prova')) + transport.get(str(remote_base_dir / '*.txt'), str(local_workdir / 'prova')) # copy of folder into file - with open(os.path.join(local_destination, 'existing.txt'), 'w', encoding='utf8') as fhandle: + with open(local_workdir / 'existing.txt', 'w', encoding='utf8') as fhandle: fhandle.write(text) with pytest.raises(OSError): - transport.get('local', os.path.join(local_destination, 'existing.txt')) - os.remove(os.path.join(local_destination, 'existing.txt')) + transport.get(str(remote_base_dir), str(local_workdir / 'existing.txt')) + (local_workdir / 'existing.txt').unlink() + # fifth test, copying one file into a folder - os.mkdir(os.path.join(local_destination, 'prova')) - transport.get(os.path.join('local', 'a.txt'), os.path.join(local_destination, 'prova')) - assert set(os.listdir(os.path.join(local_destination, 'prova'))) == set(['a.txt']) - shutil.rmtree(os.path.join(local_destination, 'prova')) - # sixth test, copying one file into a file - transport.get(os.path.join('local', 'a.txt'), os.path.join(local_destination, 'prova')) - assert os.path.isfile(os.path.join(local_destination, 'prova')) - os.remove(os.path.join(local_destination, 'prova')) + (local_workdir / 'prova').mkdir() + transport.get(str(remote_base_dir / 'a.txt'), str(local_workdir / 'prova')) + assert set(['a.txt']) == set([p.name for p in (local_workdir / 'prova').iterdir()]) + shutil.rmtree(local_workdir / 'prova') - # exit - transport.chdir('..') - transport.rmtree(directory) + # sixth test, copying one file into a file + transport.get(str(remote_base_dir / 'a.txt'), str(local_workdir / 'prova')) + assert (local_workdir / 'prova').is_file() + (local_workdir / 'prova').unlink() def test_put_get_abs_path_tree(custom_transport): From 31202e85d6f4b53a3c261e8ef9307b9dbf5b2525 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Sun, 24 Nov 2024 19:20:21 +0100 Subject: [PATCH 15/20] test_put_get_abs_path_tree --- tests/transports/test_all_plugins.py | 42 +++++++++------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 3a7f3921b7..d468a1caf1 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -264,7 +264,6 @@ def test_dir_permissions_creation_modification(custom_transport, remote_tmp_path # the new directory modes. To see if we want a higher # level function to ask for the mode, or we just # use get_attribute - transport.chdir(directory) # change permissions of an empty string, non existing folder. fake_dir = '' @@ -274,10 +273,7 @@ def test_dir_permissions_creation_modification(custom_transport, remote_tmp_path fake_dir = 'pippo' with pytest.raises(OSError): # chmod to a non existing folder - transport.chmod(fake_dir, 0o777) - - transport.chdir('..') - transport.rmdir(directory) + transport.chmod(str(remote_tmp_path / fake_dir), 0o777) def test_dir_reading_permissions(custom_transport, remote_tmp_path): @@ -832,30 +828,26 @@ def test_get(custom_transport, tmp_path_factory): (local_workdir / 'prova').unlink() -def test_put_get_abs_path_tree(custom_transport): +def test_put_get_abs_path_tree(custom_transport, tmp_path_factory): """Test of exception for non existing files and abs path""" - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + local_subfolder = str(local_dir / directory / 'tmp1') + remote_subfolder = str(remote_dir / 'tmp2') + retrieved_subfolder = str(local_dir / directory / 'tmp3') - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + (local_dir / directory / local_subfolder).mkdir(parents=True) - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) + local_file_name = Path(local_subfolder) / 'file.txt' - transport.chdir(directory) - local_file_name = os.path.join(local_subfolder, 'file.txt') - pathlib.Path(local_file_name).touch() + text = 'Viva Verdi\n' + with open(local_file_name, 'w', encoding='utf8') as fhandle: + fhandle.write(text) + # here use full path in src and dst # 'tmp1' is not an abs path with pytest.raises(ValueError): transport.put('tmp1', remote_subfolder) @@ -890,14 +882,6 @@ def test_put_get_abs_path_tree(custom_transport): with pytest.raises(ValueError): transport.gettree(remote_subfolder, 'delete_me_tree') - os.remove(os.path.join(local_subfolder, 'file.txt')) - os.rmdir(local_subfolder) - transport.rmtree(remote_subfolder) - - transport.chdir('..') - transport.rmdir(directory) - - def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): """Test of exception put/get of empty strings""" # TODO : verify the correctness of \n at the end of a file From 3c1a7024413bd2acfebf10514bd8e6ef99ef4a62 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Sun, 24 Nov 2024 20:00:54 +0100 Subject: [PATCH 16/20] Amend --- tests/transports/test_all_plugins.py | 95 +++++++++------------------- 1 file changed, 29 insertions(+), 66 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index d468a1caf1..443766fbfa 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -12,12 +12,8 @@ """ import io -import os -import pathlib -import random import shutil import signal -import string import tempfile import time import uuid @@ -410,13 +406,9 @@ def test_put_get_abs_path_file(custom_transport, tmp_path_factory): retrieved_file_name = 'file_retrieved.txt' remote_file_name = 'file_remote.txt' - local_file_rel_path = local_file_name - local_file_abs_path = str(local_dir / directory / local_file_name) - remote_file_rel_path = remote_file_name - local_file_abs_path = str(local_dir / directory / local_file_name) retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) remote_file_abs_path = str(remote_dir / directory / remote_file_name) @@ -509,29 +501,29 @@ def test_put_get_empty_string_file(custom_transport, tmp_path_factory): def test_put_and_get_tree(custom_transport, tmp_path_factory): """Test putting and getting files.""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir: Path = tmp_path_factory.mktemp('local') + remote_dir: Path = tmp_path_factory.mktemp('remote') directory = 'tmp_try' with custom_transport as transport: - local_subfolder = str(local_dir / directory / 'tmp1') - remote_subfolder = str(remote_dir / 'tmp2') - retrieved_subfolder = str(local_dir / directory / 'tmp3') + local_subfolder: Path = local_dir / directory / 'tmp1' + remote_subfolder: Path = remote_dir / 'tmp2' + retrieved_subfolder: Path = local_dir / directory / 'tmp3' - (local_dir / directory / local_subfolder).mkdir(parents=True) + local_subfolder.mkdir(parents=True) - local_file_name = Path(local_subfolder) / 'file.txt' + local_file = local_subfolder / 'file.txt' text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file, 'w', encoding='utf8') as fhandle: fhandle.write(text) # here use full path in src and dst - transport.puttree(local_subfolder, remote_subfolder) - transport.gettree(remote_subfolder, retrieved_subfolder) + transport.puttree(str(local_subfolder), str(remote_subfolder)) + transport.gettree(str(remote_subfolder), str(retrieved_subfolder)) - list_of_dirs = transport.listdir(str(local_dir / directory)) + list_of_dirs = [p.name for p in (local_dir / directory).iterdir()] assert local_subfolder not in list_of_dirs assert remote_subfolder not in list_of_dirs @@ -539,8 +531,8 @@ def test_put_and_get_tree(custom_transport, tmp_path_factory): assert 'tmp1' in list_of_dirs assert 'tmp3' in list_of_dirs - list_pushed_file = transport.listdir(remote_subfolder) - list_retrieved_file = transport.listdir(retrieved_subfolder) + list_pushed_file = transport.listdir(str(remote_subfolder)) + list_retrieved_file = [p.name for p in retrieved_subfolder.iterdir()] assert 'file.txt' in list_pushed_file assert 'file.txt' in list_retrieved_file @@ -781,7 +773,7 @@ def test_get(custom_transport, tmp_path_factory): # first test get. Get two files matching patterns, from mocked remote folder into a local folder transport.get(str(remote_base_dir / '*.txt'), str(local_workdir)) - assert set(['a.txt', 'c.txt']) == set(os.listdir(local_workdir)) + assert set(['a.txt', 'c.txt']) == set([p.name for p in (local_workdir).iterdir()]) (local_workdir / 'a.txt').unlink() (local_workdir / 'c.txt').unlink() @@ -882,35 +874,24 @@ def test_put_get_abs_path_tree(custom_transport, tmp_path_factory): with pytest.raises(ValueError): transport.gettree(remote_subfolder, 'delete_me_tree') + def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): """Test of exception put/get of empty strings""" - # TODO : verify the correctness of \n at the end of a file - local_dir = os.path.join('/', 'tmp') - remote_dir = local_dir - # local_dir = tmp_path_factory.mktemp('local') - # remote_dir = tmp_path_factory.mktemp('remote') - + local_dir = tmp_path_factory.mktemp('local') + remote_dir = tmp_path_factory.mktemp('remote') directory = 'tmp_try' with custom_transport as transport: - transport.chdir(remote_dir) - - while os.path.exists(os.path.join(local_dir, directory)): - # I append a random letter/number until it is unique - directory += random.choice(string.ascii_uppercase + string.digits) + local_subfolder: Path = local_dir / directory / 'tmp1' + remote_subfolder: Path = remote_dir / 'tmp2' + retrieved_subfolder: Path = local_dir / directory / 'tmp3' - local_subfolder = os.path.join(local_dir, directory, 'tmp1') - remote_subfolder = 'tmp2' - retrieved_subfolder = os.path.join(local_dir, directory, 'tmp3') + local_subfolder.mkdir(parents=True) - os.mkdir(os.path.join(local_dir, directory)) - os.mkdir(os.path.join(local_dir, directory, local_subfolder)) - - transport.chdir(directory) - local_file_name = os.path.join(local_subfolder, 'file.txt') + local_file = local_subfolder / 'file.txt' text = 'Viva Verdi\n' - with open(local_file_name, 'w', encoding='utf8') as fhandle: + with open(local_file, 'w', encoding='utf8') as fhandle: fhandle.write(text) # localpath is an empty string @@ -922,7 +903,7 @@ def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): with pytest.raises(OSError): transport.puttree(local_subfolder, '') - transport.puttree(local_subfolder, remote_subfolder) + transport.puttree(str(local_subfolder), str(remote_subfolder)) # remote path is an empty string with pytest.raises(OSError): @@ -935,27 +916,14 @@ def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): # TODO : get doesn't retrieve empty files. # Is it what we want? - transport.gettree(remote_subfolder, retrieved_subfolder) - - os.remove(os.path.join(local_subfolder, 'file.txt')) - os.rmdir(local_subfolder) - transport.remove(os.path.join(remote_subfolder, 'file.txt')) - transport.rmdir(remote_subfolder) - # If it couldn't end the copy, it leaves what he did on local file - # here I am mixing local with remote - assert 'file.txt' in transport.listdir('tmp3') - os.remove(os.path.join(retrieved_subfolder, 'file.txt')) - os.rmdir(retrieved_subfolder) + transport.gettree(str(remote_subfolder), str(retrieved_subfolder)) - transport.chdir('..') - transport.rmdir(directory) + assert 'file.txt' in [p.name for p in retrieved_subfolder.iterdir()] def test_gettree_nested_directory(custom_transport, remote_tmp_path: Path, tmp_path: Path): """Test `gettree` for a nested directory.""" - # with tempfile.TemporaryDirectory() as dir_remote, tempfile.TemporaryDirectory() as dir_local: content = b'dummy\ncontent' - # filepath = os.path.join(dir_remote, 'sub', 'path', 'filename.txt') dir_path = remote_tmp_path / 'sub' / 'path' dir_path.mkdir(parents=True) @@ -978,19 +946,12 @@ def test_exec_pwd(custom_transport, remote_tmp_path): execution (done in this module, in the _exec_command_internal function). """ # Start value - delete_at_end = False - with custom_transport as transport: # To compare with: getcwd uses the normalized ('realpath') path - location = transport.normalize('/tmp') subfolder = """_'s f"#""" # A folder with characters to escape subfolder_fullpath = str(remote_tmp_path / subfolder) - if not transport.isdir(subfolder_fullpath): - # Since I created the folder, I will remember to - # delete it at the end of this test - delete_at_end = True - transport.mkdir(subfolder_fullpath) + transport.mkdir(subfolder_fullpath) assert transport.isdir(subfolder_fullpath) @@ -1171,6 +1132,8 @@ def test_asynchronous_execution(custom_transport, tmp_path): """ # Use a unique name, using a UUID, to avoid concurrent tests (or very rapid # tests that follow each other) to overwrite the same destination + import os + script_fname = f'sleep-submit-{uuid.uuid4().hex}-{custom_transport.__class__.__name__}.sh' scheduler = SchedulerFactory('core.direct')() From 3b79fafafc35eb9439f603f1e657ce7c0ba2d1d0 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Mon, 25 Nov 2024 17:10:17 +0100 Subject: [PATCH 17/20] remove mypy for tests --- .pre-commit-config.yaml | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 32305828b4..3bcd486ef1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -90,6 +90,7 @@ repos: .docker/.*| docs/.*| utils/.*| + tests/.*| src/aiida/calculations/arithmetic/add.py| src/aiida/calculations/diff_tutorial/calculations.py| @@ -192,17 +193,6 @@ repos: src/aiida/transports/plugins/local.py| src/aiida/transports/plugins/ssh.py| src/aiida/workflows/arithmetic/multiply_add.py| - - tests/conftest.py| - tests/repository/conftest.py| - tests/repository/test_repository.py| - tests/sphinxext/sources/workchain/conf.py| - tests/sphinxext/sources/workchain_broken/conf.py| - tests/storage/psql_dos/migrations/conftest.py| - tests/storage/psql_dos/migrations/django_branch/test_0026_0027_traj_data.py| - tests/test_calculation_node.py| - tests/test_nodes.py| - )$ - id: dm-generate-all From 6203a3bd16b52a812da8b110fa263bf14ee19b27 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Mon, 25 Nov 2024 20:22:08 +0100 Subject: [PATCH 18/20] all use specific fixture --- tests/transports/test_all_plugins.py | 176 +++++++++++++-------------- 1 file changed, 88 insertions(+), 88 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index 443766fbfa..c77ff75bb7 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -31,11 +31,17 @@ @pytest.fixture(scope='function') -def remote_tmp_path(tmp_path_factory): - """Mock the remote tmp path using tmp_path_factory to create folder start with 'remote'""" +def tmp_path_remote(tmp_path_factory): + """Mock the remote tmp path using tmp_path_factory to create folder start with prefix 'remote'""" return tmp_path_factory.mktemp('remote') +@pytest.fixture(scope='function') +def tmp_path_local(tmp_path_factory): + """Mock the local tmp path using tmp_path_factory to create folder start with prefix 'local'""" + return tmp_path_factory.mktemp('local') + + @pytest.fixture(scope='function', params=entry_point.get_entry_point_names('aiida.transports')) def custom_transport(request, tmp_path_factory, monkeypatch) -> Transport: """Fixture that parametrizes over all the registered implementations of the ``CommonRelaxWorkChain``.""" @@ -65,36 +71,36 @@ def test_is_open(custom_transport): assert not custom_transport.is_open -def test_deprecate_chdir_and_getcwd(custom_transport, remote_tmp_path): +def test_chdir_and_getcwd_deprecated(custom_transport, tmp_path_remote): """Test to be deprecated ``chdir``/``getcwd`` methods still work.""" with custom_transport as transport: - location = str(remote_tmp_path) + location = str(tmp_path_remote) transport.chdir(location) assert location == transport.getcwd() -def test_chdir_to_empty_string(custom_transport, remote_tmp_path): +def test_chdir_to_empty_string_deprecated(custom_transport, tmp_path_remote): """I check that if I pass an empty string to chdir, the cwd does not change (this is a paramiko default behavior), but getcwd() is still correctly defined. """ with custom_transport as transport: - new_dir = str(remote_tmp_path) + new_dir = str(tmp_path_remote) transport.chdir(new_dir) transport.chdir('') assert new_dir == transport.getcwd() -def test_makedirs(custom_transport, remote_tmp_path): +def test_makedirs(custom_transport, tmp_path_remote): """Verify the functioning of makedirs command""" with custom_transport as transport: # define folder structure - dir_tree = str(remote_tmp_path / '1' / '2') + dir_tree = str(tmp_path_remote / '1' / '2') # I create the tree transport.makedirs(dir_tree) # verify the existence - assert transport.isdir(str(remote_tmp_path / '1')) + assert transport.isdir(str(tmp_path_remote / '1')) assert dir_tree # try to recreate the same folder @@ -104,27 +110,24 @@ def test_makedirs(custom_transport, remote_tmp_path): # recreate but with ignore flag transport.makedirs(dir_tree, True) - transport.rmdir(dir_tree) - transport.rmdir(str(remote_tmp_path / '1')) - -def test_rmtree(custom_transport, remote_tmp_path): +def test_rmtree(custom_transport, tmp_path_remote): """Verify the functioning of rmtree command""" with custom_transport as transport: # define folder structure - dir_tree = str(remote_tmp_path / '1' / '2') + dir_tree = str(tmp_path_remote / '1' / '2') # I create the tree transport.makedirs(dir_tree) # remove it - transport.rmtree(str(remote_tmp_path / '1')) + transport.rmtree(str(tmp_path_remote / '1')) # verify the removal - assert not transport.isdir(str(remote_tmp_path / '1')) + assert not transport.isdir(str(tmp_path_remote / '1')) # also tests that it works with a single file # create file local_file_name = 'file.txt' text = 'Viva Verdi\n' - single_file_path = str(remote_tmp_path / local_file_name) + single_file_path = str(tmp_path_remote / local_file_name) with open(single_file_path, 'w', encoding='utf8') as fhandle: fhandle.write(text) # remove it @@ -133,29 +136,29 @@ def test_rmtree(custom_transport, remote_tmp_path): assert not transport.isfile(single_file_path) -def test_listdir(custom_transport, remote_tmp_path): +def test_listdir(custom_transport, tmp_path_remote): """Create directories, verify listdir, delete a folder with subfolders""" with custom_transport as transport: list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - transport.mkdir(str(remote_tmp_path / this_dir)) + transport.mkdir(str(tmp_path_remote / this_dir)) for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - transport.putfile(tmpf.name, str(remote_tmp_path / fname)) + transport.putfile(tmpf.name, str(tmp_path_remote / fname)) - list_found = transport.listdir(str(remote_tmp_path)) + list_found = transport.listdir(str(tmp_path_remote)) assert sorted(list_found) == sorted(list_of_dir + list_of_files) - assert sorted(transport.listdir(str(remote_tmp_path), 'a*')), sorted(['as', 'a2', 'a4f']) - assert sorted(transport.listdir(str(remote_tmp_path), 'a?')), sorted(['as', 'a2']) - assert sorted(transport.listdir(str(remote_tmp_path), 'a[2-4]*')), sorted(['a2', 'a4f']) + assert sorted(transport.listdir(str(tmp_path_remote), 'a*')), sorted(['as', 'a2', 'a4f']) + assert sorted(transport.listdir(str(tmp_path_remote), 'a?')), sorted(['as', 'a2']) + assert sorted(transport.listdir(str(tmp_path_remote), 'a[2-4]*')), sorted(['a2', 'a4f']) -def test_listdir_withattributes(custom_transport, remote_tmp_path): +def test_listdir_withattributes(custom_transport, tmp_path_remote): """Create directories, verify listdir_withattributes, delete a folder with subfolders""" def simplify_attributes(data): @@ -171,38 +174,38 @@ def simplify_attributes(data): list_of_dir = ['1', '-f a&', 'as', 'a2', 'a4f'] list_of_files = ['a', 'b'] for this_dir in list_of_dir: - transport.mkdir(str(remote_tmp_path / this_dir)) + transport.mkdir(str(tmp_path_remote / this_dir)) for fname in list_of_files: with tempfile.NamedTemporaryFile() as tmpf: # Just put an empty file there at the right file name - transport.putfile(tmpf.name, str(remote_tmp_path / fname)) + transport.putfile(tmpf.name, str(tmp_path_remote / fname)) comparison_list = {k: True for k in list_of_dir} for k in list_of_files: comparison_list[k] = False - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path))), comparison_list - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a*')), { + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote))), comparison_list + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a*')), { 'as': True, 'a2': True, 'a4f': True, 'a': False, } - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a?')), { + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a?')), { 'as': True, 'a2': True, } - assert simplify_attributes(transport.listdir_withattributes(str(remote_tmp_path), 'a[2-4]*')), { + assert simplify_attributes(transport.listdir_withattributes(str(tmp_path_remote), 'a[2-4]*')), { 'a2': True, 'a4f': True, } -def test_dir_creation_deletion(custom_transport, remote_tmp_path): +def test_dir_creation_deletion(custom_transport, tmp_path_remote): """Test creating and deleting directories.""" with custom_transport as transport: - new_dir = str(remote_tmp_path / 'new') + new_dir = str(tmp_path_remote / 'new') transport.mkdir(new_dir) with pytest.raises(OSError): @@ -213,16 +216,16 @@ def test_dir_creation_deletion(custom_transport, remote_tmp_path): assert not transport.isfile(new_dir) -def test_dir_copy(custom_transport, remote_tmp_path): +def test_dir_copy(custom_transport, tmp_path_remote): """Verify if in the copy of a directory also the protection bits are carried over """ with custom_transport as transport: # Create a src dir - src_dir = str(remote_tmp_path / 'copy_src') + src_dir = str(tmp_path_remote / 'copy_src') transport.mkdir(src_dir) - dst_dir = str(remote_tmp_path / 'copy_dst') + dst_dir = str(tmp_path_remote / 'copy_dst') transport.copy(src_dir, dst_dir) with pytest.raises(ValueError): @@ -232,12 +235,12 @@ def test_dir_copy(custom_transport, remote_tmp_path): transport.copy('', dst_dir) -def test_dir_permissions_creation_modification(custom_transport, remote_tmp_path): +def test_dir_permissions_creation_modification(custom_transport, tmp_path_remote): """Verify if chmod raises OSError when trying to change bits on a non-existing folder """ with custom_transport as transport: - directory = str(remote_tmp_path / 'test') + directory = str(tmp_path_remote / 'test') transport.makedirs(directory) @@ -269,15 +272,15 @@ def test_dir_permissions_creation_modification(custom_transport, remote_tmp_path fake_dir = 'pippo' with pytest.raises(OSError): # chmod to a non existing folder - transport.chmod(str(remote_tmp_path / fake_dir), 0o777) + transport.chmod(str(tmp_path_remote / fake_dir), 0o777) -def test_dir_reading_permissions(custom_transport, remote_tmp_path): +def test_dir_reading_permissions(custom_transport, tmp_path_remote): """Try to enter a directory with no read permissions. Verify that the cwd has not changed after failed try. """ with custom_transport as transport: - directory = str(remote_tmp_path / 'test') + directory = str(tmp_path_remote / 'test') # create directory with non default permissions transport.mkdir(directory) @@ -307,28 +310,25 @@ def test_isfile_isdir_to_empty_string(custom_transport): assert not transport.isfile('') -def test_isfile_isdir_to_non_existing_string(custom_transport, remote_tmp_path): +def test_isfile_isdir_to_non_existing_string(custom_transport, tmp_path_remote): """I check that isdir or isfile return False when executed on an empty string """ with custom_transport as transport: - fake_folder = str(remote_tmp_path / 'pippo') + fake_folder = str(tmp_path_remote / 'pippo') assert not transport.isfile(fake_folder) assert not transport.isdir(fake_folder) with pytest.raises(OSError): transport.chdir(fake_folder) -def test_put_and_get(custom_transport, tmp_path_factory): +def test_put_and_get(custom_transport, tmp_path_remote, tmp_path_local): """Test putting and getting files.""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') - directory = 'tmp_try' with custom_transport as transport: - (local_dir / directory).mkdir() - transport.mkdir(str(remote_dir / directory)) + (tmp_path_local / directory).mkdir() + transport.mkdir(str(tmp_path_remote / directory)) local_file_name = 'file.txt' retrieved_file_name = 'file_retrieved.txt' @@ -336,9 +336,9 @@ def test_put_and_get(custom_transport, tmp_path_factory): remote_file_name = 'file_remote.txt' # here use full path in src and dst - local_file_abs_path = str(local_dir / directory / local_file_name) - retrieved_file_abs_path = str(local_dir / directory / retrieved_file_name) - remote_file_abs_path = str(remote_dir / directory / remote_file_name) + local_file_abs_path = str(tmp_path_local / directory / local_file_name) + retrieved_file_abs_path = str(tmp_path_local / directory / retrieved_file_name) + remote_file_abs_path = str(tmp_path_remote / directory / remote_file_name) text = 'Viva Verdi\n' with open(local_file_abs_path, 'w', encoding='utf8') as fhandle: @@ -347,7 +347,7 @@ def test_put_and_get(custom_transport, tmp_path_factory): transport.put(local_file_abs_path, remote_file_abs_path) transport.get(remote_file_abs_path, retrieved_file_abs_path) - list_of_files = transport.listdir(str(remote_dir / directory)) + list_of_files = transport.listdir(str(tmp_path_remote / directory)) # it is False because local_file_name has the full path, # while list_of_files has not assert local_file_name not in list_of_files @@ -355,10 +355,10 @@ def test_put_and_get(custom_transport, tmp_path_factory): assert retrieved_file_name not in list_of_files -def test_putfile_and_getfile(custom_transport, tmp_path_factory): +def test_putfile_and_getfile(custom_transport, tmp_path_remote, tmp_path_local): """Test putting and getting files.""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' @@ -391,10 +391,10 @@ def test_putfile_and_getfile(custom_transport, tmp_path_factory): assert retrieved_file_name not in list_of_files -def test_put_get_abs_path_file(custom_transport, tmp_path_factory): +def test_put_get_abs_path_file(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception for non existing files and abs path""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' @@ -437,10 +437,10 @@ def test_put_get_abs_path_file(custom_transport, tmp_path_factory): transport.getfile(remote_file_rel_path, 'delete_me.txt') -def test_put_get_empty_string_file(custom_transport, tmp_path_factory): +def test_put_get_empty_string_file(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception put/get of empty strings""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' @@ -499,10 +499,10 @@ def test_put_get_empty_string_file(custom_transport, tmp_path_factory): transport.getfile(remote_file_abs_path, retrieved_file_abs_path) -def test_put_and_get_tree(custom_transport, tmp_path_factory): +def test_put_and_get_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test putting and getting files.""" - local_dir: Path = tmp_path_factory.mktemp('local') - remote_dir: Path = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' @@ -608,9 +608,9 @@ def test_put_and_get_overwrite( ) -def test_copy(custom_transport, tmp_path_factory): +def test_copy(custom_transport, tmp_path_remote): """Test copying from a remote src to remote dst""" - remote_dir = tmp_path_factory.mktemp('remote') + remote_dir = tmp_path_remote directory = 'tmp_try' @@ -675,13 +675,13 @@ def test_copy(custom_transport, tmp_path_factory): transport.rmtree(str(workdir)) -def test_put(custom_transport, tmp_path_factory): +def test_put(custom_transport, tmp_path_remote, tmp_path_local): """Test putting files. Those are similar tests of copy, just with the put function which copy from mocked local to mocked remote and therefore the local path must be absolute """ - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: @@ -747,10 +747,10 @@ def test_put(custom_transport, tmp_path_factory): transport.rmtree(str(remote_workdir)) -def test_get(custom_transport, tmp_path_factory): +def test_get(custom_transport, tmp_path_remote, tmp_path_local): """Test getting files.""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: @@ -820,10 +820,10 @@ def test_get(custom_transport, tmp_path_factory): (local_workdir / 'prova').unlink() -def test_put_get_abs_path_tree(custom_transport, tmp_path_factory): +def test_put_get_abs_path_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception for non existing files and abs path""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: @@ -875,10 +875,10 @@ def test_put_get_abs_path_tree(custom_transport, tmp_path_factory): transport.gettree(remote_subfolder, 'delete_me_tree') -def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): +def test_put_get_empty_string_tree(custom_transport, tmp_path_remote, tmp_path_local): """Test of exception put/get of empty strings""" - local_dir = tmp_path_factory.mktemp('local') - remote_dir = tmp_path_factory.mktemp('remote') + local_dir = tmp_path_local + remote_dir = tmp_path_remote directory = 'tmp_try' with custom_transport as transport: @@ -921,10 +921,10 @@ def test_put_get_empty_string_tree(custom_transport, tmp_path_factory): assert 'file.txt' in [p.name for p in retrieved_subfolder.iterdir()] -def test_gettree_nested_directory(custom_transport, remote_tmp_path: Path, tmp_path: Path): +def test_gettree_nested_directory(custom_transport, tmp_path_remote, tmp_path_local): """Test `gettree` for a nested directory.""" content = b'dummy\ncontent' - dir_path = remote_tmp_path / 'sub' / 'path' + dir_path = tmp_path_remote / 'sub' / 'path' dir_path.mkdir(parents=True) file_path = str(dir_path / 'filename.txt') @@ -933,12 +933,12 @@ def test_gettree_nested_directory(custom_transport, remote_tmp_path: Path, tmp_p handle.write(content) with custom_transport as transport: - transport.gettree(str(remote_tmp_path / 'sub' / 'path'), str(tmp_path / 'sub' / 'path')) + transport.gettree(str(tmp_path_remote / 'sub' / 'path'), str(tmp_path_local / 'sub' / 'path')) - assert (tmp_path / 'sub' / 'path' / 'filename.txt').is_file + assert (tmp_path_local / 'sub' / 'path' / 'filename.txt').is_file -def test_exec_pwd(custom_transport, remote_tmp_path): +def test_exec_pwd(custom_transport, tmp_path_remote): """I create a strange subfolder with a complicated name and then see if I can run ``ls``. This also checks the correct escaping of funny characters, both in the directory @@ -949,13 +949,13 @@ def test_exec_pwd(custom_transport, remote_tmp_path): with custom_transport as transport: # To compare with: getcwd uses the normalized ('realpath') path subfolder = """_'s f"#""" # A folder with characters to escape - subfolder_fullpath = str(remote_tmp_path / subfolder) + subfolder_fullpath = str(tmp_path_remote / subfolder) transport.mkdir(subfolder_fullpath) assert transport.isdir(subfolder_fullpath) - retcode, stdout, stderr = transport.exec_command_wait(f'ls {remote_tmp_path!s}') + retcode, stdout, stderr = transport.exec_command_wait(f'ls {tmp_path_remote!s}') assert retcode == 0 assert stdout.strip() in subfolder_fullpath assert stderr == '' @@ -1040,7 +1040,7 @@ def test_exec_with_wrong_stdin(custom_transport): transport.exec_command_wait('cat', stdin=1) -def test_transfer_big_stdout(custom_transport, tmp_path): +def test_transfer_big_stdout(custom_transport, tmp_path_remote): """Test the transfer of a large amount of data on stdout.""" # Create a "big" file of > 2MB (10MB here; in general, larger than the buffer size) min_file_size_bytes = 5 * 1024 * 1024 @@ -1059,7 +1059,7 @@ def test_transfer_big_stdout(custom_transport, tmp_path): transport: Transport # We cannot use tempfile.mkdtemp because we're on a remote folder directory_name = 'temp_dir_test_transfer_big_stdout' - directory_path = tmp_path / directory_name + directory_path = tmp_path_remote / directory_name transport.mkdir(str(directory_path)) with tempfile.NamedTemporaryFile(mode='wb') as tmpf: From f4de64dc806e41f2f61860acf1cfb703d1338158 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Mon, 25 Nov 2024 20:49:30 +0100 Subject: [PATCH 19/20] ra --- tests/transports/test_all_plugins.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index c77ff75bb7..f9aa175a9b 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -51,6 +51,7 @@ def custom_transport(request, tmp_path_factory, monkeypatch) -> Transport: kwargs = {'machine': 'localhost', 'timeout': 30, 'load_system_host_keys': True, 'key_policy': 'AutoAddPolicy'} elif request.param == 'core.ssh_auto': kwargs = {'machine': 'localhost'} + # The transport config is store in a indepenednt tmp path to not mix up with the files under operating. filepath_config = tmp_path_factory.mktemp('transport') / 'config' monkeypatch.setattr(plugin, 'FILEPATH_CONFIG', filepath_config) if not filepath_config.exists(): @@ -497,6 +498,7 @@ def test_put_get_empty_string_file(custom_transport, tmp_path_remote, tmp_path_l transport.get(remote_file_abs_path, retrieved_file_abs_path) # overwrite retrieved_file_name transport.getfile(remote_file_abs_path, retrieved_file_abs_path) + assert 'file_retrieved.txt' in [p.name for p in (local_dir / directory).iterdir()] def test_put_and_get_tree(custom_transport, tmp_path_remote, tmp_path_local): @@ -719,11 +721,18 @@ def test_put(custom_transport, tmp_path_remote, tmp_path_local): assert transport.isfile(str(remote_workdir / 'prova')) transport.remove(str(remote_workdir / 'prova')) - # fourth test put: can't copy more than one file on the same file, + # fourth test put: can't copy more than one file to the same file, # i.e., the destination should be a folder with pytest.raises(OSError): transport.put(str(local_base_dir / '*.txt'), str(remote_workdir / 'prova')) + # can't copy folder to an exist file + with open(remote_workdir / 'existing.txt', 'w', encoding='utf8') as fhandle: + fhandle.write(text) + with pytest.raises(OSError): + transport.put(str(local_base_dir), str(remote_workdir / 'existing.txt')) + transport.remove(str(remote_workdir / 'existing.txt')) + # fifth test, copying one file into a folder transport.mkdir(str(remote_workdir / 'prova')) transport.put(str(local_base_dir / 'a.txt'), str(remote_workdir / 'prova')) @@ -933,7 +942,7 @@ def test_gettree_nested_directory(custom_transport, tmp_path_remote, tmp_path_lo handle.write(content) with custom_transport as transport: - transport.gettree(str(tmp_path_remote / 'sub' / 'path'), str(tmp_path_local / 'sub' / 'path')) + transport.gettree(str(tmp_path_remote), str(tmp_path_local)) assert (tmp_path_local / 'sub' / 'path' / 'filename.txt').is_file @@ -1056,7 +1065,6 @@ def test_transfer_big_stdout(custom_transport, tmp_path_remote): fcontent = (file_line_binary * line_repetitions).decode('utf8') with custom_transport as transport: - transport: Transport # We cannot use tempfile.mkdtemp because we're on a remote folder directory_name = 'temp_dir_test_transfer_big_stdout' directory_path = tmp_path_remote / directory_name From cd035ad30bdbc1c569095cbcea651b7e30fc03c7 Mon Sep 17 00:00:00 2001 From: Jusong Yu Date: Thu, 28 Nov 2024 13:14:46 +0100 Subject: [PATCH 20/20] rali [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/transports/test_all_plugins.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/transports/test_all_plugins.py b/tests/transports/test_all_plugins.py index f9aa175a9b..8aea2529cb 100644 --- a/tests/transports/test_all_plugins.py +++ b/tests/transports/test_all_plugins.py @@ -51,11 +51,12 @@ def custom_transport(request, tmp_path_factory, monkeypatch) -> Transport: kwargs = {'machine': 'localhost', 'timeout': 30, 'load_system_host_keys': True, 'key_policy': 'AutoAddPolicy'} elif request.param == 'core.ssh_auto': kwargs = {'machine': 'localhost'} - # The transport config is store in a indepenednt tmp path to not mix up with the files under operating. + # The transport config is store in a independent temporary path per test to not mix up + # with the files under operating. filepath_config = tmp_path_factory.mktemp('transport') / 'config' monkeypatch.setattr(plugin, 'FILEPATH_CONFIG', filepath_config) - if not filepath_config.exists(): - filepath_config.write_text('Host localhost') + + filepath_config.write_text('Host localhost') else: kwargs = {} @@ -496,9 +497,17 @@ def test_put_get_empty_string_file(custom_transport, tmp_path_remote, tmp_path_l # TODO : get doesn't retrieve empty files. # Is it what we want? transport.get(remote_file_abs_path, retrieved_file_abs_path) - # overwrite retrieved_file_name + assert Path(retrieved_file_abs_path).exists() + t1 = Path(retrieved_file_abs_path).stat().st_mtime_ns + + # overwrite retrieved_file_name in 0.01 s + time.sleep(0.01) transport.getfile(remote_file_abs_path, retrieved_file_abs_path) - assert 'file_retrieved.txt' in [p.name for p in (local_dir / directory).iterdir()] + assert Path(retrieved_file_abs_path).exists() + t2 = Path(retrieved_file_abs_path).stat().st_mtime_ns + + # Check st_mtime_ns to sure it is override + assert t2 > t1 def test_put_and_get_tree(custom_transport, tmp_path_remote, tmp_path_local):