From df17a5a87434cb40a1c8eb28355ca3427888cfac Mon Sep 17 00:00:00 2001 From: Joe Ziminski <55797454+JoeZiminski@users.noreply.github.com> Date: Fri, 6 Oct 2023 09:29:59 +0100 Subject: [PATCH 01/57] lots of changes, sort out. --- tests/ssh_test_images/Dockerfile | 16 ++ tests/ssh_test_utils.py | 26 +++ .../test_ssh_file_transfer.py | 180 ++++++++++++------ tests/tests_integration/test_ssh_setup.py | 6 +- 4 files changed, 164 insertions(+), 64 deletions(-) create mode 100644 tests/ssh_test_images/Dockerfile diff --git a/tests/ssh_test_images/Dockerfile b/tests/ssh_test_images/Dockerfile new file mode 100644 index 000000000..dea02ae45 --- /dev/null +++ b/tests/ssh_test_images/Dockerfile @@ -0,0 +1,16 @@ +# Use a base image with the desired OS (e.g., Ubuntu, Debian, etc.) +FROM ubuntu:latest +# Install SSH server +RUN apt-get update && \ + apt-get install -y openssh-server +RUN apt-get install nano +# Create an SSH user +RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser +# Set the SSH user's password (replace "password" with your desired password) +RUN echo 'sshuser:password' | chpasswd +# Allow SSH access +RUN mkdir /var/run/sshd +# Expose the SSH port +EXPOSE 22 +# Start SSH server on container startup +CMD ["/usr/sbin/sshd", "-D"] diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 0838669f3..9b310c503 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -53,3 +53,29 @@ def setup_hostkeys(project): project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True ) restore_mock_input(orig_builtin) + + orig_getpass = copy.deepcopy(ssh.getpass.getpass) + ssh.getpass.getpass = lambda _: "password" + + ssh.setup_ssh_key(project.cfg, log=False) + ssh.getpass.getpass = orig_getpass + + +def build_docker_image(project): + import os + import subprocess + from pathlib import Path + + image_path = Path(__file__).parent / "ssh_test_images" + os.chdir(image_path) + subprocess.run("docker build -t ssh_server .", shell=True) + subprocess.run( + "docker run -d -p 22:22 ssh_server", shell=True + ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + + setup_project_for_ssh( + project, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", + ) diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 393de076c..0207c3edc 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -1,34 +1,36 @@ """ """ import copy -import glob import shutil -import time +import stat from pathlib import Path import pandas as pd +import paramiko import pytest import ssh_test_utils import test_utils from file_conflicts_pathtable import get_pathtable -from pytest import ssh_config +#from pytest import ssh_config +from datashuttle.utils import ssh + +TEST_SSH = True # TODO: base on whether docker / singularity is installed. class TestFileTransfer: @pytest.fixture( scope="class", - params=[ # Set running SSH or local filesystem (see docstring). - # False, + params=[ + # False, pytest.param( True, marks=pytest.mark.skipif( - ssh_config.TEST_SSH is False, - reason="TEST_SSH is set to False.", + TEST_SSH is False, reason="TEST_SSH is set to False." ), ), ], ) - def pathtable_and_project(self, request, tmpdir_factory): + def project_and_test_information(self, request, tmpdir_factory): """ Create a project for SSH testing. Setup the project as normal, and switch configs @@ -72,12 +74,7 @@ def pathtable_and_project(self, request, tmpdir_factory): testing_ssh = request.param tmp_path = tmpdir_factory.mktemp("test") - if testing_ssh: - base_path = ssh_config.FILESYSTEM_PATH - central_path = ssh_config.SERVER_PATH - else: - base_path = tmp_path / "test with space" - central_path = base_path + base_path = tmp_path / "test with space" test_project_name = "test_file_conflicts" project = test_utils.setup_project_fixture( @@ -85,6 +82,7 @@ def pathtable_and_project(self, request, tmpdir_factory): ) if testing_ssh: +<<<<<<< HEAD ssh_test_utils.setup_project_for_ssh( project, test_utils.make_test_path( @@ -95,20 +93,18 @@ def pathtable_and_project(self, request, tmpdir_factory): ) # Initialise the SSH connection +======= + ssh_test_utils.build_docker_image(project) +>>>>>>> b5d54ed (lots of changes, sort out.) ssh_test_utils.setup_hostkeys(project) - shutil.copy(ssh_config.SSH_KEY_PATH, project.cfg.file_path.parent) pathtable = get_pathtable(project.cfg["local_path"]) - test_utils.create_all_pathtable_files(pathtable) - project.testing_ssh = testing_ssh - yield [pathtable, project] + self.create_all_pathtable_files(pathtable) - test_utils.teardown_project(project) + yield [pathtable, project, testing_ssh] - if testing_ssh: - for result in glob.glob(ssh_config.FILESYSTEM_PATH): - shutil.rmtree(result) + test_utils.teardown_project(project) # ------------------------------------------------------------------------- # Utils @@ -156,14 +152,14 @@ def central_from_local(self, path_): ["anat", "behav", "all_non_datatype"], ], ) - @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) +# @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) def test_all_data_transfer_options( self, - pathtable_and_project, + project_and_test_information, sub_names, ses_names, datatype, - upload_or_download, +# upload_or_download, ): """ Parse the arguments to filter the pathtable, getting @@ -175,34 +171,32 @@ def test_all_data_transfer_options( on setting up and swapping local / central paths for upload / download tests. """ - pathtable, project = pathtable_and_project + pathtable, project, testing_ssh = project_and_test_information - transfer_function = test_utils.handle_upload_or_download( - project, - upload_or_download, - transfer_method="custom", - swap_last_folder_only=project.testing_ssh, - )[0] - transfer_function( - "rawdata", sub_names, ses_names, datatype, init_log=False - ) + # transfer_function = test_utils.handle_upload_or_download( + # project, + # upload_or_download, + # swap_last_folder_only=testing_ssh, + # )[0] - if upload_or_download == "download": - test_utils.swap_local_and_central_paths( - project, swap_last_folder_only=project.testing_ssh - ) + project.upload(sub_names, ses_names, datatype, init_log=False) + # transfer_function(sub_names, ses_names, datatype, init_log=False) - sub_names = self.parse_arguments(pathtable, sub_names, "sub") - ses_names = self.parse_arguments(pathtable, ses_names, "ses") - datatype = self.parse_arguments(pathtable, datatype, "datatype") + # if upload_or_download == "download": + # test_utils.swap_local_and_central_paths( + # project, swap_last_folder_only=testing_ssh + # ) - # Filter pathtable to get files that were expected - # to be transferred + parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") + parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") + parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") + + # Filter pathtable to get files that were expected to be transferred ( sub_ses_dtype_arguments, extra_arguments, - ) = self.make_pathtable_search_filter(sub_names, ses_names, datatype) + ) = self.make_pathtable_search_filter(parsed_sub_names, parsed_ses_names, parsed_datatype) datatype_folders = self.query_table(pathtable, sub_ses_dtype_arguments) extra_folders = self.query_table(pathtable, extra_arguments) @@ -217,28 +211,90 @@ def test_all_data_transfer_options( # When transferring with SSH, there is a delay before # filesystem catches up - if project.testing_ssh: - time.sleep(0.5) + # if testing_ssh: + # time.sleep(0.5) # Check what paths were actually moved # (through the local filesystem), and test - path_to_search = ( - self.central_from_local(project.cfg["local_path"]) / "rawdata" - ) - all_transferred = path_to_search.glob("**/*") - paths_to_transferred_files = list( - filter(Path.is_file, all_transferred) - ) + def sftp_recursive_search(sftp, path_, all_filenames): + try: + sftp.stat(path_) + except FileNotFoundError: + return + + for file_or_folder in sftp.listdir_attr(path_): + if stat.S_ISDIR(file_or_folder.st_mode): + sftp_recursive_search( + sftp, + path_ + "/" + file_or_folder.filename, + all_filenames, + ) + else: + all_filenames.append(path_ + "/" + file_or_folder.filename) + + with paramiko.SSHClient() as client: + ssh.connect_client(client, project.cfg) + + sftp = client.open_sftp() + + all_filenames = [] + + sftp_recursive_search( + sftp, + (project.cfg["central_path"] / "rawdata").as_posix(), + all_filenames, + ) - assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths - ) + paths_to_transferred_files = [] + for path_ in all_filenames: + parts = Path(path_).parts + paths_to_transferred_files.append( + Path(*parts[parts.index("rawdata") :]) + ) + + expected_transferred_paths_ = [] + for path_ in expected_transferred_paths: + parts = Path(path_).parts + expected_transferred_paths_.append( + Path(*parts[parts.index("rawdata") :]) + ) + + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths_ + ) + + project.upload_all() + shutil.rmtree(project.cfg["local_path"] / "rawdata") # TOOD: var + + breakpoint() + + true_local_path = project.cfg["local_path"] + tmp_local_path = project.cfg["local_path"] / "tmp_local" + tmp_local_path.mkdirs() + project.update_config("local_path", tmp_local_path) + + project.download(sub_names, ses_names, datatype, init_log=False) # TODO: why is this connecting so many times? + + all_transferred = list((project.cfg["local_path"] / "rawdata").glob("**/*")) + all_transferred = [path_ for path_ in all_transferred if path_.is_file()] + + paths_to_transferred_files = [] + for path_ in all_transferred: # TODO: rename all filenames + parts = Path(path_).parts + paths_to_transferred_files.append( + Path(*parts[parts.index("rawdata"):]) + ) + + assert sorted(paths_to_transferred_files) == sorted(expected_transferred_paths_) + + shutil.rmtree(project.cfg["local_path"]) # TOOD: var + + project.update_config("local_path", true_local_path) + + with paramiko.SSHClient() as client: + ssh.connect_client(client, project.cfg) - # Teardown here, because we have session scope. - try: - shutil.rmtree(self.central_from_local(project.cfg["local_path"])) - except FileNotFoundError: - pass + client.exec_command(f"rm -rf {(project.cfg['central_path'] / 'rawdata').as_posix()}") # TODO: own function as need to do on teardown) # --------------------------------------------------------------------------------------------------------------- # Utils diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 752985a67..293b36510 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -7,12 +7,14 @@ import pytest import ssh_test_utils import test_utils -from pytest import ssh_config +# from pytest import ssh_config from datashuttle.utils import ssh +TEST_SSH = False -@pytest.mark.skipif(ssh_config.TEST_SSH is False, reason="TEST_SSH is false") + +@pytest.mark.skipif(TEST_SSH is False, reason="TEST_SSH is false") class TestSSH: @pytest.fixture(scope="function") def project(test, tmp_path): From 3275487c51f3cb143b9e08c6373b90233a2e304a Mon Sep 17 00:00:00 2001 From: Joe Ziminski <55797454+JoeZiminski@users.noreply.github.com> Date: Mon, 9 Oct 2023 03:05:31 +0100 Subject: [PATCH 02/57] still working, needs a bit more tidying up. --- datashuttle/utils/data_transfer.py | 1 - tests/ssh_test_utils.py | 39 +- .../test_ssh_file_transfer.py | 375 ++++++++---------- 3 files changed, 210 insertions(+), 205 deletions(-) diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 21121b8e6..c617f4c13 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -157,7 +157,6 @@ def build_a_list_of_all_files_and_folders_to_transfer(self) -> List[str]: self.update_list_with_non_ses_sub_level_folders( extra_folder_names, extra_filenames, sub ) - continue # Datatype (sub and ses level) -------------------------------- diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 9b310c503..539efb4d7 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,5 +1,8 @@ import builtins import copy +import stat + +import paramiko from datashuttle.utils import rclone, ssh @@ -55,7 +58,7 @@ def setup_hostkeys(project): restore_mock_input(orig_builtin) orig_getpass = copy.deepcopy(ssh.getpass.getpass) - ssh.getpass.getpass = lambda _: "password" + ssh.getpass.getpass = lambda _: "password" # type: ignore ssh.setup_ssh_key(project.cfg, log=False) ssh.getpass.getpass = orig_getpass @@ -79,3 +82,37 @@ def build_docker_image(project): central_host_id="localhost", central_host_username="sshuser", ) + + +def sftp_recursive_file_search(sftp, path_, all_filenames): + try: + sftp.stat(path_) + except FileNotFoundError: + return + + for file_or_folder in sftp.listdir_attr(path_): + if stat.S_ISDIR(file_or_folder.st_mode): + sftp_recursive_file_search( + sftp, + path_ + "/" + file_or_folder.filename, + all_filenames, + ) + else: + all_filenames.append(path_ + "/" + file_or_folder.filename) + + +def recursive_search_central(project): + """ """ + with paramiko.SSHClient() as client: + ssh.connect_client(client, project.cfg) + + sftp = client.open_sftp() + + all_filenames = [] + + sftp_recursive_file_search( + sftp, + (project.cfg["central_path"] / "rawdata").as_posix(), + all_filenames, + ) + return all_filenames diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 0207c3edc..44d525bc1 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -2,7 +2,6 @@ import copy import shutil -import stat from pathlib import Path import pandas as pd @@ -17,61 +16,41 @@ TEST_SSH = True # TODO: base on whether docker / singularity is installed. + +PARAM_SUBS = [ + ["all"], + ["all_sub"], + ["all_non_sub"], + ["sub-001"], + ["sub-003_date-20231901"], + ["sub-002", "all_non_sub"], +] +PARAM_SES = [ + ["all"], + ["all_non_ses"], + ["all_ses"], + ["ses-001"], + ["ses-002_random-key"], + ["all_non_ses", "ses-001"], +] +PARAM_DATATYPE = [ + ["all"], + ["all_non_datatype"], + ["all_datatype"], + ["behav"], + ["ephys"], + ["anat"], + ["funcimg"], + ["anat", "behav", "all_non_datatype"], +] + + class TestFileTransfer: @pytest.fixture( scope="class", - params=[ - # False, - pytest.param( - True, - marks=pytest.mark.skipif( - TEST_SSH is False, reason="TEST_SSH is set to False." - ), - ), - ], ) - def project_and_test_information(self, request, tmpdir_factory): - """ - Create a project for SSH testing. Setup - the project as normal, and switch configs - to use SSH connection. - - Although SSH is used for transfer, for SSH tests, - checking the created filepaths is always - done through the local filesystem for speed - and convenience. As such, the drive that is - SSH to must also be mounted and the path - supplied to the location SSH'd to. - - For speed, create the project once, - and all files to transfer. Then in the - test function, the folder are transferred. - Partial cleanup is done in the test function - i.e. deleting the central_path to which the - items have been transferred. This is achieved - by using "class" scope. - - NOTES - ----- - - Pytest params - The `params` key sets the - `params` attribute on the pytest `request` fixture. - This attribute is used to set the `testing_ssh` variable - to `True` or `False`. In the first run, this is set to - `False`, meaning local filesystem tests are run. In the - second run, this is set with a pytest parameter that is - `True` (i.e. SSH tests are run) but is skipped if `TEST_SSH` - in `ssh_config` (set in conftest.py` is `False`. - - - For convenience, files are transferred - with SSH and then checked through the local filesystem - mount. This is significantly easier than checking - everything through SFTP. However, on Windows the - mounted filesystem is quite slow to update, taking - a few seconds after SSH transfer. This makes the - tests run very slowly. We can get rid - of this limitation on linux. - """ - testing_ssh = request.param + def pathtable_and_project(self, tmpdir_factory): + """ """ tmp_path = tmpdir_factory.mktemp("test") base_path = tmp_path / "test with space" @@ -82,7 +61,6 @@ def project_and_test_information(self, request, tmpdir_factory): ) if testing_ssh: -<<<<<<< HEAD ssh_test_utils.setup_project_for_ssh( project, test_utils.make_test_path( @@ -93,19 +71,30 @@ def project_and_test_information(self, request, tmpdir_factory): ) # Initialise the SSH connection -======= ssh_test_utils.build_docker_image(project) ->>>>>>> b5d54ed (lots of changes, sort out.) + ssh_test_utils.setup_hostkeys(project) pathtable = get_pathtable(project.cfg["local_path"]) self.create_all_pathtable_files(pathtable) - yield [pathtable, project, testing_ssh] + yield [pathtable, project] test_utils.teardown_project(project) + @pytest.fixture( + scope="class", + ) + def ssh_setup(self, pathtable_and_project): + pathtable, project = pathtable_and_project + ssh_test_utils.build_docker_image(project) + ssh_test_utils.setup_hostkeys(project) + + project.upload_all() + + return [pathtable, project] + # ------------------------------------------------------------------------- # Utils # ------------------------------------------------------------------------- @@ -117,189 +106,169 @@ def central_from_local(self, path_): # Test File Transfer - All Options # ------------------------------------------------------------------------- - @pytest.mark.parametrize( - "sub_names", - [ - ["all"], - ["all_sub"], - ["all_non_sub"], - ["sub-001"], - ["sub-003_date-20231901"], - ["sub-002", "all_non_sub"], - ], - ) - @pytest.mark.parametrize( - "ses_names", - [ - ["all"], - ["all_non_ses"], - ["all_ses"], - ["ses-001"], - ["ses-002_random-key"], - ["all_non_ses", "ses-001"], - ], - ) - @pytest.mark.parametrize( - "datatype", - [ - ["all"], - ["all_non_datatype"], - ["all_datatype"], - ["behav"], - ["ephys"], - ["anat"], - ["funcimg"], - ["anat", "behav", "all_non_datatype"], - ], - ) -# @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) - def test_all_data_transfer_options( + @pytest.mark.parametrize("sub_names", PARAM_SUBS) + @pytest.mark.parametrize("ses_names", PARAM_SES) + @pytest.mark.parametrize("datatype", PARAM_DATATYPE) + @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) + def test_combinations_filesystem_transfer( self, - project_and_test_information, + pathtable_and_project, sub_names, ses_names, datatype, -# upload_or_download, + upload_or_download, ): - """ - Parse the arguments to filter the pathtable, getting - the files expected to be transferred passed on the arguments - Note files in sub/ses/datatype folders must be handled - separately to those in non-sub, non-ses, non-datatype folders - - see test_utils.swap_local_and_central_paths() for the logic - on setting up and swapping local / central paths for - upload / download tests. - """ - pathtable, project, testing_ssh = project_and_test_information - + """ """ + pathtable, project = pathtable_and_project - # transfer_function = test_utils.handle_upload_or_download( - # project, - # upload_or_download, - # swap_last_folder_only=testing_ssh, - # )[0] + transfer_function = test_utils.handle_upload_or_download( + project, + upload_or_download, + transfer_method="custom", + swap_last_folder_only=False, + )[0] - project.upload(sub_names, ses_names, datatype, init_log=False) - # transfer_function(sub_names, ses_names, datatype, init_log=False) - - # if upload_or_download == "download": - # test_utils.swap_local_and_central_paths( - # project, swap_last_folder_only=testing_ssh - # ) - - parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") - parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") - parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") + transfer_function("rawdata", sub_names, ses_names, datatype, init_log=False) - # Filter pathtable to get files that were expected to be transferred - ( - sub_ses_dtype_arguments, - extra_arguments, - ) = self.make_pathtable_search_filter(parsed_sub_names, parsed_ses_names, parsed_datatype) - - datatype_folders = self.query_table(pathtable, sub_ses_dtype_arguments) - extra_folders = self.query_table(pathtable, extra_arguments) - - expected_paths = pd.concat([datatype_folders, extra_folders]) - expected_paths = expected_paths.drop_duplicates(subset="path") + if upload_or_download == "download": + test_utils.swap_local_and_central_paths( + project, swap_last_folder_only=False + ) - central_base_paths = expected_paths.base_folder.map( - lambda x: str(x).replace("local", "central") + expected_transferred_paths = self.get_expected_transferred_paths( + pathtable, sub_names, ses_names, datatype ) - expected_transferred_paths = central_base_paths / expected_paths.path - - # When transferring with SSH, there is a delay before - # filesystem catches up - # if testing_ssh: - # time.sleep(0.5) # Check what paths were actually moved # (through the local filesystem), and test - def sftp_recursive_search(sftp, path_, all_filenames): - try: - sftp.stat(path_) - except FileNotFoundError: - return - - for file_or_folder in sftp.listdir_attr(path_): - if stat.S_ISDIR(file_or_folder.st_mode): - sftp_recursive_search( - sftp, - path_ + "/" + file_or_folder.filename, - all_filenames, - ) - else: - all_filenames.append(path_ + "/" + file_or_folder.filename) + path_to_search = ( + self.central_from_local(project.cfg["local_path"]) / "rawdata" + ) + all_transferred = path_to_search.glob("**/*") + paths_to_transferred_files = list( + filter(Path.is_file, all_transferred) + ) - with paramiko.SSHClient() as client: - ssh.connect_client(client, project.cfg) + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths + ) - sftp = client.open_sftp() + # Teardown here, because we have session scope. + try: + shutil.rmtree(self.central_from_local(project.cfg["local_path"])) + except FileNotFoundError: + pass - all_filenames = [] + @pytest.mark.parametrize("sub_names", PARAM_SUBS) + @pytest.mark.parametrize("ses_names", PARAM_SES) + @pytest.mark.parametrize("datatype", PARAM_DATATYPE) + def test_combinations_ssh_transfer( + self, + ssh_setup, + sub_names, + ses_names, + datatype, + ): + """ """ + pathtable, project = ssh_setup - sftp_recursive_search( - sftp, - (project.cfg["central_path"] / "rawdata").as_posix(), - all_filenames, - ) + true_central_path = project.cfg["central_path"] + tmp_central_path = project.cfg["central_path"] / "tmp" + project.update_config("central_path", tmp_central_path) - paths_to_transferred_files = [] - for path_ in all_filenames: - parts = Path(path_).parts - paths_to_transferred_files.append( - Path(*parts[parts.index("rawdata") :]) - ) + breakpoint() + project.upload(sub_names, ses_names, datatype, init_log=False) - expected_transferred_paths_ = [] - for path_ in expected_transferred_paths: - parts = Path(path_).parts - expected_transferred_paths_.append( - Path(*parts[parts.index("rawdata") :]) - ) + expected_transferred_paths = self.get_expected_transferred_paths( + pathtable, sub_names, ses_names, datatype + ) - assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths_ - ) + transferred_files = ssh_test_utils.recursive_search_central(project) - project.upload_all() - shutil.rmtree(project.cfg["local_path"] / "rawdata") # TOOD: var + paths_to_transferred_files = self.remove_path_before_rawdata( + transferred_files + ) - breakpoint() + expected_transferred_paths_ = self.remove_path_before_rawdata( + expected_transferred_paths + ) + + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths_ + ) + + with paramiko.SSHClient() as client: + ssh.connect_client(client, project.cfg) + client.exec_command( + f"rm -rf {(tmp_central_path).as_posix()}" + ) # TODO: own function as need to do on teardown) true_local_path = project.cfg["local_path"] - tmp_local_path = project.cfg["local_path"] / "tmp_local" - tmp_local_path.mkdirs() + tmp_local_path = project.cfg["local_path"] / "tmp" + tmp_local_path.mkdir() project.update_config("local_path", tmp_local_path) + project.update_config("central_path", true_central_path) - project.download(sub_names, ses_names, datatype, init_log=False) # TODO: why is this connecting so many times? + project.download( + sub_names, ses_names, datatype, init_log=False + ) # TODO: why is this connecting so many times? [during search - make issue] - all_transferred = list((project.cfg["local_path"] / "rawdata").glob("**/*")) - all_transferred = [path_ for path_ in all_transferred if path_.is_file()] - - paths_to_transferred_files = [] - for path_ in all_transferred: # TODO: rename all filenames - parts = Path(path_).parts - paths_to_transferred_files.append( - Path(*parts[parts.index("rawdata"):]) - ) + all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) + all_transferred = [ + path_ for path_ in all_transferred if path_.is_file() + ] - assert sorted(paths_to_transferred_files) == sorted(expected_transferred_paths_) + paths_to_transferred_files = self.remove_path_before_rawdata( + all_transferred + ) - shutil.rmtree(project.cfg["local_path"]) # TOOD: var + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths_ + ) + shutil.rmtree(tmp_local_path) project.update_config("local_path", true_local_path) - with paramiko.SSHClient() as client: - ssh.connect_client(client, project.cfg) - - client.exec_command(f"rm -rf {(project.cfg['central_path'] / 'rawdata').as_posix()}") # TODO: own function as need to do on teardown) - # --------------------------------------------------------------------------------------------------------------- # Utils # --------------------------------------------------------------------------------------------------------------- + def get_expected_transferred_paths( + self, pathtable, sub_names, ses_names, datatype + ): + """""" + parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") + parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") + parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") + + # Filter pathtable to get files that were expected to be transferred + ( + sub_ses_dtype_arguments, + extra_arguments, + ) = self.make_pathtable_search_filter( + parsed_sub_names, parsed_ses_names, parsed_datatype + ) + + datatype_folders = self.query_table(pathtable, sub_ses_dtype_arguments) + extra_folders = self.query_table(pathtable, extra_arguments) + + expected_paths = pd.concat([datatype_folders, extra_folders]) + expected_paths = expected_paths.drop_duplicates(subset="path") + + central_base_paths = expected_paths.base_folder.map( + lambda x: str(x).replace("local", "central") + ) + expected_transferred_paths = central_base_paths / expected_paths.path + + return expected_transferred_paths + + def remove_path_before_rawdata(self, list_of_paths): + cut_paths = [] + for path_ in list_of_paths: # TODO: rename all filenames + parts = Path(path_).parts + cut_paths.append(Path(*parts[parts.index("rawdata") :])) + return cut_paths + def query_table(self, pathtable, arguments): """ Search the table for arguments, return empty From accd30d580d995efa83f195aded78c60ea7838fd Mon Sep 17 00:00:00 2001 From: Joe Ziminski <55797454+JoeZiminski@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:48:46 +0100 Subject: [PATCH 03/57] Continue working. --- tests/tests_integration/test_ssh_setup.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 293b36510..c86ee8434 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -11,7 +11,7 @@ # from pytest import ssh_config from datashuttle.utils import ssh -TEST_SSH = False +TEST_SSH = True @pytest.mark.skipif(TEST_SSH is False, reason="TEST_SSH is false") @@ -29,9 +29,9 @@ def project(test, tmp_path): ssh_test_utils.setup_project_for_ssh( project, - ssh_config.FILESYSTEM_PATH, - ssh_config.CENTRAL_HOST_ID, - ssh_config.USERNAME, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", # TODO: centralise these + central_host_id="localhost", + central_host_username="sshuser", ) yield project From dfd4f8255b8ece0322930509811a969b8f7f3ac4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 24 Oct 2023 12:15:38 +0000 Subject: [PATCH 04/57] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/tests_integration/test_ssh_file_transfer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 44d525bc1..c2e2b6c76 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -10,8 +10,8 @@ import ssh_test_utils import test_utils from file_conflicts_pathtable import get_pathtable -#from pytest import ssh_config +# from pytest import ssh_config from datashuttle.utils import ssh TEST_SSH = True # TODO: base on whether docker / singularity is installed. From 7f0326eed914b57018ba569254935cc15ba5ab8e Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 10 Nov 2023 09:12:46 +0000 Subject: [PATCH 05/57] Tidy ups. --- tests/ssh_test_utils.py | 80 ++++++++++++++++--- .../test_ssh_file_transfer.py | 28 ++++--- tests/tests_integration/test_ssh_setup.py | 25 ++---- 3 files changed, 90 insertions(+), 43 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 539efb4d7..3e47da4f3 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,7 +1,13 @@ import builtins import copy +import platform import stat - +import subprocess +import sys +import warnings +import os +import subprocess +from pathlib import Path import paramiko from datashuttle.utils import rclone, ssh @@ -47,33 +53,54 @@ def restore_mock_input(orig_builtin): builtins.input = orig_builtin -def setup_hostkeys(project): +def setup_hostkeys(project, setup_ssh_key_pair=True): # TODO: RENAME FUNCTION """ Convenience function to verify the server hostkey. + + This requires monkeypatching a number of functions involved + in the SSH setup process. `input()` is patched to always + return the required hostkey confirmation "y". `getpass()` is + patched to allways return the password for the container in which + SSH tests are run. `isatty()` is patched because when running this + for some reason it appears to be in a TTY - this might be a + container thing. """ + # Monkeypatch orig_builtin = setup_mock_input(input_="y") - ssh.verify_ssh_central_host( - project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True - ) - restore_mock_input(orig_builtin) orig_getpass = copy.deepcopy(ssh.getpass.getpass) ssh.getpass.getpass = lambda _: "password" # type: ignore - ssh.setup_ssh_key(project.cfg, log=False) + orig_isatty = copy.deepcopy(sys.stdin.isatty) + sys.stdin.isatty = lambda: True + + # Run setup + verified = ssh.verify_ssh_central_host( + project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True + ) + + if setup_ssh_key_pair: + ssh.setup_ssh_key(project.cfg, log=False) + + # Restore functions + restore_mock_input(orig_builtin) ssh.getpass.getpass = orig_getpass + sys.stdin.isatty = orig_isatty + + return verified def build_docker_image(project): - import os - import subprocess - from pathlib import Path + """""" + container_software = is_docker_or_singularity_installed() + assert container_software is not False, ("docker or singularity not installed, " + "this should be checked at the top of test script") image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - subprocess.run("docker build -t ssh_server .", shell=True) + subprocess.run(f"{container_software} build -t ssh_server .", shell=True) subprocess.run( - "docker run -d -p 22:22 ssh_server", shell=True + f"{container_software} run -d -p 22:22 ssh_server", shell=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server setup_project_for_ssh( @@ -116,3 +143,32 @@ def recursive_search_central(project): all_filenames, ) return all_filenames + + +def get_test_ssh(): + """""" + if is_docker_or_singularity_installed(): + test_ssh = True + else: + warnings.warn("SSH tests are not run as docker (Windows, macOS) " + "or singularity (Linux) is not installed.") + test_ssh = False + + return test_ssh + + +def is_docker_or_singularity_installed(): # TODO: need to test + """""" + check_install = lambda command: subprocess.run( + command, shell=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL + ).returncode == 0 + + installed = False + if platform.system() == "Linux": + if check_install("singularity version"): + installed = "singularity" + else: + if check_install("docker -v"): + installed = "docker" + + return installed diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index c2e2b6c76..1ab888ed3 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -11,10 +11,9 @@ import test_utils from file_conflicts_pathtable import get_pathtable -# from pytest import ssh_config from datashuttle.utils import ssh -TEST_SSH = True # TODO: base on whether docker / singularity is installed. +TEST_SSH = ssh_test_utils.get_test_ssh() PARAM_SUBS = [ @@ -159,9 +158,10 @@ def test_combinations_filesystem_transfer( except FileNotFoundError: pass - @pytest.mark.parametrize("sub_names", PARAM_SUBS) - @pytest.mark.parametrize("ses_names", PARAM_SES) - @pytest.mark.parametrize("datatype", PARAM_DATATYPE) + @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") + @pytest.mark.parametrize("sub_names", [["all"], ["all_non_sub", "sub-002"]]) + @pytest.mark.parametrize("ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]]) + @pytest.mark.parametrize("datatype", [["all"], ["anat", "all_ses_level_non_datatype"]]) def test_combinations_ssh_transfer( self, ssh_setup, @@ -169,14 +169,17 @@ def test_combinations_ssh_transfer( ses_names, datatype, ): - """ """ + """ + This is very slow. maybe 8 s per test. Nearly all in the + upload() and download() part so unavoidable. Most code is shared, + this should be okay though my paranoid + """ pathtable, project = ssh_setup true_central_path = project.cfg["central_path"] - tmp_central_path = project.cfg["central_path"] / "tmp" + tmp_central_path = project.cfg["central_path"] / "tmp" / project.project_name project.update_config("central_path", tmp_central_path) - breakpoint() project.upload(sub_names, ses_names, datatype, init_log=False) expected_transferred_paths = self.get_expected_transferred_paths( @@ -201,17 +204,18 @@ def test_combinations_ssh_transfer( ssh.connect_client(client, project.cfg) client.exec_command( f"rm -rf {(tmp_central_path).as_posix()}" - ) # TODO: own function as need to do on teardown) + ) true_local_path = project.cfg["local_path"] - tmp_local_path = project.cfg["local_path"] / "tmp" - tmp_local_path.mkdir() + tmp_local_path = project.cfg["local_path"] / "tmp" / project.project_name + tmp_local_path.mkdir(exist_ok=True, parents=True) + project.update_config("local_path", tmp_local_path) project.update_config("central_path", true_central_path) project.download( sub_names, ses_names, datatype, init_log=False - ) # TODO: why is this connecting so many times? [during search - make issue] + ) all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) all_transferred = [ diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index c86ee8434..27c141f8e 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -1,9 +1,3 @@ -""" -SSH configs are set in conftest.py . The password -should be stored in a file called test_ssh_password.txt located -in the same folder as test_ssh.py -""" - import pytest import ssh_test_utils import test_utils @@ -11,10 +5,10 @@ # from pytest import ssh_config from datashuttle.utils import ssh -TEST_SSH = True +TEST_SSH = ssh_test_utils.get_test_ssh() -@pytest.mark.skipif(TEST_SSH is False, reason="TEST_SSH is false") +@pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSH: @pytest.fixture(scope="function") def project(test, tmp_path): @@ -27,12 +21,7 @@ def project(test, tmp_path): test_project_name = "test_ssh" project = test_utils.setup_project_fixture(tmp_path, test_project_name) - ssh_test_utils.setup_project_for_ssh( - project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", # TODO: centralise these - central_host_id="localhost", - central_host_username="sshuser", - ) + ssh_test_utils.build_docker_image(project) # TODO: rename function yield project test_utils.teardown_project(project) @@ -69,16 +58,14 @@ def test_verify_ssh_central_host_accept(self, capsys, project): and check hostkey is successfully accepted and written to configs. """ test_utils.clear_capsys(capsys) - orig_builtin = ssh_test_utils.setup_mock_input(input_="y") - verified = ssh.verify_ssh_central_host( - project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True + verified = ssh_test_utils.setup_hostkeys( + project, setup_ssh_key_pair=False ) - ssh_test_utils.restore_mock_input(orig_builtin) - assert verified captured = capsys.readouterr() + assert captured.out == "Host accepted.\n" with open(project.cfg.hostkeys_path, "r") as file: From f12683c3242d5b62ca44843c69ac167e2e01636e Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 10 Nov 2023 09:17:35 +0000 Subject: [PATCH 06/57] Fix linting. --- tests/ssh_test_utils.py | 30 ++++++++++++------- .../test_ssh_file_transfer.py | 28 ++++++++++------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 3e47da4f3..e1e14874c 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,13 +1,13 @@ import builtins import copy +import os import platform import stat import subprocess import sys import warnings -import os -import subprocess from pathlib import Path + import paramiko from datashuttle.utils import rclone, ssh @@ -60,7 +60,7 @@ def setup_hostkeys(project, setup_ssh_key_pair=True): # TODO: RENAME FUNCTION This requires monkeypatching a number of functions involved in the SSH setup process. `input()` is patched to always return the required hostkey confirmation "y". `getpass()` is - patched to allways return the password for the container in which + patched to always return the password for the container in which SSH tests are run. `isatty()` is patched because when running this for some reason it appears to be in a TTY - this might be a container thing. @@ -93,8 +93,10 @@ def setup_hostkeys(project, setup_ssh_key_pair=True): # TODO: RENAME FUNCTION def build_docker_image(project): """""" container_software = is_docker_or_singularity_installed() - assert container_software is not False, ("docker or singularity not installed, " - "this should be checked at the top of test script") + assert container_software is not False, ( + "docker or singularity not installed, " + "this should be checked at the top of test script" + ) image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) @@ -150,8 +152,10 @@ def get_test_ssh(): if is_docker_or_singularity_installed(): test_ssh = True else: - warnings.warn("SSH tests are not run as docker (Windows, macOS) " - "or singularity (Linux) is not installed.") + warnings.warn( + "SSH tests are not run as docker (Windows, macOS) " + "or singularity (Linux) is not installed." + ) test_ssh = False return test_ssh @@ -159,9 +163,15 @@ def get_test_ssh(): def is_docker_or_singularity_installed(): # TODO: need to test """""" - check_install = lambda command: subprocess.run( - command, shell=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL - ).returncode == 0 + check_install = ( + lambda command: subprocess.run( + command, + shell=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ).returncode + == 0 + ) installed = False if platform.system() == "Linux": diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 1ab888ed3..d5e505d4e 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -159,9 +159,15 @@ def test_combinations_filesystem_transfer( pass @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") - @pytest.mark.parametrize("sub_names", [["all"], ["all_non_sub", "sub-002"]]) - @pytest.mark.parametrize("ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]]) - @pytest.mark.parametrize("datatype", [["all"], ["anat", "all_ses_level_non_datatype"]]) + @pytest.mark.parametrize( + "sub_names", [["all"], ["all_non_sub", "sub-002"]] + ) + @pytest.mark.parametrize( + "ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]] + ) + @pytest.mark.parametrize( + "datatype", [["all"], ["anat", "all_ses_level_non_datatype"]] + ) def test_combinations_ssh_transfer( self, ssh_setup, @@ -177,7 +183,9 @@ def test_combinations_ssh_transfer( pathtable, project = ssh_setup true_central_path = project.cfg["central_path"] - tmp_central_path = project.cfg["central_path"] / "tmp" / project.project_name + tmp_central_path = ( + project.cfg["central_path"] / "tmp" / project.project_name + ) project.update_config("central_path", tmp_central_path) project.upload(sub_names, ses_names, datatype, init_log=False) @@ -202,20 +210,18 @@ def test_combinations_ssh_transfer( with paramiko.SSHClient() as client: ssh.connect_client(client, project.cfg) - client.exec_command( - f"rm -rf {(tmp_central_path).as_posix()}" - ) + client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") true_local_path = project.cfg["local_path"] - tmp_local_path = project.cfg["local_path"] / "tmp" / project.project_name + tmp_local_path = ( + project.cfg["local_path"] / "tmp" / project.project_name + ) tmp_local_path.mkdir(exist_ok=True, parents=True) project.update_config("local_path", tmp_local_path) project.update_config("central_path", true_central_path) - project.download( - sub_names, ses_names, datatype, init_log=False - ) + project.download(sub_names, ses_names, datatype, init_log=False) all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) all_transferred = [ From e03f72df948c9673e455ae927816934aa285ece9 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 10 Nov 2023 11:10:51 +0000 Subject: [PATCH 07/57] Tidy ups and documentation. --- tests/ssh_test_utils.py | 6 +- .../test_ssh_file_transfer.py | 148 +++++++++++------- tests/tests_integration/test_ssh_setup.py | 4 +- 3 files changed, 96 insertions(+), 62 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index e1e14874c..b4fa2a423 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -53,7 +53,7 @@ def restore_mock_input(orig_builtin): builtins.input = orig_builtin -def setup_hostkeys(project, setup_ssh_key_pair=True): # TODO: RENAME FUNCTION +def setup_ssh_connection(project, setup_ssh_key_pair=True): """ Convenience function to verify the server hostkey. @@ -90,7 +90,7 @@ def setup_hostkeys(project, setup_ssh_key_pair=True): # TODO: RENAME FUNCTION return verified -def build_docker_image(project): +def setup_project_and_container_for_ssh(project): """""" container_software = is_docker_or_singularity_installed() assert container_software is not False, ( @@ -161,7 +161,7 @@ def get_test_ssh(): return test_ssh -def is_docker_or_singularity_installed(): # TODO: need to test +def is_docker_or_singularity_installed(): """""" check_install = ( lambda command: subprocess.run( diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index d5e505d4e..68dace731 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -86,24 +86,25 @@ def pathtable_and_project(self, tmpdir_factory): scope="class", ) def ssh_setup(self, pathtable_and_project): + """ """ pathtable, project = pathtable_and_project - ssh_test_utils.build_docker_image(project) - ssh_test_utils.setup_hostkeys(project) + ssh_test_utils.setup_project_and_container_for_ssh(project) + ssh_test_utils.setup_ssh_connection(project) project.upload_all() return [pathtable, project] - # ------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- # Utils - # ------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- def central_from_local(self, path_): return Path(str(copy.copy(path_)).replace("local", "central")) - # ------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- # Test File Transfer - All Options - # ------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- @pytest.mark.parametrize("sub_names", PARAM_SUBS) @pytest.mark.parametrize("ses_names", PARAM_SES) @@ -120,6 +121,9 @@ def test_combinations_filesystem_transfer( """ """ pathtable, project = pathtable_and_project + # Transfer the data, swapping the paths to move a subset of + # files from the already set up directory to a new directory + # using upload or download. transfer_function = test_utils.handle_upload_or_download( project, upload_or_download, @@ -144,10 +148,15 @@ def test_combinations_filesystem_transfer( self.central_from_local(project.cfg["local_path"]) / "rawdata" ) all_transferred = path_to_search.glob("**/*") + paths_to_transferred_files = list( filter(Path.is_file, all_transferred) ) + paths_to_transferred_files = self.remove_path_before_rawdata( + paths_to_transferred_files + ) + assert sorted(paths_to_transferred_files) == sorted( expected_transferred_paths ) @@ -182,6 +191,8 @@ def test_combinations_ssh_transfer( """ pathtable, project = ssh_setup + # Upload data from the setup local project to a temporary + # central directory. true_central_path = project.cfg["central_path"] tmp_central_path = ( project.cfg["central_path"] / "tmp" / project.project_name @@ -194,24 +205,20 @@ def test_combinations_ssh_transfer( pathtable, sub_names, ses_names, datatype ) + # Search the paths that were transferred and tidy them up, + # then check against the paths that were expected to be transferred. transferred_files = ssh_test_utils.recursive_search_central(project) paths_to_transferred_files = self.remove_path_before_rawdata( transferred_files ) - expected_transferred_paths_ = self.remove_path_before_rawdata( - expected_transferred_paths - ) - assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths_ + expected_transferred_paths ) - with paramiko.SSHClient() as client: - ssh.connect_client(client, project.cfg) - client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") - + # Now, move data from the central path where the project is + # setup, to a temp local folder to test download. true_local_path = project.cfg["local_path"] tmp_local_path = ( project.cfg["local_path"] / "tmp" / project.project_name @@ -223,6 +230,8 @@ def test_combinations_ssh_transfer( project.download(sub_names, ses_names, datatype, init_log=False) + # Find the transferred paths, tidy them up + # and check expected paths were transferred. all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) all_transferred = [ path_ for path_ in all_transferred if path_.is_file() @@ -233,20 +242,30 @@ def test_combinations_ssh_transfer( ) assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths_ + expected_transferred_paths ) + # Clean up, removing the temp directories and + # resetting the project paths. + with paramiko.SSHClient() as client: + ssh.connect_client(client, project.cfg) + client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") + shutil.rmtree(tmp_local_path) + project.update_config("local_path", true_local_path) - # --------------------------------------------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- # Utils - # --------------------------------------------------------------------------------------------------------------- + # ---------------------------------------------------------------------------------- def get_expected_transferred_paths( self, pathtable, sub_names, ses_names, datatype ): - """""" + """ + Process the expected files that are transferred using the logic in + `make_pathtable_search_filter()` to + """ parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") @@ -265,16 +284,56 @@ def get_expected_transferred_paths( expected_paths = pd.concat([datatype_folders, extra_folders]) expected_paths = expected_paths.drop_duplicates(subset="path") - central_base_paths = expected_paths.base_folder.map( - lambda x: str(x).replace("local", "central") - ) - expected_transferred_paths = central_base_paths / expected_paths.path + expected_paths = self.remove_path_before_rawdata(expected_paths) - return expected_transferred_paths + return expected_paths + + def make_pathtable_search_filter(self, sub_names, ses_names, datatype): + """ + Create a string of arguments to pass to pd.query() that will + create the table of only transferred sub, ses and datatype. + + Two arguments must be created, one of all sub / ses / datatypes + and the other of all non sub/ non ses / non datatype + folders. These must be handled separately as they are + mutually exclusive. + """ + sub_ses_dtype_arguments = [] + extra_arguments = [] + + for sub in sub_names: + if sub == "all_non_sub": + extra_arguments += ["is_non_sub == True"] + else: + for ses in ses_names: + if ses == "all_non_ses": + extra_arguments += [ + f"(parent_sub == '{sub}' & is_non_ses == True)" + ] + else: + for dtype in datatype: + if dtype == "all_ses_level_non_datatype": + extra_arguments += [ + f"(parent_sub == '{sub}' & parent_ses == '{ses}' " + f"& is_ses_level_non_datatype == True)" + ] + else: + sub_ses_dtype_arguments += [ + f"(parent_sub == '{sub}' & parent_ses == '{ses}' " + f"& (parent_datatype == '{dtype}' " + f"| parent_datatype == '{dtype}'))" + ] + + return sub_ses_dtype_arguments, extra_arguments def remove_path_before_rawdata(self, list_of_paths): + """ + Remove the path to project files before the "rawdata" so + they can be compared no matter where the project was stored + (e.g. on a central server vs. local filesystem). + """ cut_paths = [] - for path_ in list_of_paths: # TODO: rename all filenames + for path_ in list_of_paths: parts = Path(path_).parts cut_paths.append(Path(*parts[parts.index("rawdata") :])) return cut_paths @@ -310,37 +369,12 @@ def parse_arguments(self, pathtable, list_of_names, field): list_of_names = entries return list_of_names - def make_pathtable_search_filter(self, sub_names, ses_names, datatype): + def create_all_pathtable_files(self, pathtable): """ - Create a string of arguments to pass to pd.query() that will - create the table of only transferred sub, ses and datatype. - - Two arguments must be created, one of all sub / ses / datatypes - and the other of all non sub/ non ses / non datatype - folders. These must be handled separately as they are - mutually exclusive. + Create the entire test project in the defined + location (usually project's `local_path`). """ - sub_ses_dtype_arguments = [] - extra_arguments = [] - - for sub in sub_names: - if sub == "all_non_sub": - extra_arguments += ["is_non_sub == True"] - else: - for ses in ses_names: - if ses == "all_non_ses": - extra_arguments += [ - f"(parent_sub == '{sub}' & is_non_ses == True)" - ] - else: - for dtype in datatype: - if dtype == "all_non_datatype": - extra_arguments += [ - f"(parent_sub == '{sub}' & parent_ses == '{ses}' & is_ses_level_non_datatype == True)" - ] - else: - sub_ses_dtype_arguments += [ - f"(parent_sub == '{sub}' & parent_ses == '{ses}' & (parent_datatype == '{dtype}' | parent_datatype == '{dtype}'))" - ] - - return sub_ses_dtype_arguments, extra_arguments + for i in range(pathtable.shape[0]): + filepath = pathtable["base_folder"][i] / pathtable["path"][i] + filepath.parents[0].mkdir(parents=True, exist_ok=True) + test_utils.write_file(filepath, contents="test_entry") diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 27c141f8e..3fcc4e36a 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -21,7 +21,7 @@ def project(test, tmp_path): test_project_name = "test_ssh" project = test_utils.setup_project_fixture(tmp_path, test_project_name) - ssh_test_utils.build_docker_image(project) # TODO: rename function + ssh_test_utils.setup_project_and_container_for_ssh(project) yield project test_utils.teardown_project(project) @@ -59,7 +59,7 @@ def test_verify_ssh_central_host_accept(self, capsys, project): """ test_utils.clear_capsys(capsys) - verified = ssh_test_utils.setup_hostkeys( + verified = ssh_test_utils.setup_ssh_connection( project, setup_ssh_key_pair=False ) From 5ff22c3ec73c9611082f65f5cdfdd94e8298c8e2 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 10 Nov 2023 12:24:40 +0000 Subject: [PATCH 08/57] Add documentation. --- .../test_ssh_file_transfer.py | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 68dace731..73a2dc272 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -15,7 +15,6 @@ TEST_SSH = ssh_test_utils.get_test_ssh() - PARAM_SUBS = [ ["all"], ["all_sub"], @@ -49,7 +48,10 @@ class TestFileTransfer: scope="class", ) def pathtable_and_project(self, tmpdir_factory): - """ """ + """ + Create a new test project with a test project folder + and file structure (see `get_pathtable()` for definition). + """ tmp_path = tmpdir_factory.mktemp("test") base_path = tmp_path / "test with space" @@ -86,7 +88,11 @@ def pathtable_and_project(self, tmpdir_factory): scope="class", ) def ssh_setup(self, pathtable_and_project): - """ """ + """ + After initial project setup (in `pathtable_and_project`) + setup a container and the project's SSH connection to the container. + Then upload the test project to the `central_path`. + """ pathtable, project = pathtable_and_project ssh_test_utils.setup_project_and_container_for_ssh(project) ssh_test_utils.setup_ssh_connection(project) @@ -95,13 +101,6 @@ def ssh_setup(self, pathtable_and_project): return [pathtable, project] - # ---------------------------------------------------------------------------------- - # Utils - # ---------------------------------------------------------------------------------- - - def central_from_local(self, path_): - return Path(str(copy.copy(path_)).replace("local", "central")) - # ---------------------------------------------------------------------------------- # Test File Transfer - All Options # ---------------------------------------------------------------------------------- @@ -118,7 +117,13 @@ def test_combinations_filesystem_transfer( datatype, upload_or_download, ): - """ """ + """ + Test many combinations of possible file transfer commands. The + entire test project is created in the original `local_path` + and subset of it is uploaded and tested against. To test + upload vs. download, the `local_path` and `central_path` + locations are swapped. + """ pathtable, project = pathtable_and_project # Transfer the data, swapping the paths to move a subset of @@ -185,9 +190,15 @@ def test_combinations_ssh_transfer( datatype, ): """ - This is very slow. maybe 8 s per test. Nearly all in the - upload() and download() part so unavoidable. Most code is shared, - this should be okay though my paranoid + Test a subset of argument combinations while testing over SSH connection + to a container. This is very slow, due to the rclone ssh transfer (which + is performed twice in this test, once for upload, once for download), around + 8 seconds per parameterization. + + In test setup, the entire project is created in the `local_path` and + is uploaded to `central_path`. So we only need to set up once per test, + upload and download is to temporary folders and these temporary folders + are cleaned at the end of each parameterization. """ pathtable, project = ssh_setup @@ -378,3 +389,6 @@ def create_all_pathtable_files(self, pathtable): filepath = pathtable["base_folder"][i] / pathtable["path"][i] filepath.parents[0].mkdir(parents=True, exist_ok=True) test_utils.write_file(filepath, contents="test_entry") + + def central_from_local(self, path_): + return Path(str(copy.copy(path_)).replace("local", "central")) From 790326c5e74044d12799e95d35708d77c523f8e9 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 23 Nov 2023 18:27:28 +0000 Subject: [PATCH 09/57] Try different Dockerfile for linux. --- tests/ssh_test_images/Dockerfile | 10 ++++++++-- tests/ssh_test_utils.py | 7 +++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/ssh_test_images/Dockerfile b/tests/ssh_test_images/Dockerfile index dea02ae45..6f68f030d 100644 --- a/tests/ssh_test_images/Dockerfile +++ b/tests/ssh_test_images/Dockerfile @@ -2,14 +2,20 @@ FROM ubuntu:latest # Install SSH server RUN apt-get update && \ - apt-get install -y openssh-server + apt-get upgrade -y +RUN apt-get install openssh-server -y supervisor RUN apt-get install nano + # Create an SSH user RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser # Set the SSH user's password (replace "password" with your desired password) -RUN echo 'sshuser:password' | chpasswd +RUN echo 'sshuser:password' + # Allow SSH access RUN mkdir /var/run/sshd + +RUN /usr/bin/ssh-keygen -A + # Expose the SSH port EXPOSE 22 # Start SSH server on container startup diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index b4fa2a423..5e78e485e 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -100,10 +100,9 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - subprocess.run(f"{container_software} build -t ssh_server .", shell=True) - subprocess.run( - f"{container_software} run -d -p 22:22 ssh_server", shell=True - ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + breakpoint() + subprocess.run(f"{container_software} build ssh_server .", shell=True) + subprocess.run(f"{container_software} run ssh_server", shell=True) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server setup_project_for_ssh( project, From f09815943161e977db45534cbff9651ab5eb2703 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 23 Nov 2023 18:43:34 +0000 Subject: [PATCH 10/57] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/ssh_test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 5e78e485e..810106421 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -102,7 +102,9 @@ def setup_project_and_container_for_ssh(project): os.chdir(image_path) breakpoint() subprocess.run(f"{container_software} build ssh_server .", shell=True) - subprocess.run(f"{container_software} run ssh_server", shell=True) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + subprocess.run( + f"{container_software} run ssh_server", shell=True + ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server setup_project_for_ssh( project, From f276ba97f42b52723065692e3949fbd949cc921b Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 10 Apr 2024 22:44:07 +0100 Subject: [PATCH 11/57] Fix issue after rebase. --- .../test_ssh_file_transfer.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 73a2dc272..4b2ed5242 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -97,7 +97,7 @@ def ssh_setup(self, pathtable_and_project): ssh_test_utils.setup_project_and_container_for_ssh(project) ssh_test_utils.setup_ssh_connection(project) - project.upload_all() + project.upload_rawdata() return [pathtable, project] @@ -136,7 +136,9 @@ def test_combinations_filesystem_transfer( swap_last_folder_only=False, )[0] - transfer_function("rawdata", sub_names, ses_names, datatype, init_log=False) + transfer_function( + "rawdata", sub_names, ses_names, datatype, init_log=False + ) if upload_or_download == "download": test_utils.swap_local_and_central_paths( @@ -210,7 +212,9 @@ def test_combinations_ssh_transfer( ) project.update_config("central_path", tmp_central_path) - project.upload(sub_names, ses_names, datatype, init_log=False) + project.upload_custom( + "rawdata", sub_names, ses_names, datatype, init_log=False + ) expected_transferred_paths = self.get_expected_transferred_paths( pathtable, sub_names, ses_names, datatype @@ -239,7 +243,9 @@ def test_combinations_ssh_transfer( project.update_config("local_path", tmp_local_path) project.update_config("central_path", true_central_path) - project.download(sub_names, ses_names, datatype, init_log=False) + project.download_custom( + "rawdata", sub_names, ses_names, datatype, init_log=False + ) # Find the transferred paths, tidy them up # and check expected paths were transferred. @@ -295,7 +301,7 @@ def get_expected_transferred_paths( expected_paths = pd.concat([datatype_folders, extra_folders]) expected_paths = expected_paths.drop_duplicates(subset="path") - expected_paths = self.remove_path_before_rawdata(expected_paths) + expected_paths = self.remove_path_before_rawdata(expected_paths.path) return expected_paths @@ -323,7 +329,7 @@ def make_pathtable_search_filter(self, sub_names, ses_names, datatype): ] else: for dtype in datatype: - if dtype == "all_ses_level_non_datatype": + if dtype == "all_non_datatype": extra_arguments += [ f"(parent_sub == '{sub}' & parent_ses == '{ses}' " f"& is_ses_level_non_datatype == True)" From 93615b8f3b0ac5c5522dc6013591a85e797031ea Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 10 Apr 2024 22:50:42 +0100 Subject: [PATCH 12/57] Remove breakpoint. --- tests/ssh_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 810106421..7861c805a 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -100,7 +100,7 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - breakpoint() + subprocess.run(f"{container_software} build ssh_server .", shell=True) subprocess.run( f"{container_software} run ssh_server", shell=True From e454325a5e49cd7276b45a351bdfaded04ae6ec8 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 10 Apr 2024 23:19:18 +0100 Subject: [PATCH 13/57] Don't use singularity, docker or nothing. --- tests/ssh_test_utils.py | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 7861c805a..5b24c867b 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,7 +1,6 @@ import builtins import copy import os -import platform import stat import subprocess import sys @@ -92,18 +91,17 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): def setup_project_and_container_for_ssh(project): """""" - container_software = is_docker_or_singularity_installed() - assert container_software is not False, ( - "docker or singularity not installed, " + assert is_docker_installed(), ( + "docker not installed, " "this should be checked at the top of test script" ) image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - subprocess.run(f"{container_software} build ssh_server .", shell=True) + subprocess.run("docker build ssh_server .", shell=True) subprocess.run( - f"{container_software} run ssh_server", shell=True + "docker run ssh_server", shell=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server setup_project_for_ssh( @@ -150,19 +148,13 @@ def recursive_search_central(project): def get_test_ssh(): """""" - if is_docker_or_singularity_installed(): - test_ssh = True - else: - warnings.warn( - "SSH tests are not run as docker (Windows, macOS) " - "or singularity (Linux) is not installed." - ) - test_ssh = False - - return test_ssh + docker_installed = is_docker_installed() + if not docker_installed: + warnings.warn("SSH tests are not run as docker is not installed.") + return docker_installed -def is_docker_or_singularity_installed(): +def is_docker_installed(): """""" check_install = ( lambda command: subprocess.run( @@ -173,13 +165,4 @@ def is_docker_or_singularity_installed(): ).returncode == 0 ) - - installed = False - if platform.system() == "Linux": - if check_install("singularity version"): - installed = "singularity" - else: - if check_install("docker -v"): - installed = "docker" - - return installed + return check_install("docker -v") From 8162051cc4891bbf091f69e932972e550eadfbbc Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 10 Apr 2024 23:38:15 +0100 Subject: [PATCH 14/57] Rework ssh setup. --- tests/ssh_test_utils.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 5b24c867b..2e13a049b 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -20,12 +20,11 @@ def setup_project_for_ssh( to central """ project.update_config_file( + connection_method="ssh", central_path=central_path, + central_host_id=central_host_id, + central_host_username=central_host_username, ) - project.update_config_file(central_host_id=central_host_id) - project.update_config_file(central_host_username=central_host_username) - project.update_config_file(connection_method="ssh") - rclone.setup_rclone_config_for_ssh( project.cfg, project.cfg.get_rclone_config_name("ssh"), From 1c04749c7c1ffc3fe415ad7a9573b93155f0df37 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 11 Apr 2024 01:27:04 +0100 Subject: [PATCH 15/57] Fix connection refused issue. --- tests/ssh_test_images/Dockerfile | 3 +++ tests/ssh_test_utils.py | 9 +++++--- .../test_ssh_file_transfer.py | 23 +++++++++++++------ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/ssh_test_images/Dockerfile b/tests/ssh_test_images/Dockerfile index 6f68f030d..228a62969 100644 --- a/tests/ssh_test_images/Dockerfile +++ b/tests/ssh_test_images/Dockerfile @@ -1,5 +1,6 @@ # Use a base image with the desired OS (e.g., Ubuntu, Debian, etc.) FROM ubuntu:latest + # Install SSH server RUN apt-get update && \ apt-get upgrade -y @@ -8,6 +9,7 @@ RUN apt-get install nano # Create an SSH user RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser + # Set the SSH user's password (replace "password" with your desired password) RUN echo 'sshuser:password' @@ -18,5 +20,6 @@ RUN /usr/bin/ssh-keygen -A # Expose the SSH port EXPOSE 22 + # Start SSH server on container startup CMD ["/usr/sbin/sshd", "-D"] diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 2e13a049b..6ad4b441a 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -99,9 +99,12 @@ def setup_project_and_container_for_ssh(project): os.chdir(image_path) subprocess.run("docker build ssh_server .", shell=True) - subprocess.run( - "docker run ssh_server", shell=True + subprocess.Popen( + "docker run -p 22:22 ssh_server", shell=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + import time + + time.sleep(10) setup_project_for_ssh( project, @@ -131,7 +134,7 @@ def sftp_recursive_file_search(sftp, path_, all_filenames): def recursive_search_central(project): """ """ with paramiko.SSHClient() as client: - ssh.connect_client(client, project.cfg) + ssh.connect_client_core(client, project.cfg) sftp = client.open_sftp() diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 4b2ed5242..3ec0011ba 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -182,7 +182,7 @@ def test_combinations_filesystem_transfer( "ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]] ) @pytest.mark.parametrize( - "datatype", [["all"], ["anat", "all_ses_level_non_datatype"]] + "datatype", [["all"], ["anat", "all_non_datatype"]] ) def test_combinations_ssh_transfer( self, @@ -210,7 +210,11 @@ def test_combinations_ssh_transfer( tmp_central_path = ( project.cfg["central_path"] / "tmp" / project.project_name ) - project.update_config("central_path", tmp_central_path) + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary + + project.update_config_file(central_path=tmp_central_path) project.upload_custom( "rawdata", sub_names, ses_names, datatype, init_log=False @@ -223,7 +227,6 @@ def test_combinations_ssh_transfer( # Search the paths that were transferred and tidy them up, # then check against the paths that were expected to be transferred. transferred_files = ssh_test_utils.recursive_search_central(project) - paths_to_transferred_files = self.remove_path_before_rawdata( transferred_files ) @@ -240,8 +243,8 @@ def test_combinations_ssh_transfer( ) tmp_local_path.mkdir(exist_ok=True, parents=True) - project.update_config("local_path", tmp_local_path) - project.update_config("central_path", true_central_path) + project.update_config_file(local_path=tmp_local_path) + project.update_config_file(central_path=true_central_path) project.download_custom( "rawdata", sub_names, ses_names, datatype, init_log=False @@ -265,12 +268,18 @@ def test_combinations_ssh_transfer( # Clean up, removing the temp directories and # resetting the project paths. with paramiko.SSHClient() as client: - ssh.connect_client(client, project.cfg) + ssh.connect_client_core(client, project.cfg) client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") shutil.rmtree(tmp_local_path) - project.update_config("local_path", true_local_path) + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary + project.update_config_file(local_path=true_local_path) + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary # ---------------------------------------------------------------------------------- # Utils From b296ca61509022642faf55a5535f8a58d7acc2b4 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 11 Apr 2024 02:06:41 +0100 Subject: [PATCH 16/57] Use run not POpen for actions. --- tests/ssh_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 6ad4b441a..57e5bb9b5 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -99,7 +99,7 @@ def setup_project_and_container_for_ssh(project): os.chdir(image_path) subprocess.run("docker build ssh_server .", shell=True) - subprocess.Popen( + subprocess.run( "docker run -p 22:22 ssh_server", shell=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server import time From 43976de212ba69b4eb008a5dac2c16f404a4a894 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 11 Apr 2024 02:26:17 +0100 Subject: [PATCH 17/57] try in detachted state. --- tests/ssh_test_utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 57e5bb9b5..3411bbcd8 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -100,11 +100,8 @@ def setup_project_and_container_for_ssh(project): subprocess.run("docker build ssh_server .", shell=True) subprocess.run( - "docker run -p 22:22 ssh_server", shell=True + "docker run -d -p 22:22 ssh_server", shell=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server - import time - - time.sleep(10) setup_project_for_ssh( project, From e6eaff5af5a3d0636f0be6ac23329cdf6bcf68b8 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 18 Apr 2024 20:16:55 +0100 Subject: [PATCH 18/57] Add error messages on docker setup. --- tests/ssh_test_utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 3411bbcd8..cf1a7412c 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -98,10 +98,13 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - subprocess.run("docker build ssh_server .", shell=True) - subprocess.run( - "docker run -d -p 22:22 ssh_server", shell=True + build_output = subprocess.run("docker build -t ssh_server .", shell=True, capture_output=True) + assert build_output.returncode == 0, f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + + run_output = subprocess.run( + "docker run -d -p 22:22 ssh_server", shell=True, capture_output=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + assert run_output.returncode == 0, f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" setup_project_for_ssh( project, From 2e7d3187042621dab13bfc552ce67b5be6482991 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 18 Apr 2024 20:17:57 +0100 Subject: [PATCH 19/57] run linting. --- tests/ssh_test_utils.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index cf1a7412c..745ce29e0 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -98,13 +98,19 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - build_output = subprocess.run("docker build -t ssh_server .", shell=True, capture_output=True) - assert build_output.returncode == 0, f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + build_output = subprocess.run( + "docker build -t ssh_server .", shell=True, capture_output=True + ) + assert ( + build_output.returncode == 0 + ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" run_output = subprocess.run( "docker run -d -p 22:22 ssh_server", shell=True, capture_output=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server - assert run_output.returncode == 0, f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" + assert ( + run_output.returncode == 0 + ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" setup_project_for_ssh( project, From 9dc273435833ca889def028ae46902878d18cfb3 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 18 Apr 2024 21:06:35 +0100 Subject: [PATCH 20/57] update connection failed error message. --- datashuttle/utils/ssh.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 8f6de6785..9c05c28ca 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -183,7 +183,7 @@ def connect_client_with_logging( f"Connection to { cfg['central_host_id']} made successfully." ) - except Exception: + except Exception as e: utils.log_and_raise_error( f"Could not connect to server. Ensure that \n" f"1) You have run setup_ssh_connection() \n" @@ -191,7 +191,8 @@ def connect_client_with_logging( f"3) The central_host_id: {cfg['central_host_id']} is" f" correct.\n" f"4) The central username:" - f" {cfg['central_host_username']}, and password are correct.", + f" {cfg['central_host_username']}, and password are correct." + f"Original error: {e}", ConnectionError, ) From 4306129c9b1b4da00ce06e917859d137470229fd Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 13:29:35 +0100 Subject: [PATCH 21/57] Test version working locally on linux but only can connect with sudo. --- tests/ssh_test_images/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssh_test_images/Dockerfile b/tests/ssh_test_images/Dockerfile index 228a62969..84d5035b7 100644 --- a/tests/ssh_test_images/Dockerfile +++ b/tests/ssh_test_images/Dockerfile @@ -11,7 +11,7 @@ RUN apt-get install nano RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser # Set the SSH user's password (replace "password" with your desired password) -RUN echo 'sshuser:password' +RUN echo "sshuser:password" | chpasswd # Allow SSH access RUN mkdir /var/run/sshd From b4bf5f6354c9912753b61cac7ecc1785ae1628df Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 13:59:23 +0100 Subject: [PATCH 22/57] Try freeing up and using the free port. --- .github/workflows/code_test_and_deploy.yml | 2 ++ tests/ssh_test_utils.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 52975585f..302e61382 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -57,6 +57,8 @@ jobs: run: | python -m pip install --upgrade pip pip install .[dev] + - name: Shutdown Ubuntu MySQL (SUDO) # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 + run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it - name: Test run: pytest diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 745ce29e0..32c58d844 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -105,8 +105,9 @@ def setup_project_and_container_for_ssh(project): build_output.returncode == 0 ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + # ports - https://github.com/orgs/community/discussions/25550 run_output = subprocess.run( - "docker run -d -p 22:22 ssh_server", shell=True, capture_output=True + "docker run -d -p 3306:22 ssh_server", shell=True, capture_output=True ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server assert ( run_output.returncode == 0 From 6ff5c7c3b05985ba9e4bebb6a19052836237615b Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 14:03:43 +0100 Subject: [PATCH 23/57] Test sudo service only on linux. --- .github/workflows/code_test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 302e61382..70d59d84c 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -32,7 +32,7 @@ jobs: # macos-14 is M1, macos-13 is intel. Run on earliest and # latest python versions. All python versions are tested in # the weekly cron job. - os: [windows-latest, ubuntu-latest, macos-14, macos-13] + os: [ ubuntu-latest] # [windows-latest, macos-14, macos-13] TODO: ADD BACK # Test all Python versions for cron job, and only first/last for other triggers python-version: ${{ fromJson(github.event_name == 'schedule' && '["3.9", "3.10", "3.11", "3.12"]' || '["3.9", "3.12"]') }} From 029da65f08b711475af98157335dfb934f2fc615 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 14:22:25 +0100 Subject: [PATCH 24/57] Add sudo to docker setup commands. --- tests/ssh_test_utils.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 32c58d844..97f6aa985 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -99,19 +99,21 @@ def setup_project_and_container_for_ssh(project): os.chdir(image_path) build_output = subprocess.run( - "docker build -t ssh_server .", shell=True, capture_output=True + "sudo docker build -t ssh_server .", shell=True, capture_output=True ) assert ( build_output.returncode == 0 - ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + ), f"sudo docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" # ports - https://github.com/orgs/community/discussions/25550 run_output = subprocess.run( - "docker run -d -p 3306:22 ssh_server", shell=True, capture_output=True + "sudo docker run -d -p 3306:22 ssh_server", + shell=True, + capture_output=True, ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server assert ( run_output.returncode == 0 - ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" + ), f"sudo docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" setup_project_for_ssh( project, From 941c1e0667c68124cf06f1fdf338b26de6f55bee Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 14:45:24 +0100 Subject: [PATCH 25/57] test with auto add policy. --- datashuttle/utils/ssh.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 9c05c28ca..1dec19900 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -30,7 +30,9 @@ def connect_client_core( password: Optional[str] = None, ): client.get_host_keys().load(cfg.hostkeys_path.as_posix()) - client.set_missing_host_key_policy(paramiko.RejectPolicy()) + client.set_missing_host_key_policy( + paramiko.AutoAddPolicy() + ) # AutoAddPolicy"!! REMOVE!!! TESTING!! client.connect( cfg["central_host_id"], From ab8e9bc935cc9d681166bdadccd7616df3f32375 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 15:15:10 +0100 Subject: [PATCH 26/57] Really restrict to the connect call. --- datashuttle/utils/ssh.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 1dec19900..8c48a6585 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -3,13 +3,14 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + from pathlib import Path + from datashuttle.configs.config_class import Configs import fnmatch import getpass import stat import sys -from pathlib import Path from typing import Any, List, Optional, Tuple import paramiko @@ -32,18 +33,12 @@ def connect_client_core( client.get_host_keys().load(cfg.hostkeys_path.as_posix()) client.set_missing_host_key_policy( paramiko.AutoAddPolicy() - ) # AutoAddPolicy"!! REMOVE!!! TESTING!! + ) # TODO: ADDBACK IN!! RejectPolicy client.connect( cfg["central_host_id"], username=cfg["central_host_username"], - password=password, - key_filename=( - cfg.ssh_key_path.as_posix() - if isinstance(cfg.ssh_key_path, Path) - else None - ), - look_for_keys=True, + password="password", # TODO: ADDBACK IN!! RejectPolicy ) From a0e083c69d5665c5769a6816ae9c50b67ea4da9d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 15:52:00 +0100 Subject: [PATCH 27/57] restrict to tests of interest temporarily. --- .github/workflows/code_test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 70d59d84c..14a895736 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -60,7 +60,7 @@ jobs: - name: Shutdown Ubuntu MySQL (SUDO) # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it - name: Test - run: pytest + run: pytest -k test_combinations_ssh_transfer build_sdist_wheels: name: Build source distribution From 6b51d28918629c984a20968da9a5e6d732d95c98 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 16:31:30 +0100 Subject: [PATCH 28/57] Fix to port 3306 in tests and for paramiko. --- datashuttle/utils/ssh.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 8c48a6585..3f496f4a1 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -3,14 +3,13 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from pathlib import Path - from datashuttle.configs.config_class import Configs import fnmatch import getpass import stat import sys +from pathlib import Path from typing import Any, List, Optional, Tuple import paramiko @@ -31,14 +30,19 @@ def connect_client_core( password: Optional[str] = None, ): client.get_host_keys().load(cfg.hostkeys_path.as_posix()) - client.set_missing_host_key_policy( - paramiko.AutoAddPolicy() - ) # TODO: ADDBACK IN!! RejectPolicy + client.set_missing_host_key_policy(paramiko.RejectPolicy()) client.connect( cfg["central_host_id"], username=cfg["central_host_username"], - password="password", # TODO: ADDBACK IN!! RejectPolicy + password=password, + key_filename=( + cfg.ssh_key_path.as_posix() + if isinstance(cfg.ssh_key_path, Path) + else None + ), + look_for_keys=True, + port=3306, ) @@ -80,7 +84,7 @@ def get_remote_server_key(central_host_id: str): connection. """ transport: paramiko.Transport - with paramiko.Transport(central_host_id) as transport: + with paramiko.Transport((central_host_id, 3306)) as transport: transport.connect() key = transport.get_remote_server_key() return key @@ -88,7 +92,9 @@ def get_remote_server_key(central_host_id: str): def save_hostkey_locally(key, central_host_id, hostkeys_path) -> None: client = paramiko.SSHClient() - client.get_host_keys().add(central_host_id, key.get_name(), key) + client.get_host_keys().add( + f"[{central_host_id}]:3306", key.get_name(), key + ) client.get_host_keys().save(hostkeys_path.as_posix()) From 84a776213c8040b4676e0864658a5fb405212d7d Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 16:41:28 +0100 Subject: [PATCH 29/57] Extend port to rclone. --- datashuttle/utils/rclone.py | 4 +++- datashuttle/utils/ssh.py | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 49d7da826..899f807ea 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -10,6 +10,8 @@ from datashuttle.utils import utils from datashuttle.utils.custom_types import TopLevelFolder +PORT = 3306 # TODO: direct copy! + def call_rclone(command: str, pipe_std: bool = False) -> CompletedProcess: """ @@ -141,7 +143,7 @@ def setup_rclone_config_for_ssh( f"sftp " f"host {cfg['central_host_id']} " f"user {cfg['central_host_username']} " - f"port 22 " + f"port {PORT} " f"key_file {ssh_key_path.as_posix()}", pipe_std=True, ) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 3f496f4a1..6e74379f7 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -14,6 +14,8 @@ import paramiko +PORT = 3306 + from datashuttle.utils import utils # ----------------------------------------------------------------------------- @@ -42,7 +44,7 @@ def connect_client_core( else None ), look_for_keys=True, - port=3306, + port=PORT, ) @@ -84,7 +86,7 @@ def get_remote_server_key(central_host_id: str): connection. """ transport: paramiko.Transport - with paramiko.Transport((central_host_id, 3306)) as transport: + with paramiko.Transport((central_host_id, PORT)) as transport: transport.connect() key = transport.get_remote_server_key() return key @@ -93,7 +95,7 @@ def get_remote_server_key(central_host_id: str): def save_hostkey_locally(key, central_host_id, hostkeys_path) -> None: client = paramiko.SSHClient() client.get_host_keys().add( - f"[{central_host_id}]:3306", key.get_name(), key + f"[{central_host_id}]:{PORT}", key.get_name(), key ) client.get_host_keys().save(hostkeys_path.as_posix()) From d5e2b733cbfed4d0ed8a084e1fd35512a78a004b Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 17:22:01 +0100 Subject: [PATCH 30/57] Use environment variable to set port. --- datashuttle/configs/canonical_configs.py | 11 +++++++++++ datashuttle/utils/rclone.py | 5 ++--- datashuttle/utils/ssh.py | 11 ++++++++--- tests/ssh_test_utils.py | 8 +++++--- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/datashuttle/configs/canonical_configs.py b/datashuttle/configs/canonical_configs.py index ac41d6e4a..9d0417432 100644 --- a/datashuttle/configs/canonical_configs.py +++ b/datashuttle/configs/canonical_configs.py @@ -11,6 +11,7 @@ from __future__ import annotations +import os from typing import ( TYPE_CHECKING, Dict, @@ -58,6 +59,16 @@ def keys_str_on_file_but_path_in_class() -> list[str]: ] +def get_default_ssh_port() -> int: + """ + Get the default port used for SSH connections. + """ + if "DS_SSH_PORT" in os.environ: + return int(os.environ["DS_SSH_PORT"]) + else: + return 22 + + # ----------------------------------------------------------------------------- # Check Configs # ----------------------------------------------------------------------------- diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 899f807ea..890d0b9f7 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -6,12 +6,11 @@ from subprocess import CompletedProcess from typing import Dict, List, Literal +from datashuttle.configs import canonical_configs from datashuttle.configs.config_class import Configs from datashuttle.utils import utils from datashuttle.utils.custom_types import TopLevelFolder -PORT = 3306 # TODO: direct copy! - def call_rclone(command: str, pipe_std: bool = False) -> CompletedProcess: """ @@ -143,7 +142,7 @@ def setup_rclone_config_for_ssh( f"sftp " f"host {cfg['central_host_id']} " f"user {cfg['central_host_username']} " - f"port {PORT} " + f"port {canonical_configs.get_default_ssh_port()} " f"key_file {ssh_key_path.as_posix()}", pipe_std=True, ) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 6e74379f7..5f5ea2b65 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -16,6 +16,7 @@ PORT = 3306 +from datashuttle.configs import canonical_configs from datashuttle.utils import utils # ----------------------------------------------------------------------------- @@ -44,7 +45,7 @@ def connect_client_core( else None ), look_for_keys=True, - port=PORT, + port=canonical_configs.get_default_ssh_port(), ) @@ -86,7 +87,9 @@ def get_remote_server_key(central_host_id: str): connection. """ transport: paramiko.Transport - with paramiko.Transport((central_host_id, PORT)) as transport: + with paramiko.Transport( + (central_host_id, canonical_configs.get_default_ssh_port()) + ) as transport: transport.connect() key = transport.get_remote_server_key() return key @@ -95,7 +98,9 @@ def get_remote_server_key(central_host_id: str): def save_hostkey_locally(key, central_host_id, hostkeys_path) -> None: client = paramiko.SSHClient() client.get_host_keys().add( - f"[{central_host_id}]:{PORT}", key.get_name(), key + f"[{central_host_id}]:{canonical_configs.get_default_ssh_port()}", + key.get_name(), + key, ) client.get_host_keys().save(hostkeys_path.as_posix()) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 97f6aa985..0702a1432 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -11,6 +11,9 @@ from datashuttle.utils import rclone, ssh +PORT = 3306 # https://github.com/orgs/community/discussions/25550 +os.environ["DS_SSH_PORT"] = str(PORT) + def setup_project_for_ssh( project, central_path, central_host_id, central_host_username @@ -65,7 +68,6 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): """ # Monkeypatch orig_builtin = setup_mock_input(input_="y") - orig_getpass = copy.deepcopy(ssh.getpass.getpass) ssh.getpass.getpass = lambda _: "password" # type: ignore @@ -105,12 +107,12 @@ def setup_project_and_container_for_ssh(project): build_output.returncode == 0 ), f"sudo docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" - # ports - https://github.com/orgs/community/discussions/25550 run_output = subprocess.run( - "sudo docker run -d -p 3306:22 ssh_server", + f"sudo docker run -d -p {PORT}:22 ssh_server", shell=True, capture_output=True, ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + assert ( run_output.returncode == 0 ), f"sudo docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" From 8d115165baadeab174942627d569d37602930055 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 17:24:40 +0100 Subject: [PATCH 31/57] Add all OS back. --- .github/workflows/code_test_and_deploy.yml | 5 ++++- tests/ssh_test_utils.py | 15 ++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 14a895736..45d4cb120 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -58,7 +58,10 @@ jobs: python -m pip install --upgrade pip pip install .[dev] - name: Shutdown Ubuntu MySQL (SUDO) # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 - run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it + run: | + if [ "$RUNNER_OS" == "Linux" ]; then + sudo service mysql stop + fi - name: Test run: pytest -k test_combinations_ssh_transfer diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 0702a1432..d579e16d0 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,6 +1,7 @@ import builtins import copy import os +import platform import stat import subprocess import sys @@ -100,22 +101,26 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) + add_sudo = "sudo" if platform.system() == "Linux" else "" + build_output = subprocess.run( - "sudo docker build -t ssh_server .", shell=True, capture_output=True + f"{add_sudo} docker build -t ssh_server .", + shell=True, + capture_output=True, ) assert ( build_output.returncode == 0 - ), f"sudo docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" run_output = subprocess.run( - f"sudo docker run -d -p {PORT}:22 ssh_server", + f"{add_sudo} docker run -d -p {PORT}:22 ssh_server", shell=True, capture_output=True, - ) # ; docker build -t ssh_server .", shell=True) # ;docker run -p 22:22 ssh_server + ) assert ( run_output.returncode == 0 - ), f"sudo docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" + ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" setup_project_for_ssh( project, From 8e29a65de3aeb7ce4f77b458a01121433e21f39f Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 19 Apr 2024 17:55:26 +0100 Subject: [PATCH 32/57] try remove tag for windows. --- tests/ssh_test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index d579e16d0..e67cbd969 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -101,10 +101,12 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) + # TODO: tidy add_sudo = "sudo" if platform.system() == "Linux" else "" + add_tag = "-t" if platform.system() == "Linux" else "" build_output = subprocess.run( - f"{add_sudo} docker build -t ssh_server .", + f"{add_sudo} docker build {add_tag} ssh_server .", shell=True, capture_output=True, ) From f465747d905a276c58aef3e71fda82cc71852d91 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 20 Apr 2024 17:15:27 +0100 Subject: [PATCH 33/57] Update docker commands for windows. --- tests/ssh_test_utils.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index e67cbd969..a3f09c8cd 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -101,12 +101,15 @@ def setup_project_and_container_for_ssh(project): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - # TODO: tidy - add_sudo = "sudo" if platform.system() == "Linux" else "" - add_tag = "-t" if platform.system() == "Linux" else "" + if platform.system() == "Linux": + build_command = "sudo docker build ssh_server -f ." + run_command = f"sudo docker run -d -p {PORT}:22 ssh_server" + else: + build_command = "docker build ." + run_command = f"docker run -d -p {PORT}:22 ssh_server" build_output = subprocess.run( - f"{add_sudo} docker build {add_tag} ssh_server .", + build_command, shell=True, capture_output=True, ) @@ -115,7 +118,7 @@ def setup_project_and_container_for_ssh(project): ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" run_output = subprocess.run( - f"{add_sudo} docker run -d -p {PORT}:22 ssh_server", + run_command, shell=True, capture_output=True, ) From 324a796661afcdbc0903452c9e2d9e559ceaa429 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 20 Apr 2024 17:24:00 +0100 Subject: [PATCH 34/57] Fix nonsense docker build command. --- tests/ssh_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index a3f09c8cd..81cb53ee4 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -102,7 +102,7 @@ def setup_project_and_container_for_ssh(project): os.chdir(image_path) if platform.system() == "Linux": - build_command = "sudo docker build ssh_server -f ." + build_command = "sudo docker build -t ssh_server ." run_command = f"sudo docker run -d -p {PORT}:22 ssh_server" else: build_command = "docker build ." From 597a0b2343410ea396e0fa800f41926861e0ba18 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 11:52:32 +0100 Subject: [PATCH 35/57] Only run when docker running and on ubuntu on runners. --- .github/workflows/code_test_and_deploy.yml | 13 +++++---- tests/ssh_test_utils.py | 32 +++++++++++++++++----- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 45d4cb120..c8ee23e2b 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -57,13 +57,14 @@ jobs: run: | python -m pip install --upgrade pip pip install .[dev] - - name: Shutdown Ubuntu MySQL (SUDO) # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 - run: | - if [ "$RUNNER_OS" == "Linux" ]; then - sudo service mysql stop - fi - name: Test - run: pytest -k test_combinations_ssh_transfer + run: | + if [ "$RUNNER_OS" == "Linux" ]; then + sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 + pytest + else + pytest -k "not test_combinations_ssh_transfer and not test_ssh_setup" + fi build_sdist_wheels: name: Build source distribution diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 81cb53ee4..5db0bbdd7 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,3 +1,7 @@ +""" + +""" + import builtins import copy import os @@ -93,8 +97,8 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): def setup_project_and_container_for_ssh(project): """""" - assert is_docker_installed(), ( - "docker not installed, " + assert docker_is_running(), ( + "docker is not running, " "this should be checked at the top of test script" ) @@ -171,16 +175,31 @@ def recursive_search_central(project): def get_test_ssh(): """""" - docker_installed = is_docker_installed() + docker_installed = docker_is_running() if not docker_installed: - warnings.warn("SSH tests are not run as docker is not installed.") + warnings.warn( + "SSH tests are not run as docker either not installed or running." + ) return docker_installed +def docker_is_running(): + """""" + if not is_docker_installed(): + return False + + is_running = check_sys_command_returns_0("docker stats --no-stream") + return is_running + + def is_docker_installed(): """""" - check_install = ( - lambda command: subprocess.run( + return check_sys_command_returns_0("docker -v") + + +def check_sys_command_returns_0(command): + return ( + subprocess.run( command, shell=True, stdout=subprocess.DEVNULL, @@ -188,4 +207,3 @@ def is_docker_installed(): ).returncode == 0 ) - return check_install("docker -v") From b529809e8dc26f595421e566c96c04be56ff72de Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 12:51:01 +0100 Subject: [PATCH 36/57] Try build and run docker only once per session. --- tests/ssh_test_utils.py | 50 -------- tests/tests_integration/base.py | 110 ++++++++++++++++++ .../test_ssh_file_transfer.py | 3 +- tests/tests_integration/test_ssh_setup.py | 14 ++- 4 files changed, 122 insertions(+), 55 deletions(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 5db0bbdd7..7391d853e 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -4,21 +4,15 @@ import builtins import copy -import os -import platform import stat import subprocess import sys import warnings -from pathlib import Path import paramiko from datashuttle.utils import rclone, ssh -PORT = 3306 # https://github.com/orgs/community/discussions/25550 -os.environ["DS_SSH_PORT"] = str(PORT) - def setup_project_for_ssh( project, central_path, central_host_id, central_host_username @@ -95,50 +89,6 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): return verified -def setup_project_and_container_for_ssh(project): - """""" - assert docker_is_running(), ( - "docker is not running, " - "this should be checked at the top of test script" - ) - - image_path = Path(__file__).parent / "ssh_test_images" - os.chdir(image_path) - - if platform.system() == "Linux": - build_command = "sudo docker build -t ssh_server ." - run_command = f"sudo docker run -d -p {PORT}:22 ssh_server" - else: - build_command = "docker build ." - run_command = f"docker run -d -p {PORT}:22 ssh_server" - - build_output = subprocess.run( - build_command, - shell=True, - capture_output=True, - ) - assert ( - build_output.returncode == 0 - ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" - - run_output = subprocess.run( - run_command, - shell=True, - capture_output=True, - ) - - assert ( - run_output.returncode == 0 - ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" - - setup_project_for_ssh( - project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", - central_host_id="localhost", - central_host_username="sshuser", - ) - - def sftp_recursive_file_search(sftp, path_, all_filenames): try: sftp.stat(path_) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index bee36b019..0557871f7 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -1,7 +1,16 @@ +<<<<<<< HEAD +======= +import os +import platform +import subprocess +>>>>>>> 75d3f43 (Try build and run docker only once per session.) import warnings +from pathlib import Path import pytest +import ssh_test_utils import test_utils +from file_conflicts_pathtable import get_pathtable from datashuttle import DataShuttle @@ -71,3 +80,104 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) + + @pytest.fixture( + scope="class", + ) + def pathtable_and_project(self, tmpdir_factory): + """ + Create a new test project with a test project folder + and file structure (see `get_pathtable()` for definition). + """ + tmp_path = tmpdir_factory.mktemp("test") + + base_path = tmp_path / "test with space" + test_project_name = "test_file_conflicts" + + project, cwd = test_utils.setup_project_fixture( + base_path, test_project_name + ) + + pathtable = get_pathtable(project.cfg["local_path"]) + + self.create_all_pathtable_files(pathtable) + + yield [pathtable, project] + + test_utils.teardown_project(cwd, project) + + @pytest.fixture( + scope="session", + ) + def setup_ssh_container(self): + """""" + PORT = 3306 # https://github.com/orgs/community/discussions/25550 + os.environ["DS_SSH_PORT"] = str(PORT) + + assert ssh_test_utils.docker_is_running(), ( + "docker is not running, " + "this should be checked at the top of test script" + ) + + image_path = Path(__file__).parent.parent / "ssh_test_images" + os.chdir(image_path) + + if platform.system() == "Linux": + build_command = "sudo docker build -t ssh_server ." + run_command = f"sudo docker run -d -p {PORT}:22 ssh_server" + else: + build_command = "docker build ." + run_command = f"docker run -d -p {PORT}:22 ssh_server" + + build_output = subprocess.run( + build_command, + shell=True, + capture_output=True, + ) + assert build_output.returncode == 0, ( + f"docker build failed with: STDOUT-{build_output.stdout} STDERR-" + f"{build_output.stderr}" + ) + + run_output = subprocess.run( + run_command, + shell=True, + capture_output=True, + ) + + assert run_output.returncode == 0, ( + f"docker run failed with: STDOUT-{run_output.stdout} STDE" + f"RR-{run_output.stderr}" + ) + + # setup_project_for_ssh( + # project, + # central_path=f"/home/sshuser/datashuttle/{project.project_name}", + # central_host_id="localhost", + # central_host_username="sshuser", + # ) + + @pytest.fixture( + scope="class", + ) + def ssh_setup(self, pathtable_and_project, setup_ssh_container): + """ + After initial project setup (in `pathtable_and_project`) + setup a container and the project's SSH connection to the container. + Then upload the test project to the `central_path`. + """ + pathtable, project = pathtable_and_project + + ssh_test_utils.setup_project_for_ssh( + project, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", + ) + + # ssh_test_utils.setup_project_and_container_for_ssh(project) + ssh_test_utils.setup_ssh_connection(project) + + project.upload_rawdata() + + return [pathtable, project] diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 3ec0011ba..5858d2588 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -9,7 +9,7 @@ import pytest import ssh_test_utils import test_utils -from file_conflicts_pathtable import get_pathtable +from base import BaseTest from datashuttle.utils import ssh @@ -42,7 +42,6 @@ ["anat", "behav", "all_non_datatype"], ] - class TestFileTransfer: @pytest.fixture( scope="class", diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 3fcc4e36a..1a84e1ce5 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -1,6 +1,7 @@ import pytest import ssh_test_utils import test_utils +from base import BaseTest # from pytest import ssh_config from datashuttle.utils import ssh @@ -9,9 +10,9 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") -class TestSSH: +class TestSSH(BaseTest): @pytest.fixture(scope="function") - def project(test, tmp_path): + def project(test, tmp_path, setup_ssh_container): """ Make a project as per usual, but now add in test ssh configurations @@ -21,7 +22,14 @@ def project(test, tmp_path): test_project_name = "test_ssh" project = test_utils.setup_project_fixture(tmp_path, test_project_name) - ssh_test_utils.setup_project_and_container_for_ssh(project) + # ssh_test_utils.setup_project_and_container_for_ssh(project) + + ssh_test_utils.setup_project_for_ssh( + project, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", + ) yield project test_utils.teardown_project(project) From 14579bae65eb5815dcf0359deb59b25df9fe0a10 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 14:46:05 +0100 Subject: [PATCH 37/57] Teardown image at end of ssh tests, factor out ssh tests. --- tests/ssh_test_utils.py | 45 ++++ tests/tests_integration/base.py | 108 +------- tests/tests_integration/base_transfer.py | 166 ++++++++++++ tests/tests_integration/test_ssh.py | 246 ++++++++++++++++++ .../test_ssh_file_transfer.py | 241 +---------------- 5 files changed, 459 insertions(+), 347 deletions(-) create mode 100644 tests/tests_integration/base_transfer.py create mode 100644 tests/tests_integration/test_ssh.py diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 7391d853e..e55555597 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -4,15 +4,21 @@ import builtins import copy +import os +import platform import stat import subprocess import sys import warnings +from pathlib import Path import paramiko from datashuttle.utils import rclone, ssh +PORT = 3306 # https://github.com/orgs/community/discussions/25550 +os.environ["DS_SSH_PORT"] = str(PORT) + def setup_project_for_ssh( project, central_path, central_host_id, central_host_username @@ -89,6 +95,45 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): return verified +def setup_ssh_container(container_name): + """""" + assert docker_is_running(), ( + "docker is not running, " + "this should be checked at the top of test script" + ) + + image_path = Path(__file__).parent / "ssh_test_images" + os.chdir(image_path) + + if platform.system() == "Linux": + build_command = "sudo docker build -t ssh_server ." + run_command = f"sudo docker run -d -p {PORT}:22 --name {container_name} ssh_server" + else: + build_command = "docker build ." + run_command = ( + f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" + ) + + build_output = subprocess.run( + build_command, + shell=True, + capture_output=True, + ) + assert ( + build_output.returncode == 0 + ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + + run_output = subprocess.run( + run_command, + shell=True, + capture_output=True, + ) + + assert ( + run_output.returncode == 0 + ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" + + def sftp_recursive_file_search(sftp, path_, all_filenames): try: sftp.stat(path_) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 0557871f7..98f1f5b80 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -1,16 +1,11 @@ -<<<<<<< HEAD -======= import os import platform import subprocess ->>>>>>> 75d3f43 (Try build and run docker only once per session.) + import warnings -from pathlib import Path import pytest -import ssh_test_utils import test_utils -from file_conflicts_pathtable import get_pathtable from datashuttle import DataShuttle @@ -80,104 +75,3 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) - - @pytest.fixture( - scope="class", - ) - def pathtable_and_project(self, tmpdir_factory): - """ - Create a new test project with a test project folder - and file structure (see `get_pathtable()` for definition). - """ - tmp_path = tmpdir_factory.mktemp("test") - - base_path = tmp_path / "test with space" - test_project_name = "test_file_conflicts" - - project, cwd = test_utils.setup_project_fixture( - base_path, test_project_name - ) - - pathtable = get_pathtable(project.cfg["local_path"]) - - self.create_all_pathtable_files(pathtable) - - yield [pathtable, project] - - test_utils.teardown_project(cwd, project) - - @pytest.fixture( - scope="session", - ) - def setup_ssh_container(self): - """""" - PORT = 3306 # https://github.com/orgs/community/discussions/25550 - os.environ["DS_SSH_PORT"] = str(PORT) - - assert ssh_test_utils.docker_is_running(), ( - "docker is not running, " - "this should be checked at the top of test script" - ) - - image_path = Path(__file__).parent.parent / "ssh_test_images" - os.chdir(image_path) - - if platform.system() == "Linux": - build_command = "sudo docker build -t ssh_server ." - run_command = f"sudo docker run -d -p {PORT}:22 ssh_server" - else: - build_command = "docker build ." - run_command = f"docker run -d -p {PORT}:22 ssh_server" - - build_output = subprocess.run( - build_command, - shell=True, - capture_output=True, - ) - assert build_output.returncode == 0, ( - f"docker build failed with: STDOUT-{build_output.stdout} STDERR-" - f"{build_output.stderr}" - ) - - run_output = subprocess.run( - run_command, - shell=True, - capture_output=True, - ) - - assert run_output.returncode == 0, ( - f"docker run failed with: STDOUT-{run_output.stdout} STDE" - f"RR-{run_output.stderr}" - ) - - # setup_project_for_ssh( - # project, - # central_path=f"/home/sshuser/datashuttle/{project.project_name}", - # central_host_id="localhost", - # central_host_username="sshuser", - # ) - - @pytest.fixture( - scope="class", - ) - def ssh_setup(self, pathtable_and_project, setup_ssh_container): - """ - After initial project setup (in `pathtable_and_project`) - setup a container and the project's SSH connection to the container. - Then upload the test project to the `central_path`. - """ - pathtable, project = pathtable_and_project - - ssh_test_utils.setup_project_for_ssh( - project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", - central_host_id="localhost", - central_host_username="sshuser", - ) - - # ssh_test_utils.setup_project_and_container_for_ssh(project) - ssh_test_utils.setup_ssh_connection(project) - - project.upload_rawdata() - - return [pathtable, project] diff --git a/tests/tests_integration/base_transfer.py b/tests/tests_integration/base_transfer.py new file mode 100644 index 000000000..3af49def6 --- /dev/null +++ b/tests/tests_integration/base_transfer.py @@ -0,0 +1,166 @@ +""" +""" + +import copy +from pathlib import Path + +import pandas as pd +import pytest +import test_utils +from base import BaseTest +from file_conflicts_pathtable import get_pathtable + + +class BaseTransfer(BaseTest): + + # ---------------------------------------------------------------------------------- + # Test File Transfer - All Options + # ---------------------------------------------------------------------------------- + + @pytest.fixture( + scope="class", + ) + def pathtable_and_project(self, tmpdir_factory): + """ + Create a new test project with a test project folder + and file structure (see `get_pathtable()` for definition). + """ + tmp_path = tmpdir_factory.mktemp("test") + + base_path = tmp_path / "test with space" + test_project_name = "test_file_conflicts" + + project, cwd = test_utils.setup_project_fixture( + base_path, test_project_name + ) + + pathtable = get_pathtable(project.cfg["local_path"]) + + self.create_all_pathtable_files(pathtable) + + yield [pathtable, project] + + test_utils.teardown_project(cwd, project) + + def get_expected_transferred_paths( + self, pathtable, sub_names, ses_names, datatype + ): + """ + Process the expected files that are transferred using the logic in + `make_pathtable_search_filter()` to + """ + parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") + parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") + parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") + + # Filter pathtable to get files that were expected to be transferred + ( + sub_ses_dtype_arguments, + extra_arguments, + ) = self.make_pathtable_search_filter( + parsed_sub_names, parsed_ses_names, parsed_datatype + ) + + datatype_folders = self.query_table(pathtable, sub_ses_dtype_arguments) + extra_folders = self.query_table(pathtable, extra_arguments) + + expected_paths = pd.concat([datatype_folders, extra_folders]) + expected_paths = expected_paths.drop_duplicates(subset="path") + + expected_paths = self.remove_path_before_rawdata(expected_paths.path) + + return expected_paths + + def make_pathtable_search_filter(self, sub_names, ses_names, datatype): + """ + Create a string of arguments to pass to pd.query() that will + create the table of only transferred sub, ses and datatype. + + Two arguments must be created, one of all sub / ses / datatypes + and the other of all non sub/ non ses / non datatype + folders. These must be handled separately as they are + mutually exclusive. + """ + sub_ses_dtype_arguments = [] + extra_arguments = [] + + for sub in sub_names: + if sub == "all_non_sub": + extra_arguments += ["is_non_sub == True"] + else: + for ses in ses_names: + if ses == "all_non_ses": + extra_arguments += [ + f"(parent_sub == '{sub}' & is_non_ses == True)" + ] + else: + for dtype in datatype: + if dtype == "all_non_datatype": + extra_arguments += [ + f"(parent_sub == '{sub}' & parent_ses == '{ses}' " + f"& is_ses_level_non_datatype == True)" + ] + else: + sub_ses_dtype_arguments += [ + f"(parent_sub == '{sub}' & parent_ses == '{ses}' " + f"& (parent_datatype == '{dtype}' " + f"| parent_datatype == '{dtype}'))" + ] + + return sub_ses_dtype_arguments, extra_arguments + + def remove_path_before_rawdata(self, list_of_paths): + """ + Remove the path to project files before the "rawdata" so + they can be compared no matter where the project was stored + (e.g. on a central server vs. local filesystem). + """ + cut_paths = [] + for path_ in list_of_paths: + parts = Path(path_).parts + cut_paths.append(Path(*parts[parts.index("rawdata") :])) + return cut_paths + + def query_table(self, pathtable, arguments): + """ + Search the table for arguments, return empty + if arguments empty + """ + if any(arguments): + folders = pathtable.query(" | ".join(arguments)) + else: + folders = pd.DataFrame() + return folders + + def parse_arguments(self, pathtable, list_of_names, field): + """ + Replicate datashuttle name formatting by parsing + "all" arguments and turning them into a list of all names, + (subject or session), taken from the pathtable. + """ + if list_of_names in [["all"], [f"all_{field}"]]: + entries = pathtable.query(f"parent_{field} != False")[ + f"parent_{field}" + ] + entries = list(set(entries)) + if list_of_names == ["all"]: + entries += ( + [f"all_non_{field}"] + if field != "datatype" + else ["all_non_datatype"] + ) + list_of_names = entries + return list_of_names + + def create_all_pathtable_files(self, pathtable): + """ + Create the entire test project in the defined + location (usually project's `local_path`). + """ + for i in range(pathtable.shape[0]): + filepath = pathtable["base_folder"][i] / pathtable["path"][i] + filepath.parents[0].mkdir(parents=True, exist_ok=True) + test_utils.write_file(filepath, contents="test_entry") + + def central_from_local(self, path_): + return Path(str(copy.copy(path_)).replace("local", "central")) diff --git a/tests/tests_integration/test_ssh.py b/tests/tests_integration/test_ssh.py new file mode 100644 index 000000000..898890308 --- /dev/null +++ b/tests/tests_integration/test_ssh.py @@ -0,0 +1,246 @@ +import shutil +import subprocess + +import paramiko +import pytest +import ssh_test_utils +import test_utils +from base_transfer import BaseTransfer + +# from pytest import ssh_config +from datashuttle.utils import ssh + +TEST_SSH = ssh_test_utils.get_test_ssh() + + +@pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") +class TestSSH(BaseTransfer): + + @pytest.fixture( + scope="session", + ) + def setup_ssh_container(self): + # Annoying session scope does not seem to actually work + container_name = "running_ssh_tests" + ssh_test_utils.setup_ssh_container(container_name) + yield + subprocess.run(f"docker stop {container_name}", shell=True) + subprocess.run(f"docker rm {container_name}", shell=True) + + @pytest.fixture( + scope="class", + ) + def ssh_setup(self, pathtable_and_project, setup_ssh_container): + """ + After initial project setup (in `pathtable_and_project`) + setup a container and the project's SSH connection to the container. + Then upload the test project to the `central_path`. + """ + pathtable, project = pathtable_and_project + + ssh_test_utils.setup_project_for_ssh( + project, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", + ) + + ssh_test_utils.setup_ssh_connection(project) + + project.upload_rawdata() + + return [pathtable, project] + + @pytest.fixture(scope="function") + def project(test, tmp_path, setup_ssh_container): + """ + Make a project as per usual, but now add + in test ssh configurations + """ + tmp_path = tmp_path / "test with space" + + test_project_name = "test_ssh" + project, cwd = test_utils.setup_project_fixture( + tmp_path, test_project_name + ) + ssh_test_utils.setup_project_for_ssh( + project, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", + ) + yield project + test_utils.teardown_project(cwd, project) + + # ----------------------------------------------------------------- + # Test Setup SSH Connection + # ----------------------------------------------------------------- + + @pytest.mark.parametrize("input_", ["n", "o", "@"]) + def test_verify_ssh_central_host_do_not_accept( + self, capsys, project, input_ + ): + """ + Use the main function to test this. Test the sub-function + when accepting, because this main function will also + call setup ssh key pairs which we don't want to do yet + + This should only accept for "y" so try some random strings + including "n" and check they all do not make the connection. + """ + orig_builtin = ssh_test_utils.setup_mock_input(input_) + + project.setup_ssh_connection() + + ssh_test_utils.restore_mock_input(orig_builtin) + + captured = capsys.readouterr() + + assert "Host not accepted. No connection made.\n" in captured.out + + def test_verify_ssh_central_host_accept(self, capsys, project): + """ + User is asked to accept the server hostkey. Mock this here + and check hostkey is successfully accepted and written to configs. + """ + test_utils.clear_capsys(capsys) + + verified = ssh_test_utils.setup_ssh_connection( + project, setup_ssh_key_pair=False + ) + + assert verified + captured = capsys.readouterr() + + assert captured.out == "Host accepted.\n" + + with open(project.cfg.hostkeys_path, "r") as file: + hostkey = file.readlines()[0] + + assert ( + f"[{project.cfg['central_host_id']}]:3306 ssh-ed25519 " in hostkey + ) + + def test_generate_and_write_ssh_key(self, project): + """ + Check ssh key for passwordless connection is written + to file + """ + path_to_save = project.cfg["local_path"] / "test" + ssh.generate_and_write_ssh_key(path_to_save) + + with open(path_to_save, "r") as file: + first_line = file.readlines()[0] + + assert first_line == "-----BEGIN RSA PRIVATE KEY-----\n" + + # ----------------------------------------------------------------- + # Test Setup SSH Connection + # ----------------------------------------------------------------- + + @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") + @pytest.mark.parametrize( + "sub_names", [["all"], ["all_non_sub", "sub-002"]] + ) + @pytest.mark.parametrize( + "ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]] + ) + @pytest.mark.parametrize( + "datatype", [["all"], ["anat", "all_non_datatype"]] + ) + def test_combinations_ssh_transfer( + self, + ssh_setup, + sub_names, + ses_names, + datatype, + ): + """ + Test a subset of argument combinations while testing over SSH connection + to a container. This is very slow, due to the rclone ssh transfer (which + is performed twice in this test, once for upload, once for download), around + 8 seconds per parameterization. + + In test setup, the entire project is created in the `local_path` and + is uploaded to `central_path`. So we only need to set up once per test, + upload and download is to temporary folders and these temporary folders + are cleaned at the end of each parameterization. + """ + pathtable, project = ssh_setup + + # Upload data from the setup local project to a temporary + # central directory. + true_central_path = project.cfg["central_path"] + tmp_central_path = ( + project.cfg["central_path"] / "tmp" / project.project_name + ) + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary + + project.update_config_file(central_path=tmp_central_path) + + project.upload_custom( + "rawdata", sub_names, ses_names, datatype, init_log=False + ) + + expected_transferred_paths = self.get_expected_transferred_paths( + pathtable, sub_names, ses_names, datatype + ) + + # Search the paths that were transferred and tidy them up, + # then check against the paths that were expected to be transferred. + transferred_files = ssh_test_utils.recursive_search_central(project) + paths_to_transferred_files = self.remove_path_before_rawdata( + transferred_files + ) + + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths + ) + + # Now, move data from the central path where the project is + # setup, to a temp local folder to test download. + true_local_path = project.cfg["local_path"] + tmp_local_path = ( + project.cfg["local_path"] / "tmp" / project.project_name + ) + tmp_local_path.mkdir(exist_ok=True, parents=True) + + project.update_config_file(local_path=tmp_local_path) + project.update_config_file(central_path=true_central_path) + + project.download_custom( + "rawdata", sub_names, ses_names, datatype, init_log=False + ) + + # Find the transferred paths, tidy them up + # and check expected paths were transferred. + all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) + all_transferred = [ + path_ for path_ in all_transferred if path_.is_file() + ] + + paths_to_transferred_files = self.remove_path_before_rawdata( + all_transferred + ) + + assert sorted(paths_to_transferred_files) == sorted( + expected_transferred_paths + ) + + # Clean up, removing the temp directories and + # resetting the project paths. + with paramiko.SSHClient() as client: + ssh.connect_client_core(client, project.cfg) + client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") + + shutil.rmtree(tmp_local_path) + + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary + project.update_config_file(local_path=true_local_path) + project.get_logging_path().mkdir( + parents=True, exist_ok=True + ) # TODO: why is this necessary diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_ssh_file_transfer.py index 5858d2588..1d589787e 100644 --- a/tests/tests_integration/test_ssh_file_transfer.py +++ b/tests/tests_integration/test_ssh_file_transfer.py @@ -1,17 +1,12 @@ """ """ -import copy import shutil from pathlib import Path -import pandas as pd -import paramiko import pytest import ssh_test_utils import test_utils -from base import BaseTest - -from datashuttle.utils import ssh +from base_transfer import BaseTransfer TEST_SSH = ssh_test_utils.get_test_ssh() @@ -172,237 +167,3 @@ def test_combinations_filesystem_transfer( shutil.rmtree(self.central_from_local(project.cfg["local_path"])) except FileNotFoundError: pass - - @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") - @pytest.mark.parametrize( - "sub_names", [["all"], ["all_non_sub", "sub-002"]] - ) - @pytest.mark.parametrize( - "ses_names", [["all"], ["ses-002_random-key"], ["all_non_ses"]] - ) - @pytest.mark.parametrize( - "datatype", [["all"], ["anat", "all_non_datatype"]] - ) - def test_combinations_ssh_transfer( - self, - ssh_setup, - sub_names, - ses_names, - datatype, - ): - """ - Test a subset of argument combinations while testing over SSH connection - to a container. This is very slow, due to the rclone ssh transfer (which - is performed twice in this test, once for upload, once for download), around - 8 seconds per parameterization. - - In test setup, the entire project is created in the `local_path` and - is uploaded to `central_path`. So we only need to set up once per test, - upload and download is to temporary folders and these temporary folders - are cleaned at the end of each parameterization. - """ - pathtable, project = ssh_setup - - # Upload data from the setup local project to a temporary - # central directory. - true_central_path = project.cfg["central_path"] - tmp_central_path = ( - project.cfg["central_path"] / "tmp" / project.project_name - ) - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary - - project.update_config_file(central_path=tmp_central_path) - - project.upload_custom( - "rawdata", sub_names, ses_names, datatype, init_log=False - ) - - expected_transferred_paths = self.get_expected_transferred_paths( - pathtable, sub_names, ses_names, datatype - ) - - # Search the paths that were transferred and tidy them up, - # then check against the paths that were expected to be transferred. - transferred_files = ssh_test_utils.recursive_search_central(project) - paths_to_transferred_files = self.remove_path_before_rawdata( - transferred_files - ) - - assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths - ) - - # Now, move data from the central path where the project is - # setup, to a temp local folder to test download. - true_local_path = project.cfg["local_path"] - tmp_local_path = ( - project.cfg["local_path"] / "tmp" / project.project_name - ) - tmp_local_path.mkdir(exist_ok=True, parents=True) - - project.update_config_file(local_path=tmp_local_path) - project.update_config_file(central_path=true_central_path) - - project.download_custom( - "rawdata", sub_names, ses_names, datatype, init_log=False - ) - - # Find the transferred paths, tidy them up - # and check expected paths were transferred. - all_transferred = list((tmp_local_path / "rawdata").glob("**/*")) - all_transferred = [ - path_ for path_ in all_transferred if path_.is_file() - ] - - paths_to_transferred_files = self.remove_path_before_rawdata( - all_transferred - ) - - assert sorted(paths_to_transferred_files) == sorted( - expected_transferred_paths - ) - - # Clean up, removing the temp directories and - # resetting the project paths. - with paramiko.SSHClient() as client: - ssh.connect_client_core(client, project.cfg) - client.exec_command(f"rm -rf {(tmp_central_path).as_posix()}") - - shutil.rmtree(tmp_local_path) - - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary - project.update_config_file(local_path=true_local_path) - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary - - # ---------------------------------------------------------------------------------- - # Utils - # ---------------------------------------------------------------------------------- - - def get_expected_transferred_paths( - self, pathtable, sub_names, ses_names, datatype - ): - """ - Process the expected files that are transferred using the logic in - `make_pathtable_search_filter()` to - """ - parsed_sub_names = self.parse_arguments(pathtable, sub_names, "sub") - parsed_ses_names = self.parse_arguments(pathtable, ses_names, "ses") - parsed_datatype = self.parse_arguments(pathtable, datatype, "datatype") - - # Filter pathtable to get files that were expected to be transferred - ( - sub_ses_dtype_arguments, - extra_arguments, - ) = self.make_pathtable_search_filter( - parsed_sub_names, parsed_ses_names, parsed_datatype - ) - - datatype_folders = self.query_table(pathtable, sub_ses_dtype_arguments) - extra_folders = self.query_table(pathtable, extra_arguments) - - expected_paths = pd.concat([datatype_folders, extra_folders]) - expected_paths = expected_paths.drop_duplicates(subset="path") - - expected_paths = self.remove_path_before_rawdata(expected_paths.path) - - return expected_paths - - def make_pathtable_search_filter(self, sub_names, ses_names, datatype): - """ - Create a string of arguments to pass to pd.query() that will - create the table of only transferred sub, ses and datatype. - - Two arguments must be created, one of all sub / ses / datatypes - and the other of all non sub/ non ses / non datatype - folders. These must be handled separately as they are - mutually exclusive. - """ - sub_ses_dtype_arguments = [] - extra_arguments = [] - - for sub in sub_names: - if sub == "all_non_sub": - extra_arguments += ["is_non_sub == True"] - else: - for ses in ses_names: - if ses == "all_non_ses": - extra_arguments += [ - f"(parent_sub == '{sub}' & is_non_ses == True)" - ] - else: - for dtype in datatype: - if dtype == "all_non_datatype": - extra_arguments += [ - f"(parent_sub == '{sub}' & parent_ses == '{ses}' " - f"& is_ses_level_non_datatype == True)" - ] - else: - sub_ses_dtype_arguments += [ - f"(parent_sub == '{sub}' & parent_ses == '{ses}' " - f"& (parent_datatype == '{dtype}' " - f"| parent_datatype == '{dtype}'))" - ] - - return sub_ses_dtype_arguments, extra_arguments - - def remove_path_before_rawdata(self, list_of_paths): - """ - Remove the path to project files before the "rawdata" so - they can be compared no matter where the project was stored - (e.g. on a central server vs. local filesystem). - """ - cut_paths = [] - for path_ in list_of_paths: - parts = Path(path_).parts - cut_paths.append(Path(*parts[parts.index("rawdata") :])) - return cut_paths - - def query_table(self, pathtable, arguments): - """ - Search the table for arguments, return empty - if arguments empty - """ - if any(arguments): - folders = pathtable.query(" | ".join(arguments)) - else: - folders = pd.DataFrame() - return folders - - def parse_arguments(self, pathtable, list_of_names, field): - """ - Replicate datashuttle name formatting by parsing - "all" arguments and turning them into a list of all names, - (subject or session), taken from the pathtable. - """ - if list_of_names in [["all"], [f"all_{field}"]]: - entries = pathtable.query(f"parent_{field} != False")[ - f"parent_{field}" - ] - entries = list(set(entries)) - if list_of_names == ["all"]: - entries += ( - [f"all_non_{field}"] - if field != "datatype" - else ["all_non_datatype"] - ) - list_of_names = entries - return list_of_names - - def create_all_pathtable_files(self, pathtable): - """ - Create the entire test project in the defined - location (usually project's `local_path`). - """ - for i in range(pathtable.shape[0]): - filepath = pathtable["base_folder"][i] / pathtable["path"][i] - filepath.parents[0].mkdir(parents=True, exist_ok=True) - test_utils.write_file(filepath, contents="test_entry") - - def central_from_local(self, path_): - return Path(str(copy.copy(path_)).replace("local", "central")) From db78aab0581e52ccd5e638202f3b54b7c3982c31 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 15:38:36 +0100 Subject: [PATCH 38/57] SPlit ssh tests. --- tests/tests_integration/base.py | 13 ++ tests/tests_integration/test_ssh_setup.py | 7 +- .../{test_ssh.py => test_ssh_transfer.py} | 116 ++---------------- ...ilesystem_transfer.py => test_transfer.py} | 0 ....py => test_transfer_special_arguments.py} | 0 5 files changed, 28 insertions(+), 108 deletions(-) rename tests/tests_integration/{test_ssh.py => test_ssh_transfer.py} (57%) rename tests/tests_integration/{test_filesystem_transfer.py => test_transfer.py} (100%) rename tests/tests_integration/{test_ssh_file_transfer.py => test_transfer_special_arguments.py} (100%) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 98f1f5b80..2929d63ab 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -1,10 +1,12 @@ import os import platform import subprocess +import subprocess import warnings import pytest +import ssh_test_utils import test_utils from datashuttle import DataShuttle @@ -75,3 +77,14 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) + + @pytest.fixture( + scope="session", + ) + def setup_ssh_container(self): + # Annoying session scope does not seem to actually work + container_name = "running_ssh_tests" + ssh_test_utils.setup_ssh_container(container_name) + yield + subprocess.run(f"docker stop {container_name}", shell=True) + subprocess.run(f"docker rm {container_name}", shell=True) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 1a84e1ce5..a1c3123a9 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -3,7 +3,6 @@ import test_utils from base import BaseTest -# from pytest import ssh_config from datashuttle.utils import ssh TEST_SSH = ssh_test_utils.get_test_ssh() @@ -11,6 +10,7 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSH(BaseTest): + @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): """ @@ -20,6 +20,7 @@ def project(test, tmp_path, setup_ssh_container): tmp_path = tmp_path / "test with space" test_project_name = "test_ssh" + project = test_utils.setup_project_fixture(tmp_path, test_project_name) # ssh_test_utils.setup_project_and_container_for_ssh(project) @@ -79,7 +80,9 @@ def test_verify_ssh_central_host_accept(self, capsys, project): with open(project.cfg.hostkeys_path, "r") as file: hostkey = file.readlines()[0] - assert f"{project.cfg['central_host_id']} ssh-ed25519 " in hostkey + assert ( + f"[{project.cfg['central_host_id']}]:3306 ssh-ed25519 " in hostkey + ) def test_generate_and_write_ssh_key(self, project): """ diff --git a/tests/tests_integration/test_ssh.py b/tests/tests_integration/test_ssh_transfer.py similarity index 57% rename from tests/tests_integration/test_ssh.py rename to tests/tests_integration/test_ssh_transfer.py index 898890308..da090714b 100644 --- a/tests/tests_integration/test_ssh.py +++ b/tests/tests_integration/test_ssh_transfer.py @@ -1,10 +1,8 @@ import shutil -import subprocess import paramiko import pytest import ssh_test_utils -import test_utils from base_transfer import BaseTransfer # from pytest import ssh_config @@ -14,18 +12,7 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") -class TestSSH(BaseTransfer): - - @pytest.fixture( - scope="session", - ) - def setup_ssh_container(self): - # Annoying session scope does not seem to actually work - container_name = "running_ssh_tests" - ssh_test_utils.setup_ssh_container(container_name) - yield - subprocess.run(f"docker stop {container_name}", shell=True) - subprocess.run(f"docker rm {container_name}", shell=True) +class TestSSHTransfer(BaseTransfer): @pytest.fixture( scope="class", @@ -51,89 +38,6 @@ def ssh_setup(self, pathtable_and_project, setup_ssh_container): return [pathtable, project] - @pytest.fixture(scope="function") - def project(test, tmp_path, setup_ssh_container): - """ - Make a project as per usual, but now add - in test ssh configurations - """ - tmp_path = tmp_path / "test with space" - - test_project_name = "test_ssh" - project, cwd = test_utils.setup_project_fixture( - tmp_path, test_project_name - ) - ssh_test_utils.setup_project_for_ssh( - project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", - central_host_id="localhost", - central_host_username="sshuser", - ) - yield project - test_utils.teardown_project(cwd, project) - - # ----------------------------------------------------------------- - # Test Setup SSH Connection - # ----------------------------------------------------------------- - - @pytest.mark.parametrize("input_", ["n", "o", "@"]) - def test_verify_ssh_central_host_do_not_accept( - self, capsys, project, input_ - ): - """ - Use the main function to test this. Test the sub-function - when accepting, because this main function will also - call setup ssh key pairs which we don't want to do yet - - This should only accept for "y" so try some random strings - including "n" and check they all do not make the connection. - """ - orig_builtin = ssh_test_utils.setup_mock_input(input_) - - project.setup_ssh_connection() - - ssh_test_utils.restore_mock_input(orig_builtin) - - captured = capsys.readouterr() - - assert "Host not accepted. No connection made.\n" in captured.out - - def test_verify_ssh_central_host_accept(self, capsys, project): - """ - User is asked to accept the server hostkey. Mock this here - and check hostkey is successfully accepted and written to configs. - """ - test_utils.clear_capsys(capsys) - - verified = ssh_test_utils.setup_ssh_connection( - project, setup_ssh_key_pair=False - ) - - assert verified - captured = capsys.readouterr() - - assert captured.out == "Host accepted.\n" - - with open(project.cfg.hostkeys_path, "r") as file: - hostkey = file.readlines()[0] - - assert ( - f"[{project.cfg['central_host_id']}]:3306 ssh-ed25519 " in hostkey - ) - - def test_generate_and_write_ssh_key(self, project): - """ - Check ssh key for passwordless connection is written - to file - """ - path_to_save = project.cfg["local_path"] / "test" - ssh.generate_and_write_ssh_key(path_to_save) - - with open(path_to_save, "r") as file: - first_line = file.readlines()[0] - - assert first_line == "-----BEGIN RSA PRIVATE KEY-----\n" - # ----------------------------------------------------------------- # Test Setup SSH Connection # ----------------------------------------------------------------- @@ -174,9 +78,7 @@ def test_combinations_ssh_transfer( tmp_central_path = ( project.cfg["central_path"] / "tmp" / project.project_name ) - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary + self.remake_logging_path(project) project.update_config_file(central_path=tmp_central_path) @@ -237,10 +139,12 @@ def test_combinations_ssh_transfer( shutil.rmtree(tmp_local_path) - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary + self.remake_logging_path(project) project.update_config_file(local_path=true_local_path) - project.get_logging_path().mkdir( - parents=True, exist_ok=True - ) # TODO: why is this necessary + + def remake_logging_path(self, project): + """ + Need to do this to compensate for switching + local_path location in the test environment. + """ + project.get_logging_path().mkdir(parents=True, exist_ok=True) # TOD diff --git a/tests/tests_integration/test_filesystem_transfer.py b/tests/tests_integration/test_transfer.py similarity index 100% rename from tests/tests_integration/test_filesystem_transfer.py rename to tests/tests_integration/test_transfer.py diff --git a/tests/tests_integration/test_ssh_file_transfer.py b/tests/tests_integration/test_transfer_special_arguments.py similarity index 100% rename from tests/tests_integration/test_ssh_file_transfer.py rename to tests/tests_integration/test_transfer_special_arguments.py From 9c6c064a49e0200d5a10529f7cf22e12069e9fd4 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 15:59:40 +0100 Subject: [PATCH 39/57] Add sudo to the docker teardown commands for Linux. --- tests/tests_integration/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 2929d63ab..501e7c6a0 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -86,5 +86,8 @@ def setup_ssh_container(self): container_name = "running_ssh_tests" ssh_test_utils.setup_ssh_container(container_name) yield - subprocess.run(f"docker stop {container_name}", shell=True) - subprocess.run(f"docker rm {container_name}", shell=True) + + sudo = "sudo " if platform.system() == "Linux" else "" + + subprocess.run(f"{sudo}docker stop {container_name}", shell=True) + subprocess.run(f"{sudo}rm {container_name}", shell=True) From 5466e4cf4c6fb31d6486fb34541b348f78f7617c Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 16:06:44 +0100 Subject: [PATCH 40/57] try class scope of setup ssh container. --- tests/tests_integration/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 501e7c6a0..6a83c7d9e 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -79,7 +79,7 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) @pytest.fixture( - scope="session", + scope="class", ) def setup_ssh_container(self): # Annoying session scope does not seem to actually work From e2aca7b94cb83886db1c8a03497c0b63fa6e9339 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 16:22:01 +0100 Subject: [PATCH 41/57] Try move ssh fixture to classes. --- tests/tests_integration/base.py | 15 --------------- tests/tests_integration/test_ssh_setup.py | 17 +++++++++++++++++ tests/tests_integration/test_ssh_transfer.py | 16 ++++++++++++++++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 6a83c7d9e..565b2d4b6 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -6,7 +6,6 @@ import warnings import pytest -import ssh_test_utils import test_utils from datashuttle import DataShuttle @@ -77,17 +76,3 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) - - @pytest.fixture( - scope="class", - ) - def setup_ssh_container(self): - # Annoying session scope does not seem to actually work - container_name = "running_ssh_tests" - ssh_test_utils.setup_ssh_container(container_name) - yield - - sudo = "sudo " if platform.system() == "Linux" else "" - - subprocess.run(f"{sudo}docker stop {container_name}", shell=True) - subprocess.run(f"{sudo}rm {container_name}", shell=True) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index a1c3123a9..c69f06dc5 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -1,3 +1,6 @@ +import platform +import subprocess + import pytest import ssh_test_utils import test_utils @@ -11,6 +14,20 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSH(BaseTest): + @pytest.fixture( + scope="class", + ) + def setup_ssh_container(self): + # Annoying session scope does not seem to actually work + container_name = "running_ssh_tests" + ssh_test_utils.setup_ssh_container(container_name) + yield + + sudo = "sudo " if platform.system() == "Linux" else "" + + subprocess.run(f"{sudo}docker stop {container_name}", shell=True) + subprocess.run(f"{sudo}rm {container_name}", shell=True) + @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): """ diff --git a/tests/tests_integration/test_ssh_transfer.py b/tests/tests_integration/test_ssh_transfer.py index da090714b..8bf245c58 100644 --- a/tests/tests_integration/test_ssh_transfer.py +++ b/tests/tests_integration/test_ssh_transfer.py @@ -1,4 +1,6 @@ +import platform import shutil +import subprocess import paramiko import pytest @@ -14,6 +16,20 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSHTransfer(BaseTransfer): + @pytest.fixture( + scope="class", + ) + def setup_ssh_container(self): + # Annoying session scope does not seem to actually work + container_name = "running_ssh_tests" + ssh_test_utils.setup_ssh_container(container_name) + yield + + sudo = "sudo " if platform.system() == "Linux" else "" + + subprocess.run(f"{sudo}docker stop {container_name}", shell=True) + subprocess.run(f"{sudo}rm {container_name}", shell=True) + @pytest.fixture( scope="class", ) From e157bb4acc0181244c35eb1a458c94721a9c3f77 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 16:47:44 +0100 Subject: [PATCH 42/57] Try a different command to shutdown on linux. --- tests/tests_integration/test_ssh_setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index c69f06dc5..715988a63 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -25,8 +25,7 @@ def setup_ssh_container(self): sudo = "sudo " if platform.system() == "Linux" else "" - subprocess.run(f"{sudo}docker stop {container_name}", shell=True) - subprocess.run(f"{sudo}rm {container_name}", shell=True) + subprocess.run(f"{sudo}docker rm -f {container_name}", shell=True) @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): From 905a09387292ca10cf681433fa92408e6d4aa537 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 17:58:54 +0100 Subject: [PATCH 43/57] Tidy ups and some docs. --- .github/workflows/code_test_and_deploy.yml | 3 + tests/ssh_test_utils.py | 120 ++++++++++--------- tests/tests_integration/base.py | 14 +++ tests/tests_integration/base_transfer.py | 8 +- tests/tests_integration/test_ssh_setup.py | 22 +--- tests/tests_integration/test_ssh_transfer.py | 23 +--- 6 files changed, 90 insertions(+), 100 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index c8ee23e2b..0d27fc10a 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -58,6 +58,9 @@ jobs: python -m pip install --upgrade pip pip install .[dev] - name: Test + # run SSH tests only on Linux because Windows and macOS + # are already run within a virtual container and so cannot + # run Linux containers because nested containerisation is disabled. run: | if [ "$RUNNER_OS" == "Linux" ]; then sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index e55555597..9459e428d 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -16,22 +16,26 @@ from datashuttle.utils import rclone, ssh -PORT = 3306 # https://github.com/orgs/community/discussions/25550 +# Choose port 3306 for running on GH actions +# suggested in https://github.com/orgs/community/discussions/25550 +PORT = 3306 os.environ["DS_SSH_PORT"] = str(PORT) def setup_project_for_ssh( - project, central_path, central_host_id, central_host_username + project, ): """ - Set up the project configs to use SSH connection - to central + Set up the project configs to use + SSH connection to central. The settings + set up a connection to the Dockerfile image + found in /ssh_test_images. """ project.update_config_file( connection_method="ssh", - central_path=central_path, - central_host_id=central_host_id, - central_host_username=central_host_username, + central_path=f"/home/sshuser/datashuttle/{project.project_name}", + central_host_id="localhost", + central_host_username="sshuser", ) rclone.setup_rclone_config_for_ssh( project.cfg, @@ -40,28 +44,10 @@ def setup_project_for_ssh( ) -def setup_mock_input(input_): - """ - This is very similar to pytest monkeypatch but - using that was giving me very strange output, - monkeypatch.setattr('builtins.input', lambda _: "n") - i.e. pdb went deep into some unrelated code stack - """ - orig_builtin = copy.deepcopy(builtins.input) - builtins.input = lambda _: input_ # type: ignore - return orig_builtin - - -def restore_mock_input(orig_builtin): - """ - orig_builtin: the copied, original builtins.input - """ - builtins.input = orig_builtin - - def setup_ssh_connection(project, setup_ssh_key_pair=True): """ - Convenience function to verify the server hostkey. + Convenience function to verify the server hostkey and ssh + key pairs to the Dockerfile image for ssh tests. This requires monkeypatching a number of functions involved in the SSH setup process. `input()` is patched to always @@ -72,7 +58,9 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): container thing. """ # Monkeypatch - orig_builtin = setup_mock_input(input_="y") + orig_builtin = copy.deepcopy(builtins.input) + builtins.input = lambda _: "y" # type: ignore + orig_getpass = copy.deepcopy(ssh.getpass.getpass) ssh.getpass.getpass = lambda _: "password" # type: ignore @@ -88,7 +76,7 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): ssh.setup_ssh_key(project.cfg, log=False) # Restore functions - restore_mock_input(orig_builtin) + builtins.input = orig_builtin ssh.getpass.getpass = orig_getpass sys.stdin.isatty = orig_isatty @@ -96,7 +84,10 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): def setup_ssh_container(container_name): - """""" + """ + Build and run the docker container used for + ssh tests. + """ assert docker_is_running(), ( "docker is not running, " "this should be checked at the top of test script" @@ -107,7 +98,10 @@ def setup_ssh_container(container_name): if platform.system() == "Linux": build_command = "sudo docker build -t ssh_server ." - run_command = f"sudo docker run -d -p {PORT}:22 --name {container_name} ssh_server" + run_command = ( + f"sudo docker run -d -p {PORT}:22 " + f"--name {container_name} ssh_server" + ) else: build_command = "docker build ." run_command = ( @@ -119,9 +113,10 @@ def setup_ssh_container(container_name): shell=True, capture_output=True, ) - assert ( - build_output.returncode == 0 - ), f"docker build failed with: STDOUT-{build_output.stdout} STDERR-{build_output.stderr}" + assert build_output.returncode == 0, ( + f"docker build failed with: STDOUT-{build_output.stdout} " + f"STDERR-{build_output.stderr}" + ) run_output = subprocess.run( run_command, @@ -129,12 +124,39 @@ def setup_ssh_container(container_name): capture_output=True, ) - assert ( - run_output.returncode == 0 - ), f"docker run failed with: STDOUT-{run_output.stdout} STDERR-{run_output.stderr}" + assert run_output.returncode == 0, ( + f"docker run failed with: STDOUT-{run_output.stdout} " + f"STDERR-{run_output.stderr}" + ) + + +def recursive_search_central(project): + """ + A convenience function to recursively search a + project for files through SSH, used during testing + across an SSH connection to collected names of + files that were transferred. + """ + with paramiko.SSHClient() as client: + ssh.connect_client_core(client, project.cfg) + + sftp = client.open_sftp() + + all_filenames = [] + + sftp_recursive_file_search( + sftp, + (project.cfg["central_path"] / "rawdata").as_posix(), + all_filenames, + ) + return all_filenames def sftp_recursive_file_search(sftp, path_, all_filenames): + """ + Append all filenames found within a folder, + when searching over a sftp connection. + """ try: sftp.stat(path_) except FileNotFoundError: @@ -151,25 +173,11 @@ def sftp_recursive_file_search(sftp, path_, all_filenames): all_filenames.append(path_ + "/" + file_or_folder.filename) -def recursive_search_central(project): - """ """ - with paramiko.SSHClient() as client: - ssh.connect_client_core(client, project.cfg) - - sftp = client.open_sftp() - - all_filenames = [] - - sftp_recursive_file_search( - sftp, - (project.cfg["central_path"] / "rawdata").as_posix(), - all_filenames, - ) - return all_filenames - - def get_test_ssh(): - """""" + """ + Return bool indicating whether Docker is installed and running, + which is required for ssh tests. + """ docker_installed = docker_is_running() if not docker_installed: warnings.warn( @@ -179,7 +187,6 @@ def get_test_ssh(): def docker_is_running(): - """""" if not is_docker_installed(): return False @@ -188,7 +195,6 @@ def docker_is_running(): def is_docker_installed(): - """""" return check_sys_command_returns_0("docker -v") diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 565b2d4b6..758abdfac 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -6,6 +6,7 @@ import warnings import pytest +import ssh_test_utils import test_utils from datashuttle import DataShuttle @@ -76,3 +77,16 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) + + @pytest.fixture( + scope="class", + ) + def setup_ssh_container(self): + # Annoying session scope does not seem to actually work + container_name = "running_ssh_tests" + ssh_test_utils.setup_ssh_container(container_name) + yield + + sudo = "sudo " if platform.system() == "Linux" else "" + + subprocess.run(f"{sudo}docker rm -f {container_name}", shell=True) diff --git a/tests/tests_integration/base_transfer.py b/tests/tests_integration/base_transfer.py index 3af49def6..9e61fb4de 100644 --- a/tests/tests_integration/base_transfer.py +++ b/tests/tests_integration/base_transfer.py @@ -12,10 +12,10 @@ class BaseTransfer(BaseTest): - - # ---------------------------------------------------------------------------------- - # Test File Transfer - All Options - # ---------------------------------------------------------------------------------- + """ + Class holding fixtures and methods for testing the + custom transfers with keys (e.g. all_non_sub). + """ @pytest.fixture( scope="class", diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 715988a63..2e2270037 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -1,5 +1,5 @@ -import platform -import subprocess +import builtins +import copy import pytest import ssh_test_utils @@ -14,19 +14,6 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSH(BaseTest): - @pytest.fixture( - scope="class", - ) - def setup_ssh_container(self): - # Annoying session scope does not seem to actually work - container_name = "running_ssh_tests" - ssh_test_utils.setup_ssh_container(container_name) - yield - - sudo = "sudo " if platform.system() == "Linux" else "" - - subprocess.run(f"{sudo}docker rm -f {container_name}", shell=True) - @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): """ @@ -67,11 +54,12 @@ def test_verify_ssh_central_host_do_not_accept( This should only accept for "y" so try some random strings including "n" and check they all do not make the connection. """ - orig_builtin = ssh_test_utils.setup_mock_input(input_) + orig_builtin = copy.deepcopy(builtins.input) + builtins.input = lambda _: "y" # type: ignore project.setup_ssh_connection() - ssh_test_utils.restore_mock_input(orig_builtin) + builtins.input = orig_builtin captured = capsys.readouterr() diff --git a/tests/tests_integration/test_ssh_transfer.py b/tests/tests_integration/test_ssh_transfer.py index 8bf245c58..51ba86b5e 100644 --- a/tests/tests_integration/test_ssh_transfer.py +++ b/tests/tests_integration/test_ssh_transfer.py @@ -1,13 +1,10 @@ -import platform import shutil -import subprocess import paramiko import pytest import ssh_test_utils from base_transfer import BaseTransfer -# from pytest import ssh_config from datashuttle.utils import ssh TEST_SSH = ssh_test_utils.get_test_ssh() @@ -16,20 +13,6 @@ @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") class TestSSHTransfer(BaseTransfer): - @pytest.fixture( - scope="class", - ) - def setup_ssh_container(self): - # Annoying session scope does not seem to actually work - container_name = "running_ssh_tests" - ssh_test_utils.setup_ssh_container(container_name) - yield - - sudo = "sudo " if platform.system() == "Linux" else "" - - subprocess.run(f"{sudo}docker stop {container_name}", shell=True) - subprocess.run(f"{sudo}rm {container_name}", shell=True) - @pytest.fixture( scope="class", ) @@ -43,11 +26,7 @@ def ssh_setup(self, pathtable_and_project, setup_ssh_container): ssh_test_utils.setup_project_for_ssh( project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", - central_host_id="localhost", - central_host_username="sshuser", ) - ssh_test_utils.setup_ssh_connection(project) project.upload_rawdata() @@ -163,4 +142,4 @@ def remake_logging_path(self, project): Need to do this to compensate for switching local_path location in the test environment. """ - project.get_logging_path().mkdir(parents=True, exist_ok=True) # TOD + project.get_logging_path().mkdir(parents=True, exist_ok=True) From ae192b35997a81f5da14bb7c85569ffb92b34ca5 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 17:08:39 +0100 Subject: [PATCH 44/57] Extend to macOS. --- tests/ssh_test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index 9459e428d..bd057c997 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -96,7 +96,7 @@ def setup_ssh_container(container_name): image_path = Path(__file__).parent / "ssh_test_images" os.chdir(image_path) - if platform.system() == "Linux": + if platform.system() != "Windows": build_command = "sudo docker build -t ssh_server ." run_command = ( f"sudo docker run -d -p {PORT}:22 " From 01b020eab2ae794121a9760221548fd55be288f8 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 18:06:47 +0100 Subject: [PATCH 45/57] Finish tidying up docstrings. --- tests/tests_integration/base.py | 8 ++++---- tests/tests_integration/test_ssh_setup.py | 17 ++++++----------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 758abdfac..5bb402391 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -19,8 +19,8 @@ class BaseTest: @pytest.fixture(scope="function") def no_cfg_project(test): """ - Fixture that creates an empty project. Ignore the warning - that no configs are setup yet. + Fixture that creates an empty project. Ignore the + warning that no configs are set up yet. """ test_utils.delete_project_if_it_exists(TEST_PROJECT_NAME) @@ -70,8 +70,8 @@ def project(self, tmp_path, request): def clean_project_name(self): """ Create an empty project, but ensure no - configs already exists, and delete created configs - after test. + configs already exists, and delete created + configs after test. """ project_name = TEST_PROJECT_NAME test_utils.delete_project_if_it_exists(project_name) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index 2e2270037..e2802263e 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -17,8 +17,8 @@ class TestSSH(BaseTest): @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): """ - Make a project as per usual, but now add - in test ssh configurations + Set up a project with configs for SSH into + the test Dockerfile image. """ tmp_path = tmp_path / "test with space" @@ -47,15 +47,10 @@ def test_verify_ssh_central_host_do_not_accept( self, capsys, project, input_ ): """ - Use the main function to test this. Test the sub-function - when accepting, because this main function will also - call setup ssh key pairs which we don't want to do yet - - This should only accept for "y" so try some random strings - including "n" and check they all do not make the connection. + Test that host not accepted if input is not "y". """ orig_builtin = copy.deepcopy(builtins.input) - builtins.input = lambda _: "y" # type: ignore + builtins.input = lambda _: input_ # type: ignore project.setup_ssh_connection() @@ -90,8 +85,8 @@ def test_verify_ssh_central_host_accept(self, capsys, project): def test_generate_and_write_ssh_key(self, project): """ - Check ssh key for passwordless connection is written - to file + Check ssh key for passwordless connection + is written to file. """ path_to_save = project.cfg["local_path"] / "test" ssh.generate_and_write_ssh_key(path_to_save) From 5af3addc557c3194ea425c7961f3e3385674450b Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 18:13:20 +0100 Subject: [PATCH 46/57] Change ssh test image name and fix docstring. --- tests/tests_integration/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index 5bb402391..cdfe94e94 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -82,8 +82,11 @@ def clean_project_name(self): scope="class", ) def setup_ssh_container(self): - # Annoying session scope does not seem to actually work - container_name = "running_ssh_tests" + """ + Set up the Dockerfile container for SSH tests and + delete it on teardown. + """ + container_name = "datashuttle_ssh_tests" ssh_test_utils.setup_ssh_container(container_name) yield From 679f3b8342693874b3224fe42ecd85128dfd26d9 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 22 Apr 2024 18:22:36 +0100 Subject: [PATCH 47/57] Small tidy ups. --- datashuttle/utils/ssh.py | 2 -- tests/ssh_test_utils.py | 4 ---- 2 files changed, 6 deletions(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index 5f5ea2b65..570240859 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -14,8 +14,6 @@ import paramiko -PORT = 3306 - from datashuttle.configs import canonical_configs from datashuttle.utils import utils diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index bd057c997..c672e5ae1 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -1,7 +1,3 @@ -""" - -""" - import builtins import copy import os From c29e25043f7709594b7687f9ca00b03d53a4ce5c Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 00:19:01 +0100 Subject: [PATCH 48/57] Small fixes after rebase. --- tests/tests_integration/base.py | 8 +-- tests/tests_integration/base_transfer.py | 7 +-- tests/tests_integration/test_ssh_setup.py | 5 -- tests/tests_integration/test_ssh_transfer.py | 1 - .../test_transfer_special_arguments.py | 59 +------------------ 5 files changed, 6 insertions(+), 74 deletions(-) diff --git a/tests/tests_integration/base.py b/tests/tests_integration/base.py index cdfe94e94..d6378c7cc 100644 --- a/tests/tests_integration/base.py +++ b/tests/tests_integration/base.py @@ -1,8 +1,4 @@ -import os -import platform import subprocess -import subprocess - import warnings import pytest @@ -90,6 +86,4 @@ def setup_ssh_container(self): ssh_test_utils.setup_ssh_container(container_name) yield - sudo = "sudo " if platform.system() == "Linux" else "" - - subprocess.run(f"{sudo}docker rm -f {container_name}", shell=True) + subprocess.run(f"docker rm -f {container_name}", shell=True) diff --git a/tests/tests_integration/base_transfer.py b/tests/tests_integration/base_transfer.py index 9e61fb4de..8222f4817 100644 --- a/tests/tests_integration/base_transfer.py +++ b/tests/tests_integration/base_transfer.py @@ -1,5 +1,4 @@ -""" -""" +""" """ import copy from pathlib import Path @@ -30,7 +29,7 @@ def pathtable_and_project(self, tmpdir_factory): base_path = tmp_path / "test with space" test_project_name = "test_file_conflicts" - project, cwd = test_utils.setup_project_fixture( + project = test_utils.setup_project_fixture( base_path, test_project_name ) @@ -40,7 +39,7 @@ def pathtable_and_project(self, tmpdir_factory): yield [pathtable, project] - test_utils.teardown_project(cwd, project) + test_utils.teardown_project(project) def get_expected_transferred_paths( self, pathtable, sub_names, ses_names, datatype diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index e2802263e..c868c16e4 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -26,13 +26,8 @@ def project(test, tmp_path, setup_ssh_container): project = test_utils.setup_project_fixture(tmp_path, test_project_name) - # ssh_test_utils.setup_project_and_container_for_ssh(project) - ssh_test_utils.setup_project_for_ssh( project, - central_path=f"/home/sshuser/datashuttle/{project.project_name}", - central_host_id="localhost", - central_host_username="sshuser", ) yield project diff --git a/tests/tests_integration/test_ssh_transfer.py b/tests/tests_integration/test_ssh_transfer.py index 51ba86b5e..691b0267c 100644 --- a/tests/tests_integration/test_ssh_transfer.py +++ b/tests/tests_integration/test_ssh_transfer.py @@ -37,7 +37,6 @@ def ssh_setup(self, pathtable_and_project, setup_ssh_container): # Test Setup SSH Connection # ----------------------------------------------------------------- - @pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") @pytest.mark.parametrize( "sub_names", [["all"], ["all_non_sub", "sub-002"]] ) diff --git a/tests/tests_integration/test_transfer_special_arguments.py b/tests/tests_integration/test_transfer_special_arguments.py index 1d589787e..6458b3d4c 100644 --- a/tests/tests_integration/test_transfer_special_arguments.py +++ b/tests/tests_integration/test_transfer_special_arguments.py @@ -15,7 +15,7 @@ ["all_sub"], ["all_non_sub"], ["sub-001"], - ["sub-003_date-20231901"], + ["sub-003_date-20231201"], ["sub-002", "all_non_sub"], ] PARAM_SES = [ @@ -37,63 +37,8 @@ ["anat", "behav", "all_non_datatype"], ] -class TestFileTransfer: - @pytest.fixture( - scope="class", - ) - def pathtable_and_project(self, tmpdir_factory): - """ - Create a new test project with a test project folder - and file structure (see `get_pathtable()` for definition). - """ - tmp_path = tmpdir_factory.mktemp("test") - - base_path = tmp_path / "test with space" - test_project_name = "test_file_conflicts" - - project = test_utils.setup_project_fixture( - base_path, test_project_name - ) - - if testing_ssh: - ssh_test_utils.setup_project_for_ssh( - project, - test_utils.make_test_path( - central_path, "central", test_project_name - ), - ssh_config.CENTRAL_HOST_ID, - ssh_config.USERNAME, - ) - - # Initialise the SSH connection - ssh_test_utils.build_docker_image(project) - - ssh_test_utils.setup_hostkeys(project) - - pathtable = get_pathtable(project.cfg["local_path"]) - - self.create_all_pathtable_files(pathtable) - - yield [pathtable, project] - - test_utils.teardown_project(project) - - @pytest.fixture( - scope="class", - ) - def ssh_setup(self, pathtable_and_project): - """ - After initial project setup (in `pathtable_and_project`) - setup a container and the project's SSH connection to the container. - Then upload the test project to the `central_path`. - """ - pathtable, project = pathtable_and_project - ssh_test_utils.setup_project_and_container_for_ssh(project) - ssh_test_utils.setup_ssh_connection(project) - - project.upload_rawdata() - return [pathtable, project] +class TestFileTransfer(BaseTransfer): # ---------------------------------------------------------------------------------- # Test File Transfer - All Options From 6d417d1db018f25a3bd81042c4047aa317d2a556 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 00:26:21 +0100 Subject: [PATCH 49/57] More fix.es --- .github/workflows/code_test_and_deploy.yml | 2 +- tests/ssh_test_images/Dockerfile | 2 +- tests/ssh_test_utils.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 0d27fc10a..49bf31e49 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -64,7 +64,7 @@ jobs: run: | if [ "$RUNNER_OS" == "Linux" ]; then sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 - pytest + pytest -k "test_ssh_setup and test_combinations_ssh_transfer" else pytest -k "not test_combinations_ssh_transfer and not test_ssh_setup" fi diff --git a/tests/ssh_test_images/Dockerfile b/tests/ssh_test_images/Dockerfile index 84d5035b7..474c8ecb3 100644 --- a/tests/ssh_test_images/Dockerfile +++ b/tests/ssh_test_images/Dockerfile @@ -8,7 +8,7 @@ RUN apt-get install openssh-server -y supervisor RUN apt-get install nano # Create an SSH user -RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser +RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo sshuser # Set the SSH user's password (replace "password" with your desired password) RUN echo "sshuser:password" | chpasswd diff --git a/tests/ssh_test_utils.py b/tests/ssh_test_utils.py index c672e5ae1..7582bc252 100644 --- a/tests/ssh_test_utils.py +++ b/tests/ssh_test_utils.py @@ -99,7 +99,7 @@ def setup_ssh_container(container_name): f"--name {container_name} ssh_server" ) else: - build_command = "docker build ." + build_command = "docker build -t ssh_server ." run_command = ( f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" ) From b7f9db830f63a05b44f258ce24c3a75f113a3fb2 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 00:31:44 +0100 Subject: [PATCH 50/57] Fix CI. --- .github/workflows/code_test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 49bf31e49..83aea1e9a 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -64,7 +64,7 @@ jobs: run: | if [ "$RUNNER_OS" == "Linux" ]; then sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 - pytest -k "test_ssh_setup and test_combinations_ssh_transfer" + pytest -k "test_combinations_ssh_transfer or test_ssh_setup" else pytest -k "not test_combinations_ssh_transfer and not test_ssh_setup" fi From 8712bff470f7bfdf8eeba167be6f783bd7b7e195 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 01:09:22 +0100 Subject: [PATCH 51/57] Skip tests on macOS. --- tests/tests_integration/test_ssh_setup.py | 6 +++++- tests/tests_integration/test_ssh_transfer.py | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_integration/test_ssh_setup.py index c868c16e4..b0f5ee95c 100644 --- a/tests/tests_integration/test_ssh_setup.py +++ b/tests/tests_integration/test_ssh_setup.py @@ -1,5 +1,6 @@ import builtins import copy +import platform import pytest import ssh_test_utils @@ -11,7 +12,10 @@ TEST_SSH = ssh_test_utils.get_test_ssh() -@pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") +@pytest.mark.skipif( + platform.system == "Darwin", reason="Docker set up is not robust on macOS." +) +@pytest.mark.skipif(not TEST_SSH, reason="TEST_SSH is false") class TestSSH(BaseTest): @pytest.fixture(scope="function") diff --git a/tests/tests_integration/test_ssh_transfer.py b/tests/tests_integration/test_ssh_transfer.py index 691b0267c..e5a1b175e 100644 --- a/tests/tests_integration/test_ssh_transfer.py +++ b/tests/tests_integration/test_ssh_transfer.py @@ -1,3 +1,4 @@ +import platform import shutil import paramiko @@ -10,7 +11,10 @@ TEST_SSH = ssh_test_utils.get_test_ssh() -@pytest.mark.skipif("not TEST_SSH", reason="TEST_SSH is false") +@pytest.mark.skipif( + platform.system == "Darwin", reason="Docker set up is not robust on macOS." +) +@pytest.mark.skipif(not TEST_SSH, reason="TEST_SSH is false") class TestSSHTransfer(BaseTransfer): @pytest.fixture( From 33e20dee496efde74158a2976931b2f55ffccebe Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 15:06:39 +0100 Subject: [PATCH 52/57] Refactor transfer tests. --- tests/{tests_integration => }/base.py | 0 tests/{tests_integration => tests_transfers}/base_transfer.py | 0 tests/{ => tests_transfers}/ssh_test_images/Dockerfile | 0 tests/{tests_integration => tests_transfers}/test_ssh_setup.py | 0 tests/{tests_integration => tests_transfers}/test_ssh_transfer.py | 0 tests/{tests_integration => tests_transfers}/test_transfer.py | 0 .../test_transfer_checks.py | 0 .../test_transfer_special_arguments.py | 0 8 files changed, 0 insertions(+), 0 deletions(-) rename tests/{tests_integration => }/base.py (100%) rename tests/{tests_integration => tests_transfers}/base_transfer.py (100%) rename tests/{ => tests_transfers}/ssh_test_images/Dockerfile (100%) rename tests/{tests_integration => tests_transfers}/test_ssh_setup.py (100%) rename tests/{tests_integration => tests_transfers}/test_ssh_transfer.py (100%) rename tests/{tests_integration => tests_transfers}/test_transfer.py (100%) rename tests/{tests_integration => tests_transfers}/test_transfer_checks.py (100%) rename tests/{tests_integration => tests_transfers}/test_transfer_special_arguments.py (100%) diff --git a/tests/tests_integration/base.py b/tests/base.py similarity index 100% rename from tests/tests_integration/base.py rename to tests/base.py diff --git a/tests/tests_integration/base_transfer.py b/tests/tests_transfers/base_transfer.py similarity index 100% rename from tests/tests_integration/base_transfer.py rename to tests/tests_transfers/base_transfer.py diff --git a/tests/ssh_test_images/Dockerfile b/tests/tests_transfers/ssh_test_images/Dockerfile similarity index 100% rename from tests/ssh_test_images/Dockerfile rename to tests/tests_transfers/ssh_test_images/Dockerfile diff --git a/tests/tests_integration/test_ssh_setup.py b/tests/tests_transfers/test_ssh_setup.py similarity index 100% rename from tests/tests_integration/test_ssh_setup.py rename to tests/tests_transfers/test_ssh_setup.py diff --git a/tests/tests_integration/test_ssh_transfer.py b/tests/tests_transfers/test_ssh_transfer.py similarity index 100% rename from tests/tests_integration/test_ssh_transfer.py rename to tests/tests_transfers/test_ssh_transfer.py diff --git a/tests/tests_integration/test_transfer.py b/tests/tests_transfers/test_transfer.py similarity index 100% rename from tests/tests_integration/test_transfer.py rename to tests/tests_transfers/test_transfer.py diff --git a/tests/tests_integration/test_transfer_checks.py b/tests/tests_transfers/test_transfer_checks.py similarity index 100% rename from tests/tests_integration/test_transfer_checks.py rename to tests/tests_transfers/test_transfer_checks.py diff --git a/tests/tests_integration/test_transfer_special_arguments.py b/tests/tests_transfers/test_transfer_special_arguments.py similarity index 100% rename from tests/tests_integration/test_transfer_special_arguments.py rename to tests/tests_transfers/test_transfer_special_arguments.py From de756a2b7e70e360a4bc916386286438febd9364 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 15:32:10 +0100 Subject: [PATCH 53/57] Continue refactoring. --- tests/base.py | 16 ----- tests/tests_transfers/base_transfer.py | 62 +++++++++++++++++++ tests/{ => tests_transfers}/ssh_test_utils.py | 55 ---------------- 3 files changed, 62 insertions(+), 71 deletions(-) rename tests/{ => tests_transfers}/ssh_test_utils.py (73%) diff --git a/tests/base.py b/tests/base.py index d6378c7cc..78cf1e6cb 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1,8 +1,6 @@ -import subprocess import warnings import pytest -import ssh_test_utils import test_utils from datashuttle import DataShuttle @@ -73,17 +71,3 @@ def clean_project_name(self): test_utils.delete_project_if_it_exists(project_name) yield project_name test_utils.delete_project_if_it_exists(project_name) - - @pytest.fixture( - scope="class", - ) - def setup_ssh_container(self): - """ - Set up the Dockerfile container for SSH tests and - delete it on teardown. - """ - container_name = "datashuttle_ssh_tests" - ssh_test_utils.setup_ssh_container(container_name) - yield - - subprocess.run(f"docker rm -f {container_name}", shell=True) diff --git a/tests/tests_transfers/base_transfer.py b/tests/tests_transfers/base_transfer.py index 8222f4817..d81056546 100644 --- a/tests/tests_transfers/base_transfer.py +++ b/tests/tests_transfers/base_transfer.py @@ -1,14 +1,23 @@ """ """ import copy +import os +import platform +import subprocess from pathlib import Path import pandas as pd import pytest +import ssh_test_utils import test_utils from base import BaseTest from file_conflicts_pathtable import get_pathtable +# Choose port 3306 for running on GH actions +# suggested in https://github.com/orgs/community/discussions/25550 +PORT = 3306 +os.environ["DS_SSH_PORT"] = str(PORT) + class BaseTransfer(BaseTest): """ @@ -16,6 +25,59 @@ class BaseTransfer(BaseTest): custom transfers with keys (e.g. all_non_sub). """ + @pytest.fixture( + scope="class", + ) + def setup_ssh_container(self): + """ + Set up the Dockerfile container for SSH tests and + delete it on teardown. + """ + container_name = "datashuttle_ssh_tests" + + assert ssh_test_utils.docker_is_running(), ( + "docker is not running, " + "this should be checked at the top of test script" + ) + + image_path = Path(__file__).parent / "ssh_test_images" + os.chdir(image_path) + + if platform.system() != "Windows": + build_command = "sudo docker build -t ssh_server ." + run_command = ( + f"sudo docker run -d -p {PORT}:22 " + f"--name {container_name} ssh_server" + ) + else: + build_command = "docker build -t ssh_server ." + run_command = f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" + + build_output = subprocess.run( + build_command, + shell=True, + capture_output=True, + ) + assert build_output.returncode == 0, ( + f"docker build failed with: STDOUT-{build_output.stdout} " + f"STDERR-{build_output.stderr}" + ) + + run_output = subprocess.run( + run_command, + shell=True, + capture_output=True, + ) + + assert run_output.returncode == 0, ( + f"docker run failed with: STDOUT-{run_output.stdout} " + f"STDERR-{run_output.stderr}" + ) + + yield + + subprocess.run(f"docker rm -f {container_name}", shell=True) + @pytest.fixture( scope="class", ) diff --git a/tests/ssh_test_utils.py b/tests/tests_transfers/ssh_test_utils.py similarity index 73% rename from tests/ssh_test_utils.py rename to tests/tests_transfers/ssh_test_utils.py index 7582bc252..aa605019d 100644 --- a/tests/ssh_test_utils.py +++ b/tests/tests_transfers/ssh_test_utils.py @@ -1,22 +1,14 @@ import builtins import copy -import os -import platform import stat import subprocess import sys import warnings -from pathlib import Path import paramiko from datashuttle.utils import rclone, ssh -# Choose port 3306 for running on GH actions -# suggested in https://github.com/orgs/community/discussions/25550 -PORT = 3306 -os.environ["DS_SSH_PORT"] = str(PORT) - def setup_project_for_ssh( project, @@ -79,53 +71,6 @@ def setup_ssh_connection(project, setup_ssh_key_pair=True): return verified -def setup_ssh_container(container_name): - """ - Build and run the docker container used for - ssh tests. - """ - assert docker_is_running(), ( - "docker is not running, " - "this should be checked at the top of test script" - ) - - image_path = Path(__file__).parent / "ssh_test_images" - os.chdir(image_path) - - if platform.system() != "Windows": - build_command = "sudo docker build -t ssh_server ." - run_command = ( - f"sudo docker run -d -p {PORT}:22 " - f"--name {container_name} ssh_server" - ) - else: - build_command = "docker build -t ssh_server ." - run_command = ( - f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" - ) - - build_output = subprocess.run( - build_command, - shell=True, - capture_output=True, - ) - assert build_output.returncode == 0, ( - f"docker build failed with: STDOUT-{build_output.stdout} " - f"STDERR-{build_output.stderr}" - ) - - run_output = subprocess.run( - run_command, - shell=True, - capture_output=True, - ) - - assert run_output.returncode == 0, ( - f"docker run failed with: STDOUT-{run_output.stdout} " - f"STDERR-{run_output.stderr}" - ) - - def recursive_search_central(project): """ A convenience function to recursively search a From e457d3a06b648cb2a85eec741ee57444b07b8b40 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 15:57:55 +0100 Subject: [PATCH 54/57] Fix tests again. --- tests/tests_transfers/test_ssh_setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests_transfers/test_ssh_setup.py b/tests/tests_transfers/test_ssh_setup.py index b0f5ee95c..2835af819 100644 --- a/tests/tests_transfers/test_ssh_setup.py +++ b/tests/tests_transfers/test_ssh_setup.py @@ -5,7 +5,7 @@ import pytest import ssh_test_utils import test_utils -from base import BaseTest +from base_transfer import BaseTransfer from datashuttle.utils import ssh @@ -16,7 +16,7 @@ platform.system == "Darwin", reason="Docker set up is not robust on macOS." ) @pytest.mark.skipif(not TEST_SSH, reason="TEST_SSH is false") -class TestSSH(BaseTest): +class TestSSH(BaseTransfer): @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): From 19bcacd89c9fc1cf329d24c6a041f92fea8a9ba5 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 16:54:25 +0100 Subject: [PATCH 55/57] Update CI script. --- .github/workflows/code_test_and_deploy.yml | 15 ++-- pyproject.toml | 1 + tests/__init__.py | 0 tests/base.py | 3 +- tests/conftest.py | 47 ------------ tests/quick_make_project.py | 5 -- tests/test_utils.py | 14 ---- tests/tests_integration/__init__.py | 0 tests/tests_integration/test_configs.py | 5 +- .../tests_integration/test_create_folders.py | 5 +- tests/tests_integration/test_datatypes.py | 5 +- tests/tests_integration/test_formatting.py | 3 +- .../tests_integration/test_local_only_mode.py | 5 +- tests/tests_integration/test_logging.py | 3 +- tests/tests_integration/test_settings.py | 3 +- tests/tests_integration/test_validation.py | 3 +- tests/tests_regression/__init__.py | 0 .../test_backwards_compatibility.py | 3 +- tests/tests_transfers/__init__.py | 0 tests/tests_transfers/base_transfer.py | 67 +--------------- .../file_conflicts_pathtable.py | 0 .../local_filesystem/__init__.py | 0 .../{ => local_filesystem}/test_transfer.py | 5 +- .../test_transfer_checks.py | 5 +- .../test_transfer_special_arguments.py | 6 +- tests/tests_transfers/ssh/__init__.py | 0 tests/tests_transfers/ssh/base_ssh.py | 76 +++++++++++++++++++ .../{ => ssh}/ssh_test_images/Dockerfile | 0 .../{ => ssh}/ssh_test_utils.py | 0 .../{ => ssh}/test_ssh_setup.py | 9 ++- .../{ => ssh}/test_ssh_transfer.py | 7 +- tests/tests_tui/__init__.py | 0 tests/tests_tui/test_local_only_project.py | 3 +- tests/tests_tui/test_tui_configs.py | 5 +- tests/tests_tui/test_tui_create_folders.py | 5 +- tests/tests_tui/test_tui_datatypes.py | 5 +- tests/tests_tui/test_tui_directorytree.py | 3 +- tests/tests_tui/test_tui_get_help.py | 3 +- tests/tests_tui/test_tui_logging.py | 3 +- tests/tests_tui/test_tui_settings.py | 3 +- tests/tests_tui/test_tui_transfer.py | 5 +- tests/tests_tui/test_tui_validate.py | 3 +- .../test_tui_widgets_and_defaults.py | 3 +- tests/tests_tui/tui_base.py | 3 +- tests/tests_unit/__init__.py | 0 45 files changed, 157 insertions(+), 182 deletions(-) create mode 100644 tests/__init__.py delete mode 100644 tests/conftest.py delete mode 100644 tests/quick_make_project.py create mode 100644 tests/tests_integration/__init__.py create mode 100644 tests/tests_regression/__init__.py create mode 100644 tests/tests_transfers/__init__.py rename tests/{ => tests_transfers}/file_conflicts_pathtable.py (100%) create mode 100644 tests/tests_transfers/local_filesystem/__init__.py rename tests/tests_transfers/{ => local_filesystem}/test_transfer.py (99%) rename tests/tests_transfers/{ => local_filesystem}/test_transfer_checks.py (98%) rename tests/tests_transfers/{ => local_filesystem}/test_transfer_special_arguments.py (96%) create mode 100644 tests/tests_transfers/ssh/__init__.py create mode 100644 tests/tests_transfers/ssh/base_ssh.py rename tests/tests_transfers/{ => ssh}/ssh_test_images/Dockerfile (100%) rename tests/tests_transfers/{ => ssh}/ssh_test_utils.py (100%) rename tests/tests_transfers/{ => ssh}/test_ssh_setup.py (95%) rename tests/tests_transfers/{ => ssh}/test_ssh_transfer.py (97%) create mode 100644 tests/tests_tui/__init__.py create mode 100644 tests/tests_unit/__init__.py diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 83aea1e9a..ff3448e2d 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -57,17 +57,18 @@ jobs: run: | python -m pip install --upgrade pip pip install .[dev] - - name: Test + - name: Test All # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. + - name: Test SSH (Linux only) + if: runner.os == 'Linux' run: | - if [ "$RUNNER_OS" == "Linux" ]; then - sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 - pytest -k "test_combinations_ssh_transfer or test_ssh_setup" - else - pytest -k "not test_combinations_ssh_transfer and not test_ssh_setup" - fi + sudo service mysql stop # free up port 3306 for ssh tests + pytest tests/tests_transfers/ssh + - name: Test All + run: | + pytest --ignore tests/tests_transfers/ssh build_sdist_wheels: name: Build source distribution diff --git a/pyproject.toml b/pyproject.toml index 9ca0f69e6..58714f173 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -107,6 +107,7 @@ select = ["I", "E", "F", "TCH", "TID252"] [tool.ruff.lint.per-file-ignores] "__init__.py" = ["F401"] +"tests/**/*" = ["TID252"] [tool.ruff.lint.mccabe] max-complexity = 18 diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/base.py b/tests/base.py index 78cf1e6cb..7ce3f3c74 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1,10 +1,11 @@ import warnings import pytest -import test_utils from datashuttle import DataShuttle +from . import test_utils + TEST_PROJECT_NAME = "test_project" diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index 2203b1025..000000000 --- a/tests/conftest.py +++ /dev/null @@ -1,47 +0,0 @@ -""" -Test configs, used for setting up SSH tests. - -Before running these tests, it is necessary to setup -an SSH key. This can be done through datashuttle -ssh.setup_ssh_key(project.cfg, log=False). - -Store this path somewhere outside of the test environment, -and it will be copied to the project test folder before testing. - -FILESYSTEM_PATH and SERVER_PATH these must point -to the same folder on the HPC, filesystem, -as a mounted drive and server as the linux path to -connect through SSH -""" - -import platform -from types import SimpleNamespace - -import pytest -import test_utils - -test_ssh = False -username = "jziminski" -central_host_id = "ssh.swc.ucl.ac.uk" -server_path = r"/ceph/neuroinformatics/neuroinformatics/scratch/datashuttle_tests/fake_data" - - -if platform.system() == "Windows": - ssh_key_path = r"C:\Users\Joe\.datashuttle\test_file_conflicts_ssh_key" - filesystem_path = "X:/neuroinformatics/scratch/datashuttle_tests/fake_data" - -else: - ssh_key_path = "/home/joe/test_file_conflicts_ssh_key" - filesystem_path = "/home/joe/ceph_mount/neuroinformatics/scratch/datashuttle_tests/fake_data" - - -def pytest_configure(config): - pytest.ssh_config = SimpleNamespace( - TEST_SSH=test_ssh, - SSH_KEY_PATH=ssh_key_path, - USERNAME=username, - CENTRAL_HOST_ID=central_host_id, - FILESYSTEM_PATH=filesystem_path, # FILESYSTEM_PATH and SERVER_PATH these must point to the same folder on the HPC, filesystem - SERVER_PATH=server_path, # as a mounted drive and server as the linux path to connect through SSH - ) - test_utils.set_datashuttle_loggers(disable=True) diff --git a/tests/quick_make_project.py b/tests/quick_make_project.py deleted file mode 100644 index 250e66507..000000000 --- a/tests/quick_make_project.py +++ /dev/null @@ -1,5 +0,0 @@ -base_path = r"C:/Users/Joe/work/git-repos/forks/yxtuix/joe" - -from test_utils import quick_create_project - -quick_create_project(base_path) diff --git a/tests/test_utils.py b/tests/test_utils.py index 0e7b45695..1b3f9a9fd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -10,7 +10,6 @@ from pathlib import Path import yaml -from file_conflicts_pathtable import get_pathtable from datashuttle import DataShuttle from datashuttle.configs import canonical_configs, canonical_folders @@ -159,19 +158,6 @@ def make_test_path(base_path, local_or_central, test_project_name): return Path(base_path) / local_or_central / test_project_name -def create_all_pathtable_files(pathtable): - """ """ - for i in range(pathtable.shape[0]): - filepath = pathtable["base_folder"][i] / pathtable["path"][i] - filepath.parents[0].mkdir(parents=True, exist_ok=True) - write_file(filepath, contents="test_entry") - - -def quick_create_project(base_path): - pathtable = get_pathtable(base_path) - create_all_pathtable_files(pathtable) - - # ----------------------------------------------------------------------------- # Test Configs # ----------------------------------------------------------------------------- diff --git a/tests/tests_integration/__init__.py b/tests/tests_integration/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_integration/test_configs.py b/tests/tests_integration/test_configs.py index feff2900d..24e152458 100644 --- a/tests/tests_integration/test_configs.py +++ b/tests/tests_integration/test_configs.py @@ -1,13 +1,14 @@ import os import pytest -import test_utils -from base import BaseTest from datashuttle import DataShuttle from datashuttle.utils import getters from datashuttle.utils.custom_exceptions import ConfigError +from .. import test_utils +from ..base import BaseTest + class TestConfigs(BaseTest): # Test Errors diff --git a/tests/tests_integration/test_create_folders.py b/tests/tests_integration/test_create_folders.py index e50623776..0ca15ab7a 100644 --- a/tests/tests_integration/test_create_folders.py +++ b/tests/tests_integration/test_create_folders.py @@ -5,12 +5,13 @@ from os.path import join import pytest -import test_utils -from base import BaseTest from datashuttle.configs import canonical_configs, canonical_folders from datashuttle.configs.canonical_tags import tags +from .. import test_utils +from ..base import BaseTest + class TestCreateFolders(BaseTest): diff --git a/tests/tests_integration/test_datatypes.py b/tests/tests_integration/test_datatypes.py index 840fc26ee..cf5c6582e 100644 --- a/tests/tests_integration/test_datatypes.py +++ b/tests/tests_integration/test_datatypes.py @@ -1,11 +1,12 @@ import os import pytest -import test_utils -from base import BaseTest from datashuttle.configs import canonical_configs +from .. import test_utils +from ..base import BaseTest + class TestDatatypes(BaseTest): """ diff --git a/tests/tests_integration/test_formatting.py b/tests/tests_integration/test_formatting.py index bf62c6a50..466c4eb97 100644 --- a/tests/tests_integration/test_formatting.py +++ b/tests/tests_integration/test_formatting.py @@ -1,9 +1,10 @@ import pytest -from base import BaseTest from datashuttle.utils import formatting from datashuttle.utils.custom_exceptions import NeuroBlueprintError +from ..base import BaseTest + class TestFormatting(BaseTest): @pytest.mark.parametrize("prefix", ["sub", "ses"]) diff --git a/tests/tests_integration/test_local_only_mode.py b/tests/tests_integration/test_local_only_mode.py index 6df59233b..30bd2736d 100644 --- a/tests/tests_integration/test_local_only_mode.py +++ b/tests/tests_integration/test_local_only_mode.py @@ -1,14 +1,15 @@ import shutil import pytest -import test_utils -from base import BaseTest from datashuttle import DataShuttle from datashuttle.utils.custom_exceptions import ( ConfigError, ) +from .. import test_utils +from ..base import BaseTest + TEST_PROJECT_NAME = "test_project" diff --git a/tests/tests_integration/test_logging.py b/tests/tests_integration/test_logging.py index 3cf2d733e..82e510dfb 100644 --- a/tests/tests_integration/test_logging.py +++ b/tests/tests_integration/test_logging.py @@ -5,7 +5,6 @@ from pathlib import Path import pytest -import test_utils from datashuttle import DataShuttle from datashuttle.configs import canonical_configs @@ -16,6 +15,8 @@ NeuroBlueprintError, ) +from .. import test_utils + class TestLogging: diff --git a/tests/tests_integration/test_settings.py b/tests/tests_integration/test_settings.py index 1f19bede4..0a655bc9d 100644 --- a/tests/tests_integration/test_settings.py +++ b/tests/tests_integration/test_settings.py @@ -2,13 +2,14 @@ import shutil import pytest -from base import BaseTest from datashuttle import DataShuttle from datashuttle.configs import canonical_configs from datashuttle.utils import validation from datashuttle.utils.custom_exceptions import NeuroBlueprintError +from ..base import BaseTest + class TestPersistentSettings(BaseTest): diff --git a/tests/tests_integration/test_validation.py b/tests/tests_integration/test_validation.py index 4ef496887..6b294d5cd 100644 --- a/tests/tests_integration/test_validation.py +++ b/tests/tests_integration/test_validation.py @@ -2,12 +2,13 @@ import shutil import pytest -from base import BaseTest from datashuttle import quick_validate_project from datashuttle.utils import formatting, validation from datashuttle.utils.custom_exceptions import NeuroBlueprintError +from ..base import BaseTest + # ----------------------------------------------------------------------------- # Inconsistent sub or ses value lengths # ----------------------------------------------------------------------------- diff --git a/tests/tests_regression/__init__.py b/tests/tests_regression/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_regression/test_backwards_compatibility.py b/tests/tests_regression/test_backwards_compatibility.py index 7e243992a..9a3514144 100644 --- a/tests/tests_regression/test_backwards_compatibility.py +++ b/tests/tests_regression/test_backwards_compatibility.py @@ -3,10 +3,11 @@ from pathlib import Path import pytest -import test_utils from datashuttle import DataShuttle +from .. import test_utils + TEST_PROJECT_NAME = "test_project" diff --git a/tests/tests_transfers/__init__.py b/tests/tests_transfers/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_transfers/base_transfer.py b/tests/tests_transfers/base_transfer.py index d81056546..32dac6dec 100644 --- a/tests/tests_transfers/base_transfer.py +++ b/tests/tests_transfers/base_transfer.py @@ -1,22 +1,14 @@ """ """ import copy -import os -import platform -import subprocess from pathlib import Path import pandas as pd import pytest -import ssh_test_utils -import test_utils -from base import BaseTest -from file_conflicts_pathtable import get_pathtable -# Choose port 3306 for running on GH actions -# suggested in https://github.com/orgs/community/discussions/25550 -PORT = 3306 -os.environ["DS_SSH_PORT"] = str(PORT) +from .. import test_utils +from ..base import BaseTest +from .file_conflicts_pathtable import get_pathtable class BaseTransfer(BaseTest): @@ -25,59 +17,6 @@ class BaseTransfer(BaseTest): custom transfers with keys (e.g. all_non_sub). """ - @pytest.fixture( - scope="class", - ) - def setup_ssh_container(self): - """ - Set up the Dockerfile container for SSH tests and - delete it on teardown. - """ - container_name = "datashuttle_ssh_tests" - - assert ssh_test_utils.docker_is_running(), ( - "docker is not running, " - "this should be checked at the top of test script" - ) - - image_path = Path(__file__).parent / "ssh_test_images" - os.chdir(image_path) - - if platform.system() != "Windows": - build_command = "sudo docker build -t ssh_server ." - run_command = ( - f"sudo docker run -d -p {PORT}:22 " - f"--name {container_name} ssh_server" - ) - else: - build_command = "docker build -t ssh_server ." - run_command = f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" - - build_output = subprocess.run( - build_command, - shell=True, - capture_output=True, - ) - assert build_output.returncode == 0, ( - f"docker build failed with: STDOUT-{build_output.stdout} " - f"STDERR-{build_output.stderr}" - ) - - run_output = subprocess.run( - run_command, - shell=True, - capture_output=True, - ) - - assert run_output.returncode == 0, ( - f"docker run failed with: STDOUT-{run_output.stdout} " - f"STDERR-{run_output.stderr}" - ) - - yield - - subprocess.run(f"docker rm -f {container_name}", shell=True) - @pytest.fixture( scope="class", ) diff --git a/tests/file_conflicts_pathtable.py b/tests/tests_transfers/file_conflicts_pathtable.py similarity index 100% rename from tests/file_conflicts_pathtable.py rename to tests/tests_transfers/file_conflicts_pathtable.py diff --git a/tests/tests_transfers/local_filesystem/__init__.py b/tests/tests_transfers/local_filesystem/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_transfers/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py similarity index 99% rename from tests/tests_transfers/test_transfer.py rename to tests/tests_transfers/local_filesystem/test_transfer.py index 8cbff488f..069ce3469 100644 --- a/tests/tests_transfers/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -4,13 +4,14 @@ from pathlib import Path import pytest -import test_utils -from base import BaseTest from datashuttle.configs import canonical_folders from datashuttle.configs.canonical_configs import get_broad_datatypes from datashuttle.configs.canonical_tags import tags +from ... import test_utils +from ...base import BaseTest + class TestFileTransfer(BaseTest): diff --git a/tests/tests_transfers/test_transfer_checks.py b/tests/tests_transfers/local_filesystem/test_transfer_checks.py similarity index 98% rename from tests/tests_transfers/test_transfer_checks.py rename to tests/tests_transfers/local_filesystem/test_transfer_checks.py index 342ed6407..2f5c909de 100644 --- a/tests/tests_transfers/test_transfer_checks.py +++ b/tests/tests_transfers/local_filesystem/test_transfer_checks.py @@ -3,11 +3,12 @@ from pathlib import Path import pytest -import test_utils -from base import BaseTest from datashuttle.utils.rclone import get_local_and_central_file_differences +from ... import test_utils +from ...base import BaseTest + class TestTransferChecks(BaseTest): @pytest.mark.parametrize( diff --git a/tests/tests_transfers/test_transfer_special_arguments.py b/tests/tests_transfers/local_filesystem/test_transfer_special_arguments.py similarity index 96% rename from tests/tests_transfers/test_transfer_special_arguments.py rename to tests/tests_transfers/local_filesystem/test_transfer_special_arguments.py index 6458b3d4c..03e9295f9 100644 --- a/tests/tests_transfers/test_transfer_special_arguments.py +++ b/tests/tests_transfers/local_filesystem/test_transfer_special_arguments.py @@ -4,11 +4,9 @@ from pathlib import Path import pytest -import ssh_test_utils -import test_utils -from base_transfer import BaseTransfer -TEST_SSH = ssh_test_utils.get_test_ssh() +from ... import test_utils +from ..base_transfer import BaseTransfer PARAM_SUBS = [ ["all"], diff --git a/tests/tests_transfers/ssh/__init__.py b/tests/tests_transfers/ssh/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_transfers/ssh/base_ssh.py b/tests/tests_transfers/ssh/base_ssh.py new file mode 100644 index 000000000..0a8c52b97 --- /dev/null +++ b/tests/tests_transfers/ssh/base_ssh.py @@ -0,0 +1,76 @@ +""" """ + +import os +import platform +import subprocess +from pathlib import Path + +import pytest + +from ..base_transfer import BaseTransfer +from . import ssh_test_utils + +# Choose port 3306 for running on GH actions +# suggested in https://github.com/orgs/community/discussions/25550 +PORT = 3306 +os.environ["DS_SSH_PORT"] = str(PORT) + + +class BaseSSHTransfer(BaseTransfer): + """ + Class holding fixtures and methods for testing the + custom transfers with keys (e.g. all_non_sub). + """ + + @pytest.fixture( + scope="class", + ) + def setup_ssh_container(self): + """ + Set up the Dockerfile container for SSH tests and + delete it on teardown. + """ + container_name = "datashuttle_ssh_tests" + + assert ssh_test_utils.docker_is_running(), ( + "docker is not running, " + "this should be checked at the top of test script" + ) + + image_path = Path(__file__).parent / "ssh_test_images" + os.chdir(image_path) + + if platform.system() != "Windows": + build_command = "sudo docker build -t ssh_server ." + run_command = ( + f"sudo docker run -d -p {PORT}:22 " + f"--name {container_name} ssh_server" + ) + else: + build_command = "docker build -t ssh_server ." + run_command = f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" + + build_output = subprocess.run( + build_command, + shell=True, + capture_output=True, + ) + assert build_output.returncode == 0, ( + f"docker build failed with: STDOUT-{build_output.stdout} " + f"STDERR-{build_output.stderr}" + ) + + run_output = subprocess.run( + run_command, + shell=True, + capture_output=True, + ) + + assert run_output.returncode == 0, ( + f"docker run failed with: STDOUT-{run_output.stdout} " + f"STDERR-{run_output.stderr}" + ) + + yield + + subprocess.run(f"docker rm -f {container_name}", shell=True) diff --git a/tests/tests_transfers/ssh_test_images/Dockerfile b/tests/tests_transfers/ssh/ssh_test_images/Dockerfile similarity index 100% rename from tests/tests_transfers/ssh_test_images/Dockerfile rename to tests/tests_transfers/ssh/ssh_test_images/Dockerfile diff --git a/tests/tests_transfers/ssh_test_utils.py b/tests/tests_transfers/ssh/ssh_test_utils.py similarity index 100% rename from tests/tests_transfers/ssh_test_utils.py rename to tests/tests_transfers/ssh/ssh_test_utils.py diff --git a/tests/tests_transfers/test_ssh_setup.py b/tests/tests_transfers/ssh/test_ssh_setup.py similarity index 95% rename from tests/tests_transfers/test_ssh_setup.py rename to tests/tests_transfers/ssh/test_ssh_setup.py index 2835af819..3d8239697 100644 --- a/tests/tests_transfers/test_ssh_setup.py +++ b/tests/tests_transfers/ssh/test_ssh_setup.py @@ -3,12 +3,13 @@ import platform import pytest -import ssh_test_utils -import test_utils -from base_transfer import BaseTransfer from datashuttle.utils import ssh +from ... import test_utils +from . import ssh_test_utils +from .base_ssh import BaseSSHTransfer + TEST_SSH = ssh_test_utils.get_test_ssh() @@ -16,7 +17,7 @@ platform.system == "Darwin", reason="Docker set up is not robust on macOS." ) @pytest.mark.skipif(not TEST_SSH, reason="TEST_SSH is false") -class TestSSH(BaseTransfer): +class TestSSH(BaseSSHTransfer): @pytest.fixture(scope="function") def project(test, tmp_path, setup_ssh_container): diff --git a/tests/tests_transfers/test_ssh_transfer.py b/tests/tests_transfers/ssh/test_ssh_transfer.py similarity index 97% rename from tests/tests_transfers/test_ssh_transfer.py rename to tests/tests_transfers/ssh/test_ssh_transfer.py index e5a1b175e..60ce29c27 100644 --- a/tests/tests_transfers/test_ssh_transfer.py +++ b/tests/tests_transfers/ssh/test_ssh_transfer.py @@ -3,11 +3,12 @@ import paramiko import pytest -import ssh_test_utils -from base_transfer import BaseTransfer from datashuttle.utils import ssh +from . import ssh_test_utils +from .base_ssh import BaseSSHTransfer + TEST_SSH = ssh_test_utils.get_test_ssh() @@ -15,7 +16,7 @@ platform.system == "Darwin", reason="Docker set up is not robust on macOS." ) @pytest.mark.skipif(not TEST_SSH, reason="TEST_SSH is false") -class TestSSHTransfer(BaseTransfer): +class TestSSHTransfer(BaseSSHTransfer): @pytest.fixture( scope="class", diff --git a/tests/tests_tui/__init__.py b/tests/tests_tui/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/tests_tui/test_local_only_project.py b/tests/tests_tui/test_local_only_project.py index 0599ea144..c3e873851 100644 --- a/tests/tests_tui/test_local_only_project.py +++ b/tests/tests_tui/test_local_only_project.py @@ -1,8 +1,9 @@ import pytest -from tui_base import TuiBase from datashuttle.tui.app import TuiApp +from .tui_base import TuiBase + class TestTuiLocalOnlyProject(TuiBase): diff --git a/tests/tests_tui/test_tui_configs.py b/tests/tests_tui/test_tui_configs.py index e8f5d0767..fc332078d 100644 --- a/tests/tests_tui/test_tui_configs.py +++ b/tests/tests_tui/test_tui_configs.py @@ -3,8 +3,6 @@ from time import monotonic import pytest -import test_utils -from tui_base import TuiBase from datashuttle.configs import load_configs from datashuttle.tui.app import TuiApp @@ -13,6 +11,9 @@ ) from datashuttle.tui.screens.project_manager import ProjectManagerScreen +from .. import test_utils +from .tui_base import TuiBase + class TestTuiConfigs(TuiBase): diff --git a/tests/tests_tui/test_tui_create_folders.py b/tests/tests_tui/test_tui_create_folders.py index 85a06cd5c..9e4df32c8 100644 --- a/tests/tests_tui/test_tui_create_folders.py +++ b/tests/tests_tui/test_tui_create_folders.py @@ -1,8 +1,6 @@ import re import pytest -import test_utils -from tui_base import TuiBase from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp @@ -11,6 +9,9 @@ ) from datashuttle.tui.screens.project_manager import ProjectManagerScreen +from .. import test_utils +from .tui_base import TuiBase + class TestTuiCreateFolders(TuiBase): diff --git a/tests/tests_tui/test_tui_datatypes.py b/tests/tests_tui/test_tui_datatypes.py index 0e9ec8a64..bc72bea95 100644 --- a/tests/tests_tui/test_tui_datatypes.py +++ b/tests/tests_tui/test_tui_datatypes.py @@ -1,10 +1,11 @@ import pytest -import test_utils -from tui_base import TuiBase from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp +from .. import test_utils +from .tui_base import TuiBase + class TestDatatypesTUI(TuiBase): """ diff --git a/tests/tests_tui/test_tui_directorytree.py b/tests/tests_tui/test_tui_directorytree.py index 1a6e03d8b..81493810d 100644 --- a/tests/tests_tui/test_tui_directorytree.py +++ b/tests/tests_tui/test_tui_directorytree.py @@ -2,10 +2,11 @@ import pyperclip import pytest -from tui_base import TuiBase from datashuttle.tui.app import TuiApp +from .tui_base import TuiBase + try: pyperclip.paste() HAS_GUI = True diff --git a/tests/tests_tui/test_tui_get_help.py b/tests/tests_tui/test_tui_get_help.py index 55d876f35..18ae5354f 100644 --- a/tests/tests_tui/test_tui_get_help.py +++ b/tests/tests_tui/test_tui_get_help.py @@ -1,8 +1,9 @@ import pytest -from tui_base import TuiBase from datashuttle.tui.app import TuiApp +from .tui_base import TuiBase + class TestTuiSettings(TuiBase): """ diff --git a/tests/tests_tui/test_tui_logging.py b/tests/tests_tui/test_tui_logging.py index cb4b8417f..fe2cdad15 100644 --- a/tests/tests_tui/test_tui_logging.py +++ b/tests/tests_tui/test_tui_logging.py @@ -1,10 +1,11 @@ import pytest -from tui_base import TuiBase from datashuttle import DataShuttle from datashuttle.tui.app import TuiApp from datashuttle.tui.tabs.logging import RichLogScreen +from .tui_base import TuiBase + class TestTuiLogging(TuiBase): diff --git a/tests/tests_tui/test_tui_settings.py b/tests/tests_tui/test_tui_settings.py index 77a65b090..98d95c024 100644 --- a/tests/tests_tui/test_tui_settings.py +++ b/tests/tests_tui/test_tui_settings.py @@ -1,8 +1,9 @@ import pytest -from tui_base import TuiBase from datashuttle.tui.app import TuiApp +from .tui_base import TuiBase + class TestTuiSettings(TuiBase): """ diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 3e8c389f1..925f457b2 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -1,10 +1,11 @@ import pytest -import test_utils -from tui_base import TuiBase from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp +from .. import test_utils +from .tui_base import TuiBase + class TestTuiTransfer(TuiBase): """ diff --git a/tests/tests_tui/test_tui_validate.py b/tests/tests_tui/test_tui_validate.py index 0972c9756..fa07c1c39 100644 --- a/tests/tests_tui/test_tui_validate.py +++ b/tests/tests_tui/test_tui_validate.py @@ -1,10 +1,11 @@ import pytest import textual -from tui_base import TuiBase import datashuttle from datashuttle.tui.app import TuiApp +from .tui_base import TuiBase + class TestTuiValidate(TuiBase): diff --git a/tests/tests_tui/test_tui_widgets_and_defaults.py b/tests/tests_tui/test_tui_widgets_and_defaults.py index 73f06a22a..5651cef8c 100644 --- a/tests/tests_tui/test_tui_widgets_and_defaults.py +++ b/tests/tests_tui/test_tui_widgets_and_defaults.py @@ -2,7 +2,6 @@ from typing import Union import pytest -from tui_base import TuiBase from datashuttle import DataShuttle from datashuttle.configs import canonical_configs @@ -12,6 +11,8 @@ ) from datashuttle.tui.screens.new_project import NewProjectScreen +from .tui_base import TuiBase + class TestTuiWidgets(TuiBase): """ diff --git a/tests/tests_tui/tui_base.py b/tests/tests_tui/tui_base.py index 510b943b2..6b5df9ba9 100644 --- a/tests/tests_tui/tui_base.py +++ b/tests/tests_tui/tui_base.py @@ -1,11 +1,12 @@ import pytest_asyncio -import test_utils from textual.widgets._tabbed_content import ContentTab from datashuttle.configs import canonical_configs from datashuttle.tui.screens.project_manager import ProjectManagerScreen from datashuttle.tui.screens.project_selector import ProjectSelectorScreen +from .. import test_utils + class TuiBase: """ diff --git a/tests/tests_unit/__init__.py b/tests/tests_unit/__init__.py new file mode 100644 index 000000000..e69de29bb From 76b5e80b81074d2600d7a0dbe46d6fb942c29bce Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 17:59:52 +0100 Subject: [PATCH 56/57] Update CI. --- .github/workflows/code_test_and_deploy.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index ff3448e2d..2fee3d260 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -32,7 +32,7 @@ jobs: # macos-14 is M1, macos-13 is intel. Run on earliest and # latest python versions. All python versions are tested in # the weekly cron job. - os: [ ubuntu-latest] # [windows-latest, macos-14, macos-13] TODO: ADD BACK + os: [ ubuntu-latest, windows-latest, macos-14, macos-13] # Test all Python versions for cron job, and only first/last for other triggers python-version: ${{ fromJson(github.event_name == 'schedule' && '["3.9", "3.10", "3.11", "3.12"]' || '["3.9", "3.12"]') }} @@ -57,7 +57,6 @@ jobs: run: | python -m pip install --upgrade pip pip install .[dev] - - name: Test All # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. @@ -66,7 +65,7 @@ jobs: run: | sudo service mysql stop # free up port 3306 for ssh tests pytest tests/tests_transfers/ssh - - name: Test All + - name: All Other Tests run: | pytest --ignore tests/tests_transfers/ssh From ee03d3a4536649c045442e83798a7e8e55432cd7 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Sat, 21 Jun 2025 23:14:16 +0100 Subject: [PATCH 57/57] Playing around with using rclone for all file searches. --- datashuttle/configs/config_class.py | 3 + datashuttle/utils/folders.py | 133 ++++++++++++++++++++++++---- 2 files changed, 121 insertions(+), 15 deletions(-) diff --git a/datashuttle/configs/config_class.py b/datashuttle/configs/config_class.py index 562c3310b..64e8364ec 100644 --- a/datashuttle/configs/config_class.py +++ b/datashuttle/configs/config_class.py @@ -200,6 +200,9 @@ def get_rclone_config_name( return f"central_{self.project_name}_{connection_method}" + def get_rclone_config_name_local(self): + return f"local_{self.project_name}_local_filesystem" + def make_rclone_transfer_options( self, overwrite_existing_files: OverwriteExistingFiles, dry_run: bool ) -> Dict: diff --git a/datashuttle/utils/folders.py b/datashuttle/utils/folders.py index 56852640c..f28f519ad 100644 --- a/datashuttle/utils/folders.py +++ b/datashuttle/utils/folders.py @@ -515,25 +515,66 @@ def search_for_folders( verbose : If `True`, when a search folder cannot be found, a message will be printed with the missing path. """ - if local_or_central == "central" and cfg["connection_method"] == "ssh": - all_folder_names, all_filenames = ssh.search_ssh_central_for_folders( - search_path, - search_prefix, - cfg, - verbose, - return_full_path, + if local_or_central == "local": + all_folder_names, all_filenames = search_gdrive_or_aws_for_folders( + search_path, search_prefix, None, return_full_path ) - else: - if not search_path.exists(): - if verbose: - utils.log_and_message( - f"No file found at {search_path.as_posix()}" - ) - return [], [] - all_folder_names, all_filenames = search_filesystem_path_for_folders( + all_folder_names_, all_filenames_ = search_filesystem_path_for_folders( search_path / search_prefix, return_full_path ) + + assert all_folder_names == all_folder_names_ + assert all_filenames == all_filenames_ + + else: + + if cfg["connection_method"] == "ssh": + all_folder_names, all_filenames = ( + ssh.search_ssh_central_for_folders( + search_path, + search_prefix, + cfg, + verbose, + return_full_path, + ) + ) + + all_folder_names_, all_filenames_ = ( + search_gdrive_or_aws_for_folders( + search_path, + search_prefix, + cfg.get_rclone_config_name("ssh"), + return_full_path, + ) + ) + assert sorted(all_folder_names) == all_folder_names_ + assert all_filenames == all_filenames_ + + else: + if not search_path.exists(): + if verbose: + utils.log_and_message( + f"No file found at {search_path.as_posix()}" + ) + return [], [] + + all_folder_names, all_filenames = search_gdrive_or_aws_for_folders( + search_path, + search_prefix, + cfg.get_rclone_config_name("local_filesystem"), + return_full_path, + ) + + all_folder_names_, all_filenames_ = ( + search_filesystem_path_for_folders( + search_path / search_prefix, return_full_path + ) + ) + + assert all_folder_names == all_folder_names_ + assert all_filenames == all_filenames_ + return all_folder_names, all_filenames @@ -565,3 +606,65 @@ def search_filesystem_path_for_folders( ) return all_folder_names, all_filenames + + +def search_gdrive_or_aws_for_folders( + search_path: Path, + search_prefix: str, + rclone_config_name: str | None, + return_full_path: bool = False, +) -> Tuple[List[Any], List[Any]]: + """ + Searches for files and folders in central path using `rclone lsjson` command. + This command lists all the files and folders in the central path in a json format. + The json contains file/folder info about each file/folder like name, type, etc. + """ + import fnmatch + import json + + from datashuttle.utils import rclone + + if rclone_config_name: + config_prefix = f"{rclone_config_name}:" + else: + config_prefix = "" + + output = rclone.call_rclone( + f'lsjson {config_prefix}"{search_path.as_posix()}"', + pipe_std=True, + ) + + all_folder_names: List[str] = [] + all_filenames: List[str] = [] + + if output.returncode != 0: + utils.log_and_message( + f"Error searching files at {search_path.as_posix()} \n {output.stderr.decode('utf-8') if output.stderr else ''}" + ) + return all_folder_names, all_filenames + + files_and_folders = json.loads(output.stdout) + + # try: + for file_or_folder in files_and_folders: + + name = file_or_folder["Name"] + + if not fnmatch.fnmatch(name, search_prefix): + continue + + is_dir = file_or_folder.get("IsDir", False) + + to_append = search_path / name if return_full_path else name + + if is_dir: + all_folder_names.append(to_append) + else: + all_filenames.append(to_append) + + # except Exception: + # utils.log_and_message( + # f"Error searching files at {search_path.as_posix()}" + # ) + + return all_folder_names, all_filenames