From ad9006bdb8cb571ece3a586ae00f3b4c1c9809d2 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:06:04 +0200 Subject: [PATCH 01/33] Create main.yml --- .github/workflows/main.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/main.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml new file mode 100644 index 0000000..f3f44e1 --- /dev/null +++ b/.github/workflows/main.yml @@ -0,0 +1,30 @@ +name: Pytest + +on: + push: + branches: + - main + pull_request: + branches: + - main + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + + - name: Run Pytest + run: | + pytest From 6a8917c7c274f366fc4e20dd7f9d9fdde2f14170 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Mon, 8 Apr 2024 16:15:13 +0200 Subject: [PATCH 02/33] Commit to launch actions --- .gitignore | 1 - tests/test_VipLauncher.py | 285 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 tests/test_VipLauncher.py diff --git a/.gitignore b/.gitignore index 81532b3..45e2d8e 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,3 @@ examples/old publish.sh dist/ -tests/* diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py new file mode 100644 index 0000000..1be5dcd --- /dev/null +++ b/tests/test_VipLauncher.py @@ -0,0 +1,285 @@ +import io +import pytest +import unittest +from pathlib import * + +import pytest_mock +from src.vip_client.utils import vip +from src.vip_client.classes import VipLauncher + +def get_properties(obj) -> dict: + """ + Get session properties as they should be returned by the getter functions + """ + # Function to parse a single element + def get_element(element): + if isinstance(element, dict): + return {key: get_element(value) for key, value in element.items()} + elif isinstance(element, list): + return [get_element(value) for value in element] + elif element is None: + return None + else: + return str(element) + # Return + return {prop: get_element(value) for prop, value in obj.input_properties.items()} + +@pytest.fixture(scope="function", autouse=True) +def setup_teardown_vip_launcher(request, mocker): + # Create a buffer file for the backup + with open('tmp_data.json', 'w') as f: + f.write('{}') + # Mock the VIP API + mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") + mocked_list_pipeline.return_value = [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") + mocked_set_api_key.side_effect = fake_set_api_key + + # Setup code before running the tests in the class + print("Handshake with VIP") + VipLauncher.init(api_key="FAKE_KEY") + print("Setup done") + +@pytest.fixture(scope="function", autouse=True) +def cleanup(): + # Teardown code after running each test function + yield + # Remove the buffer file + try: + Path('tmp_data.json').unlink() + except FileNotFoundError: + pass + + +@pytest.mark.parametrize( + "nb_runs, pipeline_id", + [ + (1, "LCModel/0.1"), + (2, "CQUEST/0.3"), + (3, "LCModel/0.1") + ] +) +def test_run_and_finish(mocker, nb_runs, pipeline_id): + + removed = False + + def fake_exists(path): + if path == '/vip/Home/test-VipLauncher/OUTPUTS': + return True + if path == 'fake_value' and not removed: + return True + return False + + def fake_pipeline_def(pipeline): + return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + return 'workflow-XXXXXX' + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + def fake_delete_path(path): + nonlocal removed + removed = True + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") + mocked_pipeline_def.side_effect = fake_pipeline_def + + mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") + mocked_init_exec.side_effect = fake_init_exec + + mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") + mocked_execution_info.side_effect = fake_execution_info + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + # Launch a Full Session Run + s = VipLauncher() + s.pipeline_id = pipeline_id + s.output_dir = PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS") + s.input_settings = { + "zipped_folder": 'fake_value', + "basis_file": 'fake_value', + "signal_file": ['fake_value', 'fake_value'], + "control_file": ['fake_value'] + } + s.run_session(nb_runs=nb_runs) + # Check the Results + assert s.workflows + assert len(s.workflows) == 1 + for wid in s.workflows: + assert s.workflows[wid]["status"] == "Finished" + assert s.pipeline_id == pipeline_id + # Finish the Session + s.finish(timeout=50) + # Check Deletion + assert removed + for wid in s.workflows: + assert s.workflows[wid]["status"] == "Removed" + +@pytest.mark.parametrize( + "backup_location, input_settings, pipeline_id, output_dir", + [ + ('vip', { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ), + (None, { + "zipped_folder": None, + "basis_file": None, + "signal_file": None, + "control_file": None + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ), + ('vip', { + "zipped_folder": 'different_value1', + "basis_file": 'different_value2', + "signal_file": ['different_value3', 'different_value4'], + "control_file": ['different_value5'] + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ) + ] +) +def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): + + def fake_exists(path): + return True + + def fake_pathlib_exists(): + return True + + def fake_delete_path(path): + return True + + def fake_upload(local_path, vip_path): + return True + + def fake_download(vip_path, local_path): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + print("EENTER") + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") + mocked_pathlib_exists.side_effect = fake_pathlib_exists + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_upload = mocker.patch("vip_client.utils.vip.upload") + mocked_upload.side_effect = fake_upload + + mocked_download_file = mocker.patch("vip_client.utils.vip.download") + mocked_download_file.side_effect = fake_download + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + + VipLauncher._BACKUP_LOCATION = backup_location + # Return if backup is disabled + if VipLauncher._BACKUP_LOCATION is None: + return + # Create session + s1 = VipLauncher() + s1.input_settings = input_settings + s1.pipeline_id = pipeline_id + s1.output_dir = output_dir + # Backup + s1._save() + # Load backup + s2 = VipLauncher(output_dir=s1.output_dir) + # Check parameters + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id + assert s2.output_dir == s1.output_dir + assert s2.workflows == s1.workflows + + +def test_properties_interface(mocker): + + def fake_exists(path): + return True + + def fake_pathlib_exists(): + return True + + def fake_delete_path(path): + return True + + def fake_upload(local_path, vip_path): + return True + + def fake_download(vip_path, local_path): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + print("EENTER") + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") + mocked_pathlib_exists.side_effect = fake_pathlib_exists + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_upload = mocker.patch("vip_client.utils.vip.upload") + mocked_upload.side_effect = fake_upload + + mocked_download_file = mocker.patch("vip_client.utils.vip.download") + mocked_download_file.side_effect = fake_download + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + + VipLauncher._BACKUP_LOCATION = "vip" + + # Copy the first session + s = VipLauncher(output_dir=PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS")) + # Backup the inputs + backup = s.input_settings + # Run a subtest for each property + for prop in s.input_settings: + setattr(s, prop, None) # Calls deleter + assert getattr(s, prop) is None # Public attribute must be None + assert not s._is_defined("_" + prop) # Private attribute must be unset + setattr(s, prop, backup[prop]) # Reset + # Test correct reset + for key, value in s.input_settings.items(): + assert getattr(s, key) == value From 018031643304280300d095b600aba608738788a2 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:16:37 +0200 Subject: [PATCH 03/33] Update main.yml --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f3f44e1..19edd89 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -4,6 +4,7 @@ on: push: branches: - main + - develop pull_request: branches: - main From 3da3ee6b2ffcd0de97e258435415b8bb862c4314 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:19:06 +0200 Subject: [PATCH 04/33] Update main.yml --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 19edd89..f57c405 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -25,6 +25,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip + pip install pytest - name: Run Pytest run: | From 9c67b82ce5559d7a468934051cb26b07d67c5a54 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:20:42 +0200 Subject: [PATCH 05/33] Update main.yml --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f57c405..d87ef2f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -25,7 +25,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install pytest + pip install pytest pytest_mock - name: Run Pytest run: | From 9294eae9fd18f3aec01398428e3dfeb3ff0538e8 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:26:18 +0200 Subject: [PATCH 06/33] Update main.yml --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d87ef2f..deec159 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,4 +29,5 @@ jobs: - name: Run Pytest run: | + pwd pytest From 927d802b6d6361f8c7637d7d5912b3fc8eba759f Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:27:20 +0200 Subject: [PATCH 07/33] Update main.yml --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index deec159..0cb9669 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,5 +29,5 @@ jobs: - name: Run Pytest run: | - pwd + ls pytest From bf4ccfc149f4d027eb42a165e82ad7039288ee8c Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:32:15 +0200 Subject: [PATCH 08/33] Update main.yml --- .github/workflows/main.yml | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0cb9669..8a268dd 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,5 +29,36 @@ jobs: - name: Run Pytest run: | - ls + name: Pytest + +on: + push: + branches: + - main + - develop + pull_request: + branches: + - main + +jobs: + test: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install pytest pytest_mock + + - name: Run Pytest + run: | + export PYTHONPATH=src:$PYTHONPATH pytest From c2062f9b69039a94bbfef4b801b652836a88a0b3 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:33:48 +0200 Subject: [PATCH 09/33] Update main.yml --- .github/workflows/main.yml | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8a268dd..8e4497e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,36 +1,5 @@ name: Pytest -on: - push: - branches: - - main - - develop - pull_request: - branches: - - main - -jobs: - test: - runs-on: ubuntu-latest - - steps: - - name: Checkout repository - uses: actions/checkout@v2 - - - name: Set up Python 3.9 - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install pytest pytest_mock - - - name: Run Pytest - run: | - name: Pytest - on: push: branches: From abeb7d957d8d0987153fe9127318781aa02facef Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:34:49 +0200 Subject: [PATCH 10/33] Update main.yml --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8e4497e..c7ed2b0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -25,7 +25,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install pytest pytest_mock + pip install pytest pytest_mock requests - name: Run Pytest run: | From 0ac9f67af2e3198619018e90d356739848fbbeaa Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Mon, 8 Apr 2024 16:44:32 +0200 Subject: [PATCH 11/33] Commit to launch actions --- tests/test_VipLauncher.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index 1be5dcd..a73e356 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -29,6 +29,7 @@ def setup_teardown_vip_launcher(request, mocker): # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: f.write('{}') + print("Trying to create the buffer file") # Mock the VIP API mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") mocked_list_pipeline.return_value = [ From 35c6c79df7e8ad39348f50648141ad9d51dd0ecb Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Mon, 8 Apr 2024 16:49:41 +0200 Subject: [PATCH 12/33] Commit the tmp file to test if it works --- tmp_data.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tmp_data.json diff --git a/tmp_data.json b/tmp_data.json new file mode 100644 index 0000000..e69de29 From cb2a85e1a24cad7713793cb2911cb67fcd64c27c Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Mon, 8 Apr 2024 17:16:39 +0200 Subject: [PATCH 13/33] New patch for unlink function (linux) --- tests/test_VipLauncher.py | 11 +++++++++++ tmp_data.json | 0 2 files changed, 11 insertions(+) delete mode 100644 tmp_data.json diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index a73e356..33bf883 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -247,6 +247,9 @@ def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newlin def fake_unlink(self): return True + + def fake_unlink_linux(): + return True mocked_exists = mocker.patch("vip_client.utils.vip.exists") mocked_exists.side_effect = fake_exists @@ -268,11 +271,19 @@ def fake_unlink(self): mocked_unlink = mocker.patch("os.unlink") mocked_unlink.side_effect = fake_unlink + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux VipLauncher._BACKUP_LOCATION = "vip" # Copy the first session s = VipLauncher(output_dir=PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS")) + s.input_settings = { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + } # Backup the inputs backup = s.input_settings # Run a subtest for each property diff --git a/tmp_data.json b/tmp_data.json deleted file mode 100644 index e69de29..0000000 From 41c08cc1f127631c8b8c6a15f388533d2d202b97 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Mon, 8 Apr 2024 17:21:36 +0200 Subject: [PATCH 14/33] New patch for unlink function (linux) --- tests/test_VipLauncher.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index 33bf883..36a330e 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -182,6 +182,9 @@ def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newlin def fake_unlink(self): return True + + def fake_unlink_linux(): + return True mocked_exists = mocker.patch("vip_client.utils.vip.exists") mocked_exists.side_effect = fake_exists @@ -203,6 +206,8 @@ def fake_unlink(self): mocked_unlink = mocker.patch("os.unlink") mocked_unlink.side_effect = fake_unlink + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux VipLauncher._BACKUP_LOCATION = backup_location # Return if backup is disabled From d812f9964f3d57aaf9ac58a98f09a9d3086b2b35 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Tue, 9 Apr 2024 16:19:20 +0200 Subject: [PATCH 15/33] First tests for VipCI --- src/vip_client/classes/VipCI.py | 2 + src/vip_client/classes/VipLauncher.py | 4 +- src/vip_client/utils/vip.py | 1 + tests/test_VipCI.py | 436 ++++++++++++++++++++++++++ 4 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 tests/test_VipCI.py diff --git a/src/vip_client/classes/VipCI.py b/src/vip_client/classes/VipCI.py index b8ad6e4..036ae9b 100644 --- a/src/vip_client/classes/VipCI.py +++ b/src/vip_client/classes/VipCI.py @@ -434,6 +434,8 @@ def _load_session(self, location="girder") -> dict: try: girder_id, _ = self._girder_path_to_id(self.vip_output_dir) folder = self._girder_client.getFolder(folderId=girder_id) + print("CASCA") + print(folder) except girder_client.HttpError as e: if e.status == 400: # Folder was not found return None diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index aa94eb2..2cf7407 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -217,7 +217,7 @@ def output_dir(self) -> str: @output_dir.setter def output_dir(self, new_dir: str) -> None: # Call deleter if agument is None - if new_dir is None: + if new_dir is None: del self.vip_output_dir return # Display @@ -1280,6 +1280,8 @@ def _load(self) -> bool: self._set(**backup_data) return True # If the backup data do not have the right properties, raise TypeError + print("SESSION DATA", session_data) + print("BACKUP DATA", backup_data) missing_props = set(session_data.keys()) - set(backup_data.keys()) if missing_props: raise TypeError(f"The following properties are missing in the backup data:\n\t{', '.join(missing_props)}") diff --git a/src/vip_client/utils/vip.py b/src/vip_client/utils/vip.py index d590ca7..a4b903a 100644 --- a/src/vip_client/utils/vip.py +++ b/src/vip_client/utils/vip.py @@ -80,6 +80,7 @@ def setApiKey(value) -> bool: Return True is correct apikey, False otherwise. Raise an error if an other problems occured """ + print("Were in the real one") url = __PREFIX + 'plateform' head_test = { 'apikey': value, diff --git a/tests/test_VipCI.py b/tests/test_VipCI.py new file mode 100644 index 0000000..394c0af --- /dev/null +++ b/tests/test_VipCI.py @@ -0,0 +1,436 @@ +import io +import pytest +from pathlib import * + +import pytest_mock +from src.vip_client.utils import vip +from src.vip_client.classes import VipCI + + +pipeline_id_global = "LCModel/0.1" + +def get_properties(obj) -> dict: + """ + Get session properties as they should be returned by the getter functions + """ + # Function to parse a single element + def get_element(element): + if isinstance(element, dict): + return {key: get_element(value) for key, value in element.items()} + elif isinstance(element, list): + return [get_element(value) for value in element] + elif element is None: + return None + else: + return str(element) + # Return + return {prop: get_element(value) for prop, value in obj.input_properties.items()} + +@pytest.fixture(scope="function", autouse=True) +def setup_teardown_vip_launcher(request, mocker): + + class FakeGirderClient(): + def __init__(self, apiUrl): + pass + + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + print("CASCA") + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(self, folderId): + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": pipeline_id_global, + 'session_name': 'test-VipLauncher', + 'workflows': { + # "workflow-fe5pXw": { + # "output_path": "/collection/ReproVIPSpectro/results/atest_recup_session/2023-11-14_15:27:07", + # "start": "2023/11/14 15:27:13", + # "status": "Finished"} + }, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + mocker.patch("girder_client.GirderClient", FakeGirderClient) + + # Create a buffer file for the backup + with open('tmp_data.json', 'w') as f: + f.write('{}') + print("Trying to create the buffer file") + # Mock the VIP API + mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") + mocked_list_pipeline.return_value = [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_vip_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + def fake_set_girder_api_key(apiKey): + return True if apiKey == "FAKE_KEY" else False + + + mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") + mocked_set_api_key.side_effect = fake_set_vip_api_key + + mocked_set_girder_api_key = mocker.patch("girder_client.GirderClient.authenticate") + mocked_set_girder_api_key.side_effect = fake_set_girder_api_key + + # Setup code before running the tests in the class + print("Handshake with VIP") + VipCI.init(vip_key="FAKE_KEY", girder_key="FAKE_KEY") + print("Setup done") + +@pytest.fixture(scope="function", autouse=True) +def cleanup(): + # Teardown code after running each test function + yield + # Remove the buffer file + try: + Path('tmp_data.json').unlink() + except FileNotFoundError: + pass + + +@pytest.mark.parametrize( + "nb_runs, pipeline_id", + [ + (1, "LCModel/0.1"), + (2, "CQUEST/0.3"), + (3, "LCModel/0.1") + ] +) +def test_run_and_finish(mocker, nb_runs, pipeline_id): + + removed = False + global pipeline_id_global # Use to dynamically change the pipeline_id of the mocked GirderClient + pipeline_id_global = pipeline_id + + class FakeGirderClient(): + def __init__(self, apiUrl): + pass + + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + print("CASCA") + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(self, folderId): + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": pipeline_id, + 'session_name': 'test-VipLauncher', + 'workflows': {}, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + mocker.patch("girder_client.GirderClient", FakeGirderClient) + + def fake_pipeline_def(pipeline): + return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} + + wf_counter = 0 + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + nonlocal wf_counter + wf_counter += 1 + return 'workflow-X' + str(wf_counter) + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + def fake_delete_path(path): + nonlocal removed + removed = True + return True + + mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") + mocked_pipeline_def.side_effect = fake_pipeline_def + + mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") + mocked_init_exec.side_effect = fake_init_exec + + mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") + mocked_execution_info.side_effect = fake_execution_info + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + # Launch a Full Session Run + s = VipCI() + s.pipeline_id = pipeline_id + s.output_dir = PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS") + s.input_settings = { + "zipped_folder": 'fake_value', + "basis_file": 'fake_value', + "signal_file": ['fake_value', 'fake_value'], + "control_file": ['fake_value'] + } + s.run_session(nb_runs=nb_runs) + # Check the Results + assert s.workflows + assert len(s.workflows) == nb_runs + for wid in s.workflows: + assert s.workflows[wid]["status"] == "Finished" + assert s.pipeline_id == pipeline_id + +@pytest.mark.parametrize( + "backup_location, input_settings, pipeline_id, output_dir", + [ + ('girder', { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ), + (None, { + "zipped_folder": None, + "basis_file": None, + "signal_file": None, + "control_file": None + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ), + ('girder', { + "zipped_folder": 'different_value1', + "basis_file": 'different_value2', + "signal_file": ['different_value3', 'different_value4'], + "control_file": ['different_value5'] + }, "LCModel/0.1", PurePosixPath("/vip/Home/test-VipLauncher/OUTPUTS"), + ) + ] +) +def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): + + global pipeline_id_global # Use to dynamically change the pipeline_id of the mocked GirderClient + pipeline_id_global = pipeline_id + + class FakeGirderClient(): + def __init__(self, apiUrl): + pass + + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + print("CASCA") + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(self, folderId): + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": pipeline_id, + 'session_name': 'test-VipLauncher', + 'workflows': {}, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + mocker.patch("girder_client.GirderClient", FakeGirderClient) + + def fake_exists(path): + return True + + def fake_pathlib_exists(): + return True + + def fake_delete_path(path): + return True + + def fake_upload(local_path, vip_path): + return True + + def fake_download(vip_path, local_path): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + print("EENTER") + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + def fake_unlink_linux(): + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") + mocked_pathlib_exists.side_effect = fake_pathlib_exists + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_upload = mocker.patch("vip_client.utils.vip.upload") + mocked_upload.side_effect = fake_upload + + mocked_download_file = mocker.patch("vip_client.utils.vip.download") + mocked_download_file.side_effect = fake_download + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux + + VipCI._BACKUP_LOCATION = backup_location + # Return if backup is disabled + if VipCI._BACKUP_LOCATION is None: + return + # Create session + s1 = VipCI() + s1.input_settings = input_settings + s1.pipeline_id = pipeline_id + s1.output_dir = output_dir + # Backup + s1._save() + # Load backup + s2 = VipCI(output_dir=s1.output_dir) + # Check parameters + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id + assert s2.output_dir == s1.output_dir + assert s2.workflows == s1.workflows + + +def test_properties_interface(mocker): + + def fake_exists(path): + return True + + def fake_pathlib_exists(): + return True + + def fake_delete_path(path): + return True + + def fake_upload(local_path, vip_path): + return True + + def fake_download(vip_path, local_path): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + print("EENTER") + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + def fake_unlink_linux(): + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") + mocked_pathlib_exists.side_effect = fake_pathlib_exists + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_upload = mocker.patch("vip_client.utils.vip.upload") + mocked_upload.side_effect = fake_upload + + mocked_download_file = mocker.patch("vip_client.utils.vip.download") + mocked_download_file.side_effect = fake_download + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux + + VipCI._BACKUP_LOCATION = "girder" + + # Copy the first session + s = VipCI() + s.input_settings = { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + } + # Backup the inputs + backup = s.input_settings + # Run a subtest for each property + for prop in s.input_settings: + setattr(s, prop, None) # Calls deleter + assert getattr(s, prop) is None # Public attribute must be None + assert not s._is_defined("_" + prop) # Private attribute must be unset + setattr(s, prop, backup[prop]) # Reset + # Test correct reset + for key, value in s.input_settings.items(): + assert getattr(s, key) == value From 1c28c60a2c524aeca1ba00e481facab82bbc6c95 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Tue, 9 Apr 2024 16:26:53 +0200 Subject: [PATCH 16/33] Add girder client as dependency with pip --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c7ed2b0..1a3adf7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -25,7 +25,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install pytest pytest_mock requests + pip install pytest pytest_mock requests girder_client - name: Run Pytest run: | From 8180a0b4264f9da5f311948d50ad411cf9ff9248 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 10 Apr 2024 10:23:23 +0200 Subject: [PATCH 17/33] First tests for VipSession --- src/vip_client/classes/VipLauncher.py | 6 + tests/test_VipCI.py | 6 - tests/test_VipLauncher.py | 3 - tests/test_VipSession.py | 344 ++++++++++++++++++++++++++ 4 files changed, 350 insertions(+), 9 deletions(-) create mode 100644 tests/test_VipSession.py diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index 2cf7407..1d84e75 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -1230,18 +1230,24 @@ def _save(self) -> bool: - Returns a success flag; - Displays information unless `_VERBOSE` is False. """ + print("BERSERK") # Return if no-backup mode is activated + print("BACKUP LOCATION: ", self._BACKUP_LOCATION) if self._BACKUP_LOCATION is None: return False # Get session properties session_data = self._data_to_save() + print("CASCO: ", session_data) # Get backup data from the output directory with self._silent_session(): backup_data = self._load_session(location=self._BACKUP_LOCATION) # If there is no backup data (i.e. None or empty dict), save immediately if not backup_data: + print("BERSERKER") return self._save_session(session_data, location=self._BACKUP_LOCATION) # If the session name is different from backup, raise an error + print("SESSION NAME: ", session_data) + print("BACKUP NAME: ", backup_data) if backup_data["session_name"] != session_data["session_name"]: raise ValueError( f"The backup data have a different session name ('{backup_data['session_name']}').\n" diff --git a/tests/test_VipCI.py b/tests/test_VipCI.py index 394c0af..7769513 100644 --- a/tests/test_VipCI.py +++ b/tests/test_VipCI.py @@ -37,7 +37,6 @@ def authenticate(self, apiKey): return True def resourceLookup(self, path): - print("CASCA") return {'_id': 'fake_id', '_modelType': 'folder'} def createFolder(self, parentId, name, reuseExisting=True, **kwargs): @@ -79,7 +78,6 @@ def listItem(self, folderId): # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: f.write('{}') - print("Trying to create the buffer file") # Mock the VIP API mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") mocked_list_pipeline.return_value = [ @@ -140,7 +138,6 @@ def authenticate(self, apiKey): return True def resourceLookup(self, path): - print("CASCA") return {'_id': 'fake_id', '_modelType': 'folder'} def createFolder(self, parentId, name, reuseExisting=True, **kwargs): @@ -261,7 +258,6 @@ def authenticate(self, apiKey): return True def resourceLookup(self, path): - print("CASCA") return {'_id': 'fake_id', '_modelType': 'folder'} def createFolder(self, parentId, name, reuseExisting=True, **kwargs): @@ -311,7 +307,6 @@ def fake_download(vip_path, local_path): return True def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - print("EENTER") return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) def fake_unlink(self): @@ -381,7 +376,6 @@ def fake_download(vip_path, local_path): return True def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - print("EENTER") return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) def fake_unlink(self): diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index 36a330e..ac68ba4 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -29,7 +29,6 @@ def setup_teardown_vip_launcher(request, mocker): # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: f.write('{}') - print("Trying to create the buffer file") # Mock the VIP API mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") mocked_list_pipeline.return_value = [ @@ -177,7 +176,6 @@ def fake_download(vip_path, local_path): return True def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - print("EENTER") return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) def fake_unlink(self): @@ -247,7 +245,6 @@ def fake_download(vip_path, local_path): return True def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - print("EENTER") return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) def fake_unlink(self): diff --git a/tests/test_VipSession.py b/tests/test_VipSession.py new file mode 100644 index 0000000..f70b3c3 --- /dev/null +++ b/tests/test_VipSession.py @@ -0,0 +1,344 @@ +import io +from unittest.mock import patch +import pytest +from pathlib import * + +import pytest_mock +from src.vip_client.utils import vip +from src.vip_client.classes import VipSession + + +pipeline_id_global = "LCModel/0.1" + +def get_properties(obj) -> dict: + """ + Get session properties as they should be returned by the getter functions + """ + # Function to parse a single element + def get_element(element): + if isinstance(element, dict): + return {key: get_element(value) for key, value in element.items()} + elif isinstance(element, list): + return [get_element(value) for value in element] + elif element is None: + return None + else: + return str(element) + # Return + return {prop: get_element(value) for prop, value in obj.input_properties.items()} + +@pytest.fixture(scope="function", autouse=True) +def setup_teardown_vip_launcher(request, mocker): + + # Create a buffer file for the backup + with open('tmp_data.json', 'w') as f: + f.write('{}') + # Mock the VIP API + mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") + mocked_list_pipeline.return_value = [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_vip_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") + mocked_set_api_key.side_effect = fake_set_vip_api_key + + # Setup code before running the tests in the class + print("Handshake with VIP") + VipSession.init(api_key="FAKE_KEY") + print("Setup done") + +@pytest.fixture(scope="function", autouse=True) +def cleanup(): + # Teardown code after running each test function + yield + # Remove the buffer file + try: + Path('tmp_data.json').unlink() + except FileNotFoundError: + pass + + +@pytest.mark.parametrize( + "nb_runs, pipeline_id", + [ + (1, "LCModel/0.1"), + (2, "CQUEST/0.3"), + (3, "LCModel/0.1") + ] +) +def test_run_and_finish(mocker, nb_runs, pipeline_id): + + removed = False + + def fake_pipeline_def(pipeline): + return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} + + wf_counter = 0 + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + nonlocal wf_counter + wf_counter += 1 + return 'workflow-X' + str(wf_counter) + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + def fake_delete_path(path): + nonlocal removed + removed = True + return True + + def fake_exists(cls=None, path=None, location="local"): + return True + + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + def fake_unlink_linux(): + return True + + def fake_list_elements(self): + return [{'name': 'element1', 'path': 'path1'}, {'name': 'element2', 'path': 'path2'}] + + def fake_pathlib_iterdir(): + return [Path('tmp_data.json')] + + with patch.object(VipSession, '_exists', fake_exists): + + mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") + mocked_pipeline_def.side_effect = fake_pipeline_def + + mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") + mocked_init_exec.side_effect = fake_init_exec + + mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") + mocked_execution_info.side_effect = fake_execution_info + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux + + mocked_list_elements = mocker.patch("vip_client.utils.vip.list_elements") + mocked_list_elements.side_effect = fake_list_elements + + mocked_pathlib_iterdir = mocker.patch("pathlib.Path.iterdir") + mocked_pathlib_iterdir.side_effect = fake_pathlib_iterdir + + # Launch a Full Session Run + s = VipSession(output_dir="test-VipSession/out", input_dir="test-VipSession/in") + s.pipeline_id = pipeline_id + s.input_settings = { + "zipped_folder": 'fake_value', + "basis_file": 'fake_value', + "signal_file": ['fake_value', 'fake_value'], + "control_file": ['fake_value'] + } + s.run_session(nb_runs=nb_runs) + # Check the Results + assert s.workflows + assert len(s.workflows) == nb_runs + for wid in s.workflows: + assert s.workflows[wid]["status"] == "Finished" + assert s.pipeline_id == pipeline_id + +@pytest.mark.parametrize( + "backup_location, input_settings, pipeline_id, output_dir", + [ + ('local', { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + }, "LCModel/0.1", "test-VipSession/out", + ), + (None, { + "zipped_folder": None, + "basis_file": None, + "signal_file": None, + "control_file": None + }, "LCModel/0.1", "test-VipSession/out", + ), + ('local', { + "zipped_folder": 'different_value1', + "basis_file": 'different_value2', + "signal_file": ['different_value3', 'different_value4'], + "control_file": ['different_value5'] + }, "LCModel/0.1", "test-VipSession/out", + ) + ] +) +def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): + def fake_pipeline_def(pipeline): + return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} + + wf_counter = 0 + s1_init = True + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + nonlocal wf_counter + wf_counter += 1 + return 'workflow-X' + str(wf_counter) + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + def fake_exists(cls=None, path=None, location="local"): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + def fake_unlink_linux(): + return True + + def fake_list_elements(self): + return [{'name': 'element1', 'path': 'path1'}, {'name': 'element2', 'path': 'path2'}] + + def fake_pathlib_iterdir(): + return [Path('tmp_data.json')] + + def fake_is_file(): + nonlocal s1_init + return not s1_init # If s1 is initialized, return False for not using the backup + + with patch.object(VipSession, '_exists', fake_exists): + mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") + mocked_pipeline_def.side_effect = fake_pipeline_def + + mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") + mocked_init_exec.side_effect = fake_init_exec + + mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") + mocked_execution_info.side_effect = fake_execution_info + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux + + mocked_list_elements = mocker.patch("vip_client.utils.vip.list_elements") + mocked_list_elements.side_effect = fake_list_elements + + mocked_pathlib_iterdir = mocker.patch("pathlib.Path.iterdir") + mocked_pathlib_iterdir.side_effect = fake_pathlib_iterdir + + mocked_is_file = mocker.patch("pathlib.Path.is_file") + mocked_is_file.side_effect = fake_is_file + + VipSession._BACKUP_LOCATION = backup_location + # Return if backup is disabled + if VipSession._BACKUP_LOCATION is None: + return + # Create session + s1 = VipSession(output_dir=output_dir) + s1.input_settings = input_settings + s1.pipeline_id = pipeline_id + # Backup + s1._save() + # Set the s1 initialization flag to False + s1_init = False + + # Load backup + s2 = VipSession(output_dir=s1.output_dir) + # Check parameters + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id + assert s2.output_dir == s1.output_dir + assert s2.workflows == s1.workflows + + +def test_properties_interface(mocker): + + def fake_exists(path): + return True + + def fake_pathlib_exists(): + return True + + def fake_delete_path(path): + return True + + def fake_upload(local_path, vip_path): + return True + + def fake_download(vip_path, local_path): + return True + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_unlink(self): + return True + + def fake_unlink_linux(): + return True + + mocked_exists = mocker.patch("vip_client.utils.vip.exists") + mocked_exists.side_effect = fake_exists + + mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") + mocked_pathlib_exists.side_effect = fake_pathlib_exists + + mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") + mocked_delete_path.side_effect = fake_delete_path + + mocked_upload = mocker.patch("vip_client.utils.vip.upload") + mocked_upload.side_effect = fake_upload + + mocked_download_file = mocker.patch("vip_client.utils.vip.download") + mocked_download_file.side_effect = fake_download + + mocked_pathlib_open = mocker.patch("pathlib.Path.open") + mocked_pathlib_open.side_effect = fake_pathlib_open + + mocked_unlink = mocker.patch("os.unlink") + mocked_unlink.side_effect = fake_unlink + mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") + mocked_unlink_linux.side_effect = fake_unlink_linux + + VipSession._BACKUP_LOCATION = "local" + + # Copy the first session + s = VipSession() + s.input_settings = { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": ['fake_value5'] + } + # Backup the inputs + backup = s.input_settings + # Run a subtest for each property + for prop in s.input_settings: + setattr(s, prop, None) # Calls deleter + assert getattr(s, prop) is None # Public attribute must be None + assert not s._is_defined("_" + prop) # Private attribute must be unset + setattr(s, prop, backup[prop]) # Reset + # Test correct reset + for key, value in s.input_settings.items(): + assert getattr(s, key) == value From ce1612c52fefb6fac02c2e4b333cfad1a91855de Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 10 Apr 2024 13:56:48 +0200 Subject: [PATCH 18/33] Refactoring tests for VipLauncher --- src/vip_client/classes/VipLauncher.py | 2 - tests/test_VipLauncher.py | 247 ++++++++++++-------------- tmp_data.json | 1 + 3 files changed, 115 insertions(+), 135 deletions(-) create mode 100644 tmp_data.json diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index 1d84e75..48f8476 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -1230,7 +1230,6 @@ def _save(self) -> bool: - Returns a success flag; - Displays information unless `_VERBOSE` is False. """ - print("BERSERK") # Return if no-backup mode is activated print("BACKUP LOCATION: ", self._BACKUP_LOCATION) if self._BACKUP_LOCATION is None: @@ -1243,7 +1242,6 @@ def _save(self) -> bool: backup_data = self._load_session(location=self._BACKUP_LOCATION) # If there is no backup data (i.e. None or empty dict), save immediately if not backup_data: - print("BERSERKER") return self._save_session(session_data, location=self._BACKUP_LOCATION) # If the session name is different from backup, raise an error print("SESSION NAME: ", session_data) diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index ac68ba4..62c2e9a 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -1,12 +1,97 @@ import io import pytest -import unittest from pathlib import * -import pytest_mock from src.vip_client.utils import vip from src.vip_client.classes import VipLauncher + +def mock_vip_api(mocker, pipeline_id): + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_list_pipeline(): + return [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + + def fake_pipeline_def(pipeline): + return { + 'identifier': pipeline_id, + 'name': 'LCModel', + 'description': 'MR spectrosocpy signal quantification software', + 'version': '0.1', + 'parameters': [ + { + 'name': 'zipped_folder', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': 'Archive containing all metabolite & macromolecules in .RAW format', + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'basis_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.basis' containing information & prior ...", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'signal_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.RAW' containing the signal to quantify", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'control_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'script_file', + 'type': 'File', + 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', + 'description': 'Script lauching lcmodel', + 'isOptional': False, 'isReturnedValue': False + } + ], + 'canExecute': True + } + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + return 'workflow-XXXXXX' + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + mocker.patch("vip_client.utils.vip.exists").return_value = True + mocker.patch("pathlib.Path.exists").return_value = True + mocker.patch("vip_client.utils.vip.upload").return_value = True + mocker.patch("vip_client.utils.vip.download").return_value = True + mocker.patch("os.unlink").return_value = True + mocker.patch("pathlib.Path.unlink").return_value = True + mocker.patch("pathlib.Path.open").side_effect = fake_pathlib_open + mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def + mocker.patch("vip_client.utils.vip.list_pipeline").side_effect = fake_list_pipeline + mocker.patch("vip_client.utils.vip.setApiKey").side_effect = fake_set_api_key + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info + def get_properties(obj) -> dict: """ Get session properties as they should be returned by the getter functions @@ -30,20 +115,7 @@ def setup_teardown_vip_launcher(request, mocker): with open('tmp_data.json', 'w') as f: f.write('{}') # Mock the VIP API - mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") - mocked_list_pipeline.return_value = [ - {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True}, - {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True} - ] - - def fake_set_api_key(api_key): - return True if api_key == "FAKE_KEY" else False - - mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") - mocked_set_api_key.side_effect = fake_set_api_key - + mock_vip_api(mocker, "LCModel/0.1") # Setup code before running the tests in the class print("Handshake with VIP") VipLauncher.init(api_key="FAKE_KEY") @@ -79,34 +151,15 @@ def fake_exists(path): return True return False - def fake_pipeline_def(pipeline): - return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} - - def fake_init_exec(pipeline, name, inputValues, resultsLocation): - return 'workflow-XXXXXX' - - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} - def fake_delete_path(path): nonlocal removed removed = True return True - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") - mocked_pipeline_def.side_effect = fake_pipeline_def - - mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") - mocked_init_exec.side_effect = fake_init_exec - - mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") - mocked_execution_info.side_effect = fake_execution_info + mocker.patch("vip_client.utils.vip.exists").side_effect = fake_exists + mocker.patch("vip_client.utils.vip.delete_path").side_effect = fake_delete_path - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path + removed = False # Launch a Full Session Run s = VipLauncher() @@ -159,58 +212,27 @@ def fake_delete_path(path): ] ) def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): + + removed = False def fake_exists(path): - return True - - def fake_pathlib_exists(): - return True - - def fake_delete_path(path): - return True - - def fake_upload(local_path, vip_path): - return True - - def fake_download(vip_path, local_path): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + if path == '/vip/Home/test-VipLauncher/OUTPUTS': + return True + if path == 'fake_value' and not removed: + return True + return False - def fake_unlink(self): + def fake_delete_path(path): + nonlocal removed + removed = True return True - def fake_unlink_linux(): - return True - - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") - mocked_pathlib_exists.side_effect = fake_pathlib_exists - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_upload = mocker.patch("vip_client.utils.vip.upload") - mocked_upload.side_effect = fake_upload + mocker.patch("vip_client.utils.vip.exists").side_effect = fake_exists + mocker.patch("vip_client.utils.vip.delete_path").side_effect = fake_delete_path - mocked_download_file = mocker.patch("vip_client.utils.vip.download") - mocked_download_file.side_effect = fake_download - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux + mock_vip_api(mocker, pipeline_id) VipLauncher._BACKUP_LOCATION = backup_location - # Return if backup is disabled - if VipLauncher._BACKUP_LOCATION is None: - return # Create session s1 = VipLauncher() s1.input_settings = input_settings @@ -221,61 +243,20 @@ def fake_unlink_linux(): # Load backup s2 = VipLauncher(output_dir=s1.output_dir) # Check parameters - assert s2.input_settings == s1.input_settings - assert s2.pipeline_id == s1.pipeline_id - assert s2.output_dir == s1.output_dir - assert s2.workflows == s1.workflows + if backup_location is not None: + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id + assert s2.output_dir == s1.output_dir + assert s2.workflows == s1.workflows + else: + assert s2.input_settings == None + assert s2.pipeline_id is None + assert s2.output_dir == s1.output_dir + assert s2.workflows == {} def test_properties_interface(mocker): - def fake_exists(path): - return True - - def fake_pathlib_exists(): - return True - - def fake_delete_path(path): - return True - - def fake_upload(local_path, vip_path): - return True - - def fake_download(vip_path, local_path): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") - mocked_pathlib_exists.side_effect = fake_pathlib_exists - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_upload = mocker.patch("vip_client.utils.vip.upload") - mocked_upload.side_effect = fake_upload - - mocked_download_file = mocker.patch("vip_client.utils.vip.download") - mocked_download_file.side_effect = fake_download - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - VipLauncher._BACKUP_LOCATION = "vip" # Copy the first session diff --git a/tmp_data.json b/tmp_data.json new file mode 100644 index 0000000..9e26dfe --- /dev/null +++ b/tmp_data.json @@ -0,0 +1 @@ +{} \ No newline at end of file From 32cb494b53dc7d02828a03fdee87ee9f8ed15bd7 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 10 Apr 2024 15:06:52 +0200 Subject: [PATCH 19/33] Refactoring tests for VipCI --- src/vip_client/classes/VipCI.py | 2 - tests/test_VipCI.py | 420 +++++++++++--------------------- tmp_data.json | 1 - 3 files changed, 145 insertions(+), 278 deletions(-) delete mode 100644 tmp_data.json diff --git a/src/vip_client/classes/VipCI.py b/src/vip_client/classes/VipCI.py index 036ae9b..b8ad6e4 100644 --- a/src/vip_client/classes/VipCI.py +++ b/src/vip_client/classes/VipCI.py @@ -434,8 +434,6 @@ def _load_session(self, location="girder") -> dict: try: girder_id, _ = self._girder_path_to_id(self.vip_output_dir) folder = self._girder_client.getFolder(folderId=girder_id) - print("CASCA") - print(folder) except girder_client.HttpError as e: if e.status == 400: # Folder was not found return None diff --git a/tests/test_VipCI.py b/tests/test_VipCI.py index 7769513..27d76f8 100644 --- a/tests/test_VipCI.py +++ b/tests/test_VipCI.py @@ -7,7 +7,142 @@ from src.vip_client.classes import VipCI -pipeline_id_global = "LCModel/0.1" +def mock_vip_api(mocker, pipeline_id): + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_list_pipeline(): + return [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + + def fake_pipeline_def(pipeline): + return { + 'identifier': pipeline_id, + 'name': 'LCModel', + 'description': 'MR spectrosocpy signal quantification software', + 'version': '0.1', + 'parameters': [ + { + 'name': 'zipped_folder', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': 'Archive containing all metabolite & macromolecules in .RAW format', + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'basis_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.basis' containing information & prior ...", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'signal_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.RAW' containing the signal to quantify", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'control_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'script_file', + 'type': 'File', + 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', + 'description': 'Script lauching lcmodel', + 'isOptional': False, 'isReturnedValue': False + } + ], + 'canExecute': True + } + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + return 'workflow-XXXXXX' + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + mocker.patch("vip_client.utils.vip.exists").return_value = True + mocker.patch("pathlib.Path.exists").return_value = True + mocker.patch("vip_client.utils.vip.upload").return_value = True + mocker.patch("vip_client.utils.vip.download").return_value = True + mocker.patch("os.unlink").return_value = True + mocker.patch("pathlib.Path.unlink").return_value = True + mocker.patch("pathlib.Path.open").side_effect = fake_pathlib_open + mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def + mocker.patch("vip_client.utils.vip.list_pipeline").side_effect = fake_list_pipeline + mocker.patch("vip_client.utils.vip.setApiKey").side_effect = fake_set_api_key + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info + +class FakeGirderClient(): + + pipeline_id = "LCModel/0.1" + def __init__(self, apiUrl): + pass + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(cls, folderId): + print("GUTS: ", cls.pipeline_id) + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": cls.pipeline_id, + 'session_name': 'test-VipLauncher', + 'workflows': {}, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + @classmethod + def set_pipeline_id(cls, pipeline_id): + print("GRIFITH: ", pipeline_id) + cls.pipeline_id = pipeline_id + + +def mock_girder_client(mocker): + mocker.patch("girder_client.GirderClient", FakeGirderClient) + def get_properties(obj) -> dict: """ @@ -28,77 +163,14 @@ def get_element(element): @pytest.fixture(scope="function", autouse=True) def setup_teardown_vip_launcher(request, mocker): - - class FakeGirderClient(): - def __init__(self, apiUrl): - pass - - def authenticate(self, apiKey): - return True - - def resourceLookup(self, path): - return {'_id': 'fake_id', '_modelType': 'folder'} - - def createFolder(self, parentId, name, reuseExisting=True, **kwargs): - return {'_id': 'fake_id'} - - def addMetadataToFolder(self, folderId, metadata): - return True - - def getFolder(self, folderId): - metadata = { - 'input_settings': { - 'zipped_folder': 'fake_value', - 'basis_file': 'fake_value', - 'signal_file': ['fake_value', 'fake_value'], - 'control_file': ['fake_value']}, - "pipeline_id": pipeline_id_global, - 'session_name': 'test-VipLauncher', - 'workflows': { - # "workflow-fe5pXw": { - # "output_path": "/collection/ReproVIPSpectro/results/atest_recup_session/2023-11-14_15:27:07", - # "start": "2023/11/14 15:27:13", - # "status": "Finished"} - }, - "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" - } - return {'_id': 'fake_id', 'meta': metadata} - - def get(self, path): - return {'_id': 'fake_id'} - - def listFiles(self, folderId): - return [{'_id': 'fake_id'}] - - def listItem(self, folderId): - return {'_id': 'fake_id'} - - mocker.patch("girder_client.GirderClient", FakeGirderClient) + # Mock the VIP API + mock_vip_api(mocker, "LCModel/0.1") + # Mock the Girder Client + mock_girder_client(mocker) # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: f.write('{}') - # Mock the VIP API - mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") - mocked_list_pipeline.return_value = [ - {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True}, - {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True} - ] - - def fake_set_vip_api_key(api_key): - return True if api_key == "FAKE_KEY" else False - - def fake_set_girder_api_key(apiKey): - return True if apiKey == "FAKE_KEY" else False - - - mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") - mocked_set_api_key.side_effect = fake_set_vip_api_key - - mocked_set_girder_api_key = mocker.patch("girder_client.GirderClient.authenticate") - mocked_set_girder_api_key.side_effect = fake_set_girder_api_key # Setup code before running the tests in the class print("Handshake with VIP") @@ -126,81 +198,17 @@ def cleanup(): ) def test_run_and_finish(mocker, nb_runs, pipeline_id): - removed = False - global pipeline_id_global # Use to dynamically change the pipeline_id of the mocked GirderClient - pipeline_id_global = pipeline_id - - class FakeGirderClient(): - def __init__(self, apiUrl): - pass - - def authenticate(self, apiKey): - return True - - def resourceLookup(self, path): - return {'_id': 'fake_id', '_modelType': 'folder'} - - def createFolder(self, parentId, name, reuseExisting=True, **kwargs): - return {'_id': 'fake_id'} - - def addMetadataToFolder(self, folderId, metadata): - return True - - def getFolder(self, folderId): - metadata = { - 'input_settings': { - 'zipped_folder': 'fake_value', - 'basis_file': 'fake_value', - 'signal_file': ['fake_value', 'fake_value'], - 'control_file': ['fake_value']}, - "pipeline_id": pipeline_id, - 'session_name': 'test-VipLauncher', - 'workflows': {}, - "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" - } - return {'_id': 'fake_id', 'meta': metadata} - - def get(self, path): - return {'_id': 'fake_id'} - - def listFiles(self, folderId): - return [{'_id': 'fake_id'}] - - def listItem(self, folderId): - return {'_id': 'fake_id'} - - mocker.patch("girder_client.GirderClient", FakeGirderClient) - - def fake_pipeline_def(pipeline): - return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} - + FakeGirderClient.set_pipeline_id(pipeline_id) wf_counter = 0 - + def fake_init_exec(pipeline, name, inputValues, resultsLocation): nonlocal wf_counter wf_counter += 1 - return 'workflow-X' + str(wf_counter) + return f'workflow-{wf_counter}' - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + # Re patch the init_exec function to update the workflow counter + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec - def fake_delete_path(path): - nonlocal removed - removed = True - return True - - mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") - mocked_pipeline_def.side_effect = fake_pipeline_def - - mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") - mocked_init_exec.side_effect = fake_init_exec - - mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") - mocked_execution_info.side_effect = fake_execution_info - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - # Launch a Full Session Run s = VipCI() s.pipeline_id = pipeline_id @@ -247,97 +255,6 @@ def fake_delete_path(path): ) def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): - global pipeline_id_global # Use to dynamically change the pipeline_id of the mocked GirderClient - pipeline_id_global = pipeline_id - - class FakeGirderClient(): - def __init__(self, apiUrl): - pass - - def authenticate(self, apiKey): - return True - - def resourceLookup(self, path): - return {'_id': 'fake_id', '_modelType': 'folder'} - - def createFolder(self, parentId, name, reuseExisting=True, **kwargs): - return {'_id': 'fake_id'} - - def addMetadataToFolder(self, folderId, metadata): - return True - - def getFolder(self, folderId): - metadata = { - 'input_settings': { - 'zipped_folder': 'fake_value', - 'basis_file': 'fake_value', - 'signal_file': ['fake_value', 'fake_value'], - 'control_file': ['fake_value']}, - "pipeline_id": pipeline_id, - 'session_name': 'test-VipLauncher', - 'workflows': {}, - "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" - } - return {'_id': 'fake_id', 'meta': metadata} - - def get(self, path): - return {'_id': 'fake_id'} - - def listFiles(self, folderId): - return [{'_id': 'fake_id'}] - - def listItem(self, folderId): - return {'_id': 'fake_id'} - - mocker.patch("girder_client.GirderClient", FakeGirderClient) - - def fake_exists(path): - return True - - def fake_pathlib_exists(): - return True - - def fake_delete_path(path): - return True - - def fake_upload(local_path, vip_path): - return True - - def fake_download(vip_path, local_path): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") - mocked_pathlib_exists.side_effect = fake_pathlib_exists - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_upload = mocker.patch("vip_client.utils.vip.upload") - mocked_upload.side_effect = fake_upload - - mocked_download_file = mocker.patch("vip_client.utils.vip.download") - mocked_download_file.side_effect = fake_download - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - VipCI._BACKUP_LOCATION = backup_location # Return if backup is disabled if VipCI._BACKUP_LOCATION is None: @@ -360,53 +277,6 @@ def fake_unlink_linux(): def test_properties_interface(mocker): - def fake_exists(path): - return True - - def fake_pathlib_exists(): - return True - - def fake_delete_path(path): - return True - - def fake_upload(local_path, vip_path): - return True - - def fake_download(vip_path, local_path): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") - mocked_pathlib_exists.side_effect = fake_pathlib_exists - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_upload = mocker.patch("vip_client.utils.vip.upload") - mocked_upload.side_effect = fake_upload - - mocked_download_file = mocker.patch("vip_client.utils.vip.download") - mocked_download_file.side_effect = fake_download - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - VipCI._BACKUP_LOCATION = "girder" # Copy the first session diff --git a/tmp_data.json b/tmp_data.json deleted file mode 100644 index 9e26dfe..0000000 --- a/tmp_data.json +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file From 761771d27cbb379976f7027d1b012ce660977916 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 10 Apr 2024 16:38:23 +0200 Subject: [PATCH 20/33] Refactoring for all the tests --- tests/FakeGirderClient.py | 44 +++++++ tests/mocked_services.py | 149 ++++++++++++++++++++++++ tests/test_VipCI.py | 144 +---------------------- tests/test_VipLauncher.py | 91 +-------------- tests/test_VipSession.py | 238 +++++++------------------------------- 5 files changed, 247 insertions(+), 419 deletions(-) create mode 100644 tests/FakeGirderClient.py create mode 100644 tests/mocked_services.py diff --git a/tests/FakeGirderClient.py b/tests/FakeGirderClient.py new file mode 100644 index 0000000..a1a40cb --- /dev/null +++ b/tests/FakeGirderClient.py @@ -0,0 +1,44 @@ + +class FakeGirderClient(): + + pipeline_id = "LCModel/0.1" + def __init__(self, apiUrl): + pass + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(cls, folderId): + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": cls.pipeline_id, + 'session_name': 'test-VipLauncher', + 'workflows': {}, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + @classmethod + def set_pipeline_id(cls, pipeline_id): + cls.pipeline_id = pipeline_id \ No newline at end of file diff --git a/tests/mocked_services.py b/tests/mocked_services.py new file mode 100644 index 0000000..57f6ce1 --- /dev/null +++ b/tests/mocked_services.py @@ -0,0 +1,149 @@ +import io +from pathlib import Path + + +def mock_vip_api(mocker, pipeline_id): + + def fake_list_pipeline(): + return [ + {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True}, + {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, + 'version': '0.1', 'parameters': [], 'canExecute': True} + ] + + def fake_set_api_key(api_key): + return True if api_key == "FAKE_KEY" else False + + def fake_pipeline_def(pipeline): + return { + 'identifier': pipeline_id, + 'name': 'LCModel', + 'description': 'MR spectrosocpy signal quantification software', + 'version': '0.1', + 'parameters': [ + { + 'name': 'zipped_folder', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': 'Archive containing all metabolite & macromolecules in .RAW format', + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'basis_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.basis' containing information & prior ...", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'signal_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.RAW' containing the signal to quantify", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'control_file', + 'type': 'File', + 'defaultValue': '$input.getDefaultValue()', + 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", + 'isOptional': False, + 'isReturnedValue': False + }, + { + 'name': 'script_file', + 'type': 'File', + 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', + 'description': 'Script lauching lcmodel', + 'isOptional': False, 'isReturnedValue': False + } + ], + 'canExecute': True + } + + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + return 'workflow-XXXXXX' + + def fake_execution_info(workflow_id): + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + + def fake_list_elements(self): + return [{'name': 'element1', 'path': 'path1'}, {'name': 'element2', 'path': 'path2'}] + + mocker.patch("vip_client.utils.vip.exists").return_value = True + mocker.patch("vip_client.utils.vip.upload").return_value = True + mocker.patch("vip_client.utils.vip.download").return_value = True + mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def + mocker.patch("vip_client.utils.vip.list_pipeline").side_effect = fake_list_pipeline + mocker.patch("vip_client.utils.vip.setApiKey").side_effect = fake_set_api_key + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info + mocker.patch("vip_client.utils.vip.list_elements").side_effect = fake_list_elements + +def mock_pathlib(mocker): + + def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): + return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) + + def fake_pathlib_iterdir(): + return [Path('tmp_data.json')] + + mocker.patch("pathlib.Path.exists").return_value = True + mocker.patch("pathlib.Path.unlink").return_value = True + mocker.patch("pathlib.Path.open").side_effect = fake_pathlib_open + mocker.patch("pathlib.Path.iterdir").side_effect = fake_pathlib_iterdir + +def mock_os(mocker): + mocker.patch("os.unlink") + + class FakeGirderClient(): + + pipeline_id = "LCModel/0.1" + def __init__(self, apiUrl): + pass + def authenticate(self, apiKey): + return True + + def resourceLookup(self, path): + return {'_id': 'fake_id', '_modelType': 'folder'} + + def createFolder(self, parentId, name, reuseExisting=True, **kwargs): + return {'_id': 'fake_id'} + + def addMetadataToFolder(self, folderId, metadata): + return True + + def getFolder(cls, folderId): + metadata = { + 'input_settings': { + 'zipped_folder': 'fake_value', + 'basis_file': 'fake_value', + 'signal_file': ['fake_value', 'fake_value'], + 'control_file': ['fake_value']}, + "pipeline_id": cls.pipeline_id, + 'session_name': 'test-VipLauncher', + 'workflows': {}, + "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" + } + return {'_id': 'fake_id', 'meta': metadata} + + def get(self, path): + return {'_id': 'fake_id'} + + def listFiles(self, folderId): + return [{'_id': 'fake_id'}] + + def listItem(self, folderId): + return {'_id': 'fake_id'} + + @classmethod + def set_pipeline_id(cls, pipeline_id): + cls.pipeline_id = pipeline_id + +def mock_girder_client(mocker): + from FakeGirderClient import FakeGirderClient + mocker.patch("girder_client.GirderClient", FakeGirderClient) \ No newline at end of file diff --git a/tests/test_VipCI.py b/tests/test_VipCI.py index 27d76f8..4335439 100644 --- a/tests/test_VipCI.py +++ b/tests/test_VipCI.py @@ -5,143 +5,8 @@ import pytest_mock from src.vip_client.utils import vip from src.vip_client.classes import VipCI - - -def mock_vip_api(mocker, pipeline_id): - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_list_pipeline(): - return [ - {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True}, - {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True} - ] - - def fake_set_api_key(api_key): - return True if api_key == "FAKE_KEY" else False - - - def fake_pipeline_def(pipeline): - return { - 'identifier': pipeline_id, - 'name': 'LCModel', - 'description': 'MR spectrosocpy signal quantification software', - 'version': '0.1', - 'parameters': [ - { - 'name': 'zipped_folder', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': 'Archive containing all metabolite & macromolecules in .RAW format', - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'basis_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.basis' containing information & prior ...", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'signal_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.RAW' containing the signal to quantify", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'control_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'script_file', - 'type': 'File', - 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', - 'description': 'Script lauching lcmodel', - 'isOptional': False, 'isReturnedValue': False - } - ], - 'canExecute': True - } - - def fake_init_exec(pipeline, name, inputValues, resultsLocation): - return 'workflow-XXXXXX' - - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} - - mocker.patch("vip_client.utils.vip.exists").return_value = True - mocker.patch("pathlib.Path.exists").return_value = True - mocker.patch("vip_client.utils.vip.upload").return_value = True - mocker.patch("vip_client.utils.vip.download").return_value = True - mocker.patch("os.unlink").return_value = True - mocker.patch("pathlib.Path.unlink").return_value = True - mocker.patch("pathlib.Path.open").side_effect = fake_pathlib_open - mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def - mocker.patch("vip_client.utils.vip.list_pipeline").side_effect = fake_list_pipeline - mocker.patch("vip_client.utils.vip.setApiKey").side_effect = fake_set_api_key - mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec - mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info - -class FakeGirderClient(): - - pipeline_id = "LCModel/0.1" - def __init__(self, apiUrl): - pass - def authenticate(self, apiKey): - return True - - def resourceLookup(self, path): - return {'_id': 'fake_id', '_modelType': 'folder'} - - def createFolder(self, parentId, name, reuseExisting=True, **kwargs): - return {'_id': 'fake_id'} - - def addMetadataToFolder(self, folderId, metadata): - return True - - def getFolder(cls, folderId): - print("GUTS: ", cls.pipeline_id) - metadata = { - 'input_settings': { - 'zipped_folder': 'fake_value', - 'basis_file': 'fake_value', - 'signal_file': ['fake_value', 'fake_value'], - 'control_file': ['fake_value']}, - "pipeline_id": cls.pipeline_id, - 'session_name': 'test-VipLauncher', - 'workflows': {}, - "vip_output_dir": "/vip/Home/test-VipLauncher/OUTPUTS" - } - return {'_id': 'fake_id', 'meta': metadata} - - def get(self, path): - return {'_id': 'fake_id'} - - def listFiles(self, folderId): - return [{'_id': 'fake_id'}] - - def listItem(self, folderId): - return {'_id': 'fake_id'} - - @classmethod - def set_pipeline_id(cls, pipeline_id): - print("GRIFITH: ", pipeline_id) - cls.pipeline_id = pipeline_id - - -def mock_girder_client(mocker): - mocker.patch("girder_client.GirderClient", FakeGirderClient) +from mocked_services import mock_vip_api, mock_girder_client, mock_pathlib, mock_os +from FakeGirderClient import FakeGirderClient def get_properties(obj) -> dict: @@ -163,10 +28,11 @@ def get_element(element): @pytest.fixture(scope="function", autouse=True) def setup_teardown_vip_launcher(request, mocker): - # Mock the VIP API + # Mock services mock_vip_api(mocker, "LCModel/0.1") - # Mock the Girder Client mock_girder_client(mocker) + mock_pathlib(mocker) + mock_os(mocker) # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index 62c2e9a..6a78e94 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -4,94 +4,9 @@ from src.vip_client.utils import vip from src.vip_client.classes import VipLauncher +from mocked_services import mock_vip_api, mock_pathlib, mock_os -def mock_vip_api(mocker, pipeline_id): - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_list_pipeline(): - return [ - {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True}, - {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True} - ] - - def fake_set_api_key(api_key): - return True if api_key == "FAKE_KEY" else False - - - def fake_pipeline_def(pipeline): - return { - 'identifier': pipeline_id, - 'name': 'LCModel', - 'description': 'MR spectrosocpy signal quantification software', - 'version': '0.1', - 'parameters': [ - { - 'name': 'zipped_folder', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': 'Archive containing all metabolite & macromolecules in .RAW format', - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'basis_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.basis' containing information & prior ...", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'signal_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.RAW' containing the signal to quantify", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'control_file', - 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', - 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", - 'isOptional': False, - 'isReturnedValue': False - }, - { - 'name': 'script_file', - 'type': 'File', - 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', - 'description': 'Script lauching lcmodel', - 'isOptional': False, 'isReturnedValue': False - } - ], - 'canExecute': True - } - - def fake_init_exec(pipeline, name, inputValues, resultsLocation): - return 'workflow-XXXXXX' - - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} - - mocker.patch("vip_client.utils.vip.exists").return_value = True - mocker.patch("pathlib.Path.exists").return_value = True - mocker.patch("vip_client.utils.vip.upload").return_value = True - mocker.patch("vip_client.utils.vip.download").return_value = True - mocker.patch("os.unlink").return_value = True - mocker.patch("pathlib.Path.unlink").return_value = True - mocker.patch("pathlib.Path.open").side_effect = fake_pathlib_open - mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def - mocker.patch("vip_client.utils.vip.list_pipeline").side_effect = fake_list_pipeline - mocker.patch("vip_client.utils.vip.setApiKey").side_effect = fake_set_api_key - mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec - mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info - def get_properties(obj) -> dict: """ Get session properties as they should be returned by the getter functions @@ -116,6 +31,8 @@ def setup_teardown_vip_launcher(request, mocker): f.write('{}') # Mock the VIP API mock_vip_api(mocker, "LCModel/0.1") + mock_pathlib(mocker) + mock_os(mocker) # Setup code before running the tests in the class print("Handshake with VIP") VipLauncher.init(api_key="FAKE_KEY") @@ -231,6 +148,8 @@ def fake_delete_path(path): mocker.patch("vip_client.utils.vip.delete_path").side_effect = fake_delete_path mock_vip_api(mocker, pipeline_id) + mock_pathlib(mocker) + mock_os(mocker) VipLauncher._BACKUP_LOCATION = backup_location # Create session diff --git a/tests/test_VipSession.py b/tests/test_VipSession.py index f70b3c3..65a1292 100644 --- a/tests/test_VipSession.py +++ b/tests/test_VipSession.py @@ -6,9 +6,7 @@ import pytest_mock from src.vip_client.utils import vip from src.vip_client.classes import VipSession - - -pipeline_id_global = "LCModel/0.1" +from mocked_services import mock_vip_api, mock_pathlib, mock_os def get_properties(obj) -> dict: """ @@ -33,20 +31,11 @@ def setup_teardown_vip_launcher(request, mocker): # Create a buffer file for the backup with open('tmp_data.json', 'w') as f: f.write('{}') + # Mock the VIP API - mocked_list_pipeline = mocker.patch("vip_client.utils.vip.list_pipeline") - mocked_list_pipeline.return_value = [ - {'identifier': 'LCModel/0.1', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True}, - {'identifier': 'CQUEST/0.3', 'name': 'LCModel', 'description': None, - 'version': '0.1', 'parameters': [], 'canExecute': True} - ] - - def fake_set_vip_api_key(api_key): - return True if api_key == "FAKE_KEY" else False - - mocked_set_api_key = mocker.patch("vip_client.utils.vip.setApiKey") - mocked_set_api_key.side_effect = fake_set_vip_api_key + mock_vip_api(mocker, "LCModel/0.1") + mock_pathlib(mocker) + mock_os(mocker) # Setup code before running the tests in the class print("Handshake with VIP") @@ -73,75 +62,27 @@ def cleanup(): ] ) def test_run_and_finish(mocker, nb_runs, pipeline_id): - - removed = False - - def fake_pipeline_def(pipeline): - return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} wf_counter = 0 + s1_init = True - def fake_init_exec(pipeline, name, inputValues, resultsLocation): - nonlocal wf_counter - wf_counter += 1 - return 'workflow-X' + str(wf_counter) - - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} - - def fake_delete_path(path): - nonlocal removed - removed = True - return True - def fake_exists(cls=None, path=None, location="local"): return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - def fake_list_elements(self): - return [{'name': 'element1', 'path': 'path1'}, {'name': 'element2', 'path': 'path2'}] - - def fake_pathlib_iterdir(): - return [Path('tmp_data.json')] - with patch.object(VipSession, '_exists', fake_exists): - - mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") - mocked_pipeline_def.side_effect = fake_pipeline_def - - mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") - mocked_init_exec.side_effect = fake_init_exec - - mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") - mocked_execution_info.side_effect = fake_execution_info - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - - mocked_list_elements = mocker.patch("vip_client.utils.vip.list_elements") - mocked_list_elements.side_effect = fake_list_elements - mocked_pathlib_iterdir = mocker.patch("pathlib.Path.iterdir") - mocked_pathlib_iterdir.side_effect = fake_pathlib_iterdir - + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + nonlocal wf_counter + wf_counter += 1 + return 'workflow-X' + str(wf_counter) + + def fake_is_file(): + nonlocal s1_init + return not s1_init # If s1 is initialized, return False for not using the backup + + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("pathlib.Path.is_file").side_effect = fake_is_file + # Launch a Full Session Run s = VipSession(output_dir="test-VipSession/out", input_dir="test-VipSession/in") s.pipeline_id = pipeline_id @@ -197,130 +138,39 @@ def fake_init_exec(pipeline, name, inputValues, resultsLocation): wf_counter += 1 return 'workflow-X' + str(wf_counter) - def fake_execution_info(workflow_id): - return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} - - def fake_exists(cls=None, path=None, location="local"): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - def fake_list_elements(self): - return [{'name': 'element1', 'path': 'path1'}, {'name': 'element2', 'path': 'path2'}] - - def fake_pathlib_iterdir(): - return [Path('tmp_data.json')] - def fake_is_file(): nonlocal s1_init return not s1_init # If s1 is initialized, return False for not using the backup - with patch.object(VipSession, '_exists', fake_exists): - mocked_pipeline_def = mocker.patch("vip_client.utils.vip.pipeline_def") - mocked_pipeline_def.side_effect = fake_pipeline_def - - mocked_init_exec = mocker.patch("vip_client.utils.vip.init_exec") - mocked_init_exec.side_effect = fake_init_exec - - mocked_execution_info = mocker.patch("vip_client.utils.vip.execution_info") - mocked_execution_info.side_effect = fake_execution_info - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - - mocked_list_elements = mocker.patch("vip_client.utils.vip.list_elements") - mocked_list_elements.side_effect = fake_list_elements - - mocked_pathlib_iterdir = mocker.patch("pathlib.Path.iterdir") - mocked_pathlib_iterdir.side_effect = fake_pathlib_iterdir - - mocked_is_file = mocker.patch("pathlib.Path.is_file") - mocked_is_file.side_effect = fake_is_file - - VipSession._BACKUP_LOCATION = backup_location - # Return if backup is disabled - if VipSession._BACKUP_LOCATION is None: - return - # Create session - s1 = VipSession(output_dir=output_dir) - s1.input_settings = input_settings - s1.pipeline_id = pipeline_id - # Backup - s1._save() - # Set the s1 initialization flag to False - s1_init = False - - # Load backup - s2 = VipSession(output_dir=s1.output_dir) - # Check parameters - assert s2.input_settings == s1.input_settings - assert s2.pipeline_id == s1.pipeline_id - assert s2.output_dir == s1.output_dir - assert s2.workflows == s1.workflows - + # Mock the VipSession method "_exists" + mocker.patch.object(VipSession, '_exists', return_value=True) + + mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def + mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("pathlib.Path.is_file").side_effect = fake_is_file + + VipSession._BACKUP_LOCATION = backup_location + # Return if backup is disabled + if VipSession._BACKUP_LOCATION is None: + return + # Create session + s1 = VipSession(output_dir=output_dir) + s1.input_settings = input_settings + s1.pipeline_id = pipeline_id + # Backup + s1._save() + # Set the s1 initialization flag to False + s1_init = False + # Load backup + s2 = VipSession(output_dir=s1.output_dir) + # Check parameters + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id + assert s2.output_dir == s1.output_dir + assert s2.workflows == s1.workflows def test_properties_interface(mocker): - def fake_exists(path): - return True - - def fake_pathlib_exists(): - return True - - def fake_delete_path(path): - return True - - def fake_upload(local_path, vip_path): - return True - - def fake_download(vip_path, local_path): - return True - - def fake_pathlib_open(mode='r', buffering=-1, encoding=None, errors=None, newline=None): - return io.open('tmp_data.json', mode, buffering, encoding, errors, newline) - - def fake_unlink(self): - return True - - def fake_unlink_linux(): - return True - - mocked_exists = mocker.patch("vip_client.utils.vip.exists") - mocked_exists.side_effect = fake_exists - - mocked_pathlib_exists = mocker.patch("pathlib.Path.exists") - mocked_pathlib_exists.side_effect = fake_pathlib_exists - - mocked_delete_path = mocker.patch("vip_client.utils.vip.delete_path") - mocked_delete_path.side_effect = fake_delete_path - - mocked_upload = mocker.patch("vip_client.utils.vip.upload") - mocked_upload.side_effect = fake_upload - - mocked_download_file = mocker.patch("vip_client.utils.vip.download") - mocked_download_file.side_effect = fake_download - - mocked_pathlib_open = mocker.patch("pathlib.Path.open") - mocked_pathlib_open.side_effect = fake_pathlib_open - - mocked_unlink = mocker.patch("os.unlink") - mocked_unlink.side_effect = fake_unlink - mocked_unlink_linux = mocker.patch("pathlib.Path.unlink") - mocked_unlink_linux.side_effect = fake_unlink_linux - VipSession._BACKUP_LOCATION = "local" # Copy the first session From 58aeb787451c378694e3ccff2c79418800a6830f Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:52:36 +0200 Subject: [PATCH 21/33] Trying to remove warning --- .github/workflows/main.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1a3adf7..8d4e604 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,11 +16,14 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@v2 + with: + node-version: '20' - name: Set up Python 3.9 uses: actions/setup-python@v2 with: python-version: 3.9 + node-version: '20' - name: Install dependencies run: | From 3f970ed9448c31588d6a728bbb3ae28fa83c1fa3 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:58:51 +0200 Subject: [PATCH 22/33] Update main.yml --- .github/workflows/main.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8d4e604..749d04a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -14,16 +14,18 @@ jobs: runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v2 + - name: Set up Node.js + uses: actions/setup-node@v2 with: node-version: '20' + - name: Checkout repository + uses: actions/checkout@v2 + - name: Set up Python 3.9 uses: actions/setup-python@v2 with: python-version: 3.9 - node-version: '20' - name: Install dependencies run: | From 541a7f918f393b3702c9229859f51a8c59fe4554 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Thu, 11 Apr 2024 10:09:38 +0200 Subject: [PATCH 23/33] Update main.yml --- .github/workflows/main.yml | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 749d04a..2684359 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,5 +1,3 @@ -name: Pytest - on: push: branches: @@ -14,16 +12,11 @@ jobs: runs-on: ubuntu-latest steps: - - name: Set up Node.js - uses: actions/setup-node@v2 - with: - node-version: '20' - - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Set up Python 3.9 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: 3.9 From c6ae2e64da624213893cf633e92c1b279b3e7eba Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Thu, 11 Apr 2024 10:39:26 +0200 Subject: [PATCH 24/33] Refactoring to avoid warning and standardize error output --- src/vip_client/classes/VipCI.py | 2 +- src/vip_client/classes/VipLauncher.py | 57 +++++++++++++++++---------- src/vip_client/classes/VipSession.py | 2 +- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/src/vip_client/classes/VipCI.py b/src/vip_client/classes/VipCI.py index b8ad6e4..28e1fb8 100644 --- a/src/vip_client/classes/VipCI.py +++ b/src/vip_client/classes/VipCI.py @@ -212,7 +212,7 @@ def run_session(self, nb_runs=1, refresh_time=30) -> VipCI: 2. Monitors pipeline executions until they are all over; and Adds metadata on Girder output folder. - /!\ This function assumes that all session properties are already set. + |!| This function assumes that all session properties are already set. Optional arguments can be provided: - Increase `nb_runs` to run more than 1 execution at once; - Set `refresh_time` to modify the default refresh time. diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index 48f8476..0fc15be 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -217,7 +217,7 @@ def output_dir(self) -> str: @output_dir.setter def output_dir(self, new_dir: str) -> None: # Call deleter if agument is None - if new_dir is None: + if new_dir is None: del self.vip_output_dir return # Display @@ -602,11 +602,11 @@ def run_session( self, nb_runs=1, refresh_time=30) -> VipLauncher: 1. Launches pipeline executions on VIP; 2. Monitors pipeline executions until they are all over. - /!\ This function assumes that all session properties are already set. + |!| This function assumes that all session properties are already set. Optional arguments can be provided: - Increase `nb_runs` to run more than 1 execution at once; - Set `refresh_time` to modify the default refresh time; - """ + """ # Run the pipeline return ( # 1. Launch `nb_runs` pipeline executions on VIP @@ -1205,7 +1205,7 @@ def _get_exec_infos(cls, workflow_id: str) -> dict: ), # Returned files (filtered information) "outputs": [] if not infos["returnedFiles"] else [ - {"path": value} for value in infos["returnedFiles"]["output_file"] + {"path": value} for value in infos["returnedFiles"].values() ] } # ------------------------------------------------ @@ -1231,12 +1231,10 @@ def _save(self) -> bool: - Displays information unless `_VERBOSE` is False. """ # Return if no-backup mode is activated - print("BACKUP LOCATION: ", self._BACKUP_LOCATION) if self._BACKUP_LOCATION is None: return False # Get session properties session_data = self._data_to_save() - print("CASCO: ", session_data) # Get backup data from the output directory with self._silent_session(): backup_data = self._load_session(location=self._BACKUP_LOCATION) @@ -1244,8 +1242,6 @@ def _save(self) -> bool: if not backup_data: return self._save_session(session_data, location=self._BACKUP_LOCATION) # If the session name is different from backup, raise an error - print("SESSION NAME: ", session_data) - print("BACKUP NAME: ", backup_data) if backup_data["session_name"] != session_data["session_name"]: raise ValueError( f"The backup data have a different session name ('{backup_data['session_name']}').\n" @@ -1284,8 +1280,6 @@ def _load(self) -> bool: self._set(**backup_data) return True # If the backup data do not have the right properties, raise TypeError - print("SESSION DATA", session_data) - print("BACKUP DATA", backup_data) missing_props = set(session_data.keys()) - set(backup_data.keys()) if missing_props: raise TypeError(f"The following properties are missing in the backup data:\n\t{', '.join(missing_props)}") @@ -1590,14 +1584,14 @@ def _check_input_keys(self, input_settings: dict) -> None: # required parameters without a default value { param["name"] for param in self._pipeline_def['parameters'] - if not param["isOptional"] and (param["defaultValue"] == '$input.getDefaultValue()') + if not param["isOptional"] and param["defaultValue"] == None } # current parameters - set(input_settings.keys()) ) # Raise an error if a field is missing if missing_fields: - raise TypeError("Missing input parameter(s) :\n" + ", ".join(missing_fields)) + raise TypeError("Missing input parameter(s): " + ", ".join(missing_fields)) # Check every input parameter is a valid field unknown_fields = ( set(input_settings.keys()) # current parameters @@ -1618,7 +1612,10 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: Prerequisite: input_settings is defined and contains only strings, or lists of strings. """ # Browse the input parameters - for param in self._pipeline_def['parameters'] : + missing_inputs = [] + invalid_chars = [] + wrong_types = [] + for param in self._pipeline_def['parameters']: # Get parameter name name = param['name'] # Skip irrelevant inputs (this should not happen after self._check_input_keys()) @@ -1634,25 +1631,43 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: ) # Check the input has no empty values if not self._is_input_full(value): - raise ValueError( - f"Parameter '{name}' contains an empty value" - ) + missing_inputs.append(name) + continue # Check invalid characters for VIP invalid = self._invalid_chars_for_vip(value) if invalid: - raise ValueError( - f"Parameter '{name}' contains some invalid character(s): {', '.join(invalid)}" - ) - # If input is a File, check file(s) existence + invalid_chars.append((name, invalid)) + continue + # If input is a File, check file(s) existence if param["type"] == "File": # Ensure every file exists at `location` + print("Checking file existence for", name) missing_file = self._first_missing_file(value, location) + print("Missing file", missing_file) if missing_file: raise FileNotFoundError( - f"Parameter '{name}': The following file is missing in the {location.upper()} file system:\n\t{missing_file}" + f"Parameter '{name}': The following file is missing in the {location.upper()} file system: {missing_file}" ) + if param["type"] == "Boolean": + if value not in ["true", "false"]: + wrong_types.append(name) + continue # Check other input formats ? else: pass # TODO + if missing_inputs: + raise ValueError( + f"Missing input value(s) for parameter(s): {', '.join(sorted(missing_inputs))}" + ) + if invalid_chars: + raise ValueError( + f"Invalid character(s) in input value(s) for parameter(s): {', '.join(sorted(invalid_chars))}" + ) + if wrong_types: + raise ValueError( + f"Wrong type(s) for parameter(s): {', '.join(sorted(wrong_types))}" + ) + # raise RuntimeError("An error should have been raised before this point.") + # ------------------------------------------------ # Function to look for empty values diff --git a/src/vip_client/classes/VipSession.py b/src/vip_client/classes/VipSession.py index c2310ec..f87f530 100644 --- a/src/vip_client/classes/VipSession.py +++ b/src/vip_client/classes/VipSession.py @@ -540,7 +540,7 @@ def run_session( 3. Monitors pipeline executions until they are all over; 4. Downloads execution results from VIP. - /!\ This method assumes that all session properties are already set. + |!| This method assumes that all session properties are already set. Optional arguments can still be provided: - Set `update_files` to False to avoid checking the input data on VIP; - Increase `nb_runs` to run more than 1 execution at once; From 5221d2cda2e7eda333e7322c674a154065c87bfe Mon Sep 17 00:00:00 2001 From: Hippolyte Blot <72436409+hippolyteblot@users.noreply.github.com> Date: Tue, 16 Apr 2024 17:39:15 +0200 Subject: [PATCH 25/33] Update main.yml --- .github/workflows/main.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2684359..bafb1bb 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,6 +11,9 @@ jobs: test: runs-on: ubuntu-latest + env: + VIP_API_KEY: ${{ secrets.VIP_API_KEY }} + steps: - name: Checkout repository uses: actions/checkout@v4 From c39cd19eef14485adf44d7dd051c69b8dfecb794 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Tue, 16 Apr 2024 17:46:51 +0200 Subject: [PATCH 26/33] Tests for vip utils --- .gitignore | 2 + src/vip_client/classes/VipLauncher.py | 10 +- src/vip_client/classes/VipSession.py | 12 ++- src/vip_client/utils/vip.py | 7 +- tests/data/file.txt | 1 + tests/data/file_copy.txt | 1 + tests/mocked_services.py | 10 +- tests/test_VipCI.py | 33 ++++--- tests/test_VipLauncher.py | 20 ++-- tests/test_VipSession.py | 62 +++++++----- tests/test_global.py | 135 ++++++++++++++++++++++++++ tests/test_vip_utils.py | 135 ++++++++++++++++++++++++++ 12 files changed, 365 insertions(+), 63 deletions(-) create mode 100644 tests/data/file.txt create mode 100644 tests/data/file_copy.txt create mode 100644 tests/test_global.py create mode 100644 tests/test_vip_utils.py diff --git a/.gitignore b/.gitignore index 45e2d8e..4cb4b2b 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ examples/old publish.sh dist/ +.env +test_pipedef.py \ No newline at end of file diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index 0fc15be..f995277 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -137,6 +137,7 @@ def input_settings(self) -> dict: @input_settings.setter def input_settings(self, input_settings: dict): + print("Setting input settings") # Call deleter if agument is None if input_settings is None: del self.input_settings @@ -1193,6 +1194,7 @@ def _get_exec_infos(cls, workflow_id: str) -> dict: # Get execution infos try : infos = vip.execution_info(workflow_id) + print(infos) except RuntimeError as vip_error: cls._handle_vip_error(vip_error) # Return filtered information @@ -1584,14 +1586,14 @@ def _check_input_keys(self, input_settings: dict) -> None: # required parameters without a default value { param["name"] for param in self._pipeline_def['parameters'] - if not param["isOptional"] and param["defaultValue"] == None + if not param["isOptional"] and param["defaultValue"] is None } # current parameters - set(input_settings.keys()) ) # Raise an error if a field is missing if missing_fields: - raise TypeError("Missing input parameter(s): " + ", ".join(missing_fields)) + raise AttributeError("Missing input parameter(s): " + ", ".join(sorted(missing_fields))) # Check every input parameter is a valid field unknown_fields = ( set(input_settings.keys()) # current parameters @@ -1616,6 +1618,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: invalid_chars = [] wrong_types = [] for param in self._pipeline_def['parameters']: + print("CHECKING PARAMETER: ", param['name']) # Get parameter name name = param['name'] # Skip irrelevant inputs (this should not happen after self._check_input_keys()) @@ -1623,6 +1626,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: continue # Get input value value = input_settings[name] + print("VALUE: ", value) # `request` will send only strings if not self._isinstance(value, str): # This should not happen raise ValueError( # Parameter could not be parsed correctly @@ -1641,9 +1645,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: # If input is a File, check file(s) existence if param["type"] == "File": # Ensure every file exists at `location` - print("Checking file existence for", name) missing_file = self._first_missing_file(value, location) - print("Missing file", missing_file) if missing_file: raise FileNotFoundError( f"Parameter '{name}': The following file is missing in the {location.upper()} file system: {missing_file}" diff --git a/src/vip_client/classes/VipSession.py b/src/vip_client/classes/VipSession.py index f87f530..b122e7d 100644 --- a/src/vip_client/classes/VipSession.py +++ b/src/vip_client/classes/VipSession.py @@ -1073,8 +1073,8 @@ def parse_value(input): # Case: single input, string or path-like elif isinstance(input, (str, os.PathLike)): # Case: VIP path - if str(input).startswith(self._SERVER_PATH_PREFIX): # PurePath.is_relative_to() is unavailable for Python <3.9 - if self._is_defined('_vip_input_dir'): + if str(input).startswith(self._SERVER_PATH_PREFIX) and len(str(input)) > 0: # PurePath.is_relative_to() is unavailable for Python <3.9 + if self._is_defined('_vip_input_dir'): input_dir = self._vip_input_dir input_path = PurePosixPath(input) else: # Return input if `_vip_input_dir` is unset @@ -1089,8 +1089,12 @@ def parse_value(input): return input # Return the part of `input_path` that is relative to `input_dir` (if relevant) try: # No condition since PurePath.is_relative_to() is unavailable for Python <3.9 - return PurePosixPath( # Force Posix flavor to avoid conflicts with Windows paths when checking equality - input_path.relative_to(input_dir)) # Relative part of `input_path` + if len(str(input)) > 0: + print("ALICE: ", PurePosixPath(input_path.relative_to(input_dir))) + x=1/0 + return PurePosixPath( # Force Posix flavor to avoid conflicts with Windows paths when checking equality + input_path.relative_to(input_dir)) # Relative part of `input_path` + return input except ValueError: # This is the case when no relative part could be found return input diff --git a/src/vip_client/utils/vip.py b/src/vip_client/utils/vip.py index a4b903a..acc3d47 100644 --- a/src/vip_client/utils/vip.py +++ b/src/vip_client/utils/vip.py @@ -80,7 +80,6 @@ def setApiKey(value) -> bool: Return True is correct apikey, False otherwise. Raise an error if an other problems occured """ - print("Were in the real one") url = __PREFIX + 'plateform' head_test = { 'apikey': value, @@ -142,6 +141,7 @@ def create_dir(path)->bool: """ Return True if done, False otherwise """ + path = path + '/' if path[-1] != '/' else path url = __PREFIX + 'path' + path rq = SESSION.put(url, headers=__headers) try: @@ -397,10 +397,11 @@ def get_exec_results(exec_id, timeout: int=None) -> str: # ----------------------------------------------------------------------------- def kill_execution(exec_id, deleteFiles=False) -> bool: - url = __PREFIX + 'executions/' + exec_id + url = __PREFIX + 'executions/' + exec_id + '/kill' if deleteFiles: url += '?deleteFiles=true' - rq = SESSION.delete(url, headers=__headers) + rq = SESSION.put(url, headers=__headers) + print("req is: ", url) try: manage_errors(rq) except RuntimeError: diff --git a/tests/data/file.txt b/tests/data/file.txt new file mode 100644 index 0000000..bad8398 --- /dev/null +++ b/tests/data/file.txt @@ -0,0 +1 @@ +This is juste a test file for upload \ No newline at end of file diff --git a/tests/data/file_copy.txt b/tests/data/file_copy.txt new file mode 100644 index 0000000..bad8398 --- /dev/null +++ b/tests/data/file_copy.txt @@ -0,0 +1 @@ +This is juste a test file for upload \ No newline at end of file diff --git a/tests/mocked_services.py b/tests/mocked_services.py index 57f6ce1..b8e658f 100644 --- a/tests/mocked_services.py +++ b/tests/mocked_services.py @@ -25,7 +25,7 @@ def fake_pipeline_def(pipeline): { 'name': 'zipped_folder', 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', + 'defaultValue': None, 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False @@ -33,7 +33,7 @@ def fake_pipeline_def(pipeline): { 'name': 'basis_file', 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', + 'defaultValue': None, 'description': "Text file with extension '.basis' containing information & prior ...", 'isOptional': False, 'isReturnedValue': False @@ -41,7 +41,7 @@ def fake_pipeline_def(pipeline): { 'name': 'signal_file', 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', + 'defaultValue': None, 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False @@ -49,9 +49,9 @@ def fake_pipeline_def(pipeline): { 'name': 'control_file', 'type': 'File', - 'defaultValue': '$input.getDefaultValue()', + 'defaultValue': None, 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", - 'isOptional': False, + 'isOptional': True, 'isReturnedValue': False }, { diff --git a/tests/test_VipCI.py b/tests/test_VipCI.py index 4335439..0c07205 100644 --- a/tests/test_VipCI.py +++ b/tests/test_VipCI.py @@ -1,4 +1,5 @@ import io +from urllib.error import HTTPError import pytest from pathlib import * @@ -66,14 +67,23 @@ def test_run_and_finish(mocker, nb_runs, pipeline_id): FakeGirderClient.set_pipeline_id(pipeline_id) wf_counter = 0 + processing = True def fake_init_exec(pipeline, name, inputValues, resultsLocation): nonlocal wf_counter wf_counter += 1 return f'workflow-{wf_counter}' + + def fake_execution_info(workflow_id): + nonlocal processing + if not processing: + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + processing -= 1 + return {'status': 'Running', 'returnedFiles': [], 'startDate': 0} # Re patch the init_exec function to update the workflow counter mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info # Launch a Full Session Run s = VipCI() @@ -122,23 +132,24 @@ def fake_init_exec(pipeline, name, inputValues, resultsLocation): def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): VipCI._BACKUP_LOCATION = backup_location - # Return if backup is disabled - if VipCI._BACKUP_LOCATION is None: - return + # Create session - s1 = VipCI() - s1.input_settings = input_settings - s1.pipeline_id = pipeline_id + s1 = VipCI(pipeline_id=pipeline_id, input_settings=input_settings) s1.output_dir = output_dir - # Backup - s1._save() + + assert s1._save() is not (VipCI._BACKUP_LOCATION is None) # Return False if no backup location + # Load backup s2 = VipCI(output_dir=s1.output_dir) # Check parameters - assert s2.input_settings == s1.input_settings - assert s2.pipeline_id == s1.pipeline_id assert s2.output_dir == s1.output_dir - assert s2.workflows == s1.workflows + if VipCI._BACKUP_LOCATION is None: + assert not s2._load() + assert s2.input_settings != s1.input_settings + assert s2.pipeline_id != s1.pipeline_id + else: + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id def test_properties_interface(mocker): diff --git a/tests/test_VipLauncher.py b/tests/test_VipLauncher.py index 6a78e94..dabe1d6 100644 --- a/tests/test_VipLauncher.py +++ b/tests/test_VipLauncher.py @@ -157,21 +157,21 @@ def fake_delete_path(path): s1.input_settings = input_settings s1.pipeline_id = pipeline_id s1.output_dir = output_dir - # Backup - s1._save() + + # Return if backup is disabled + assert s1._save() is not (VipLauncher._BACKUP_LOCATION is None) # Return False if no backup location + # Load backup s2 = VipLauncher(output_dir=s1.output_dir) # Check parameters - if backup_location is not None: + assert s2.output_dir == s1.output_dir + if VipLauncher._BACKUP_LOCATION is None: + assert not s2._load() + assert s2.input_settings != s1.input_settings + assert s2.pipeline_id != s1.pipeline_id + else: assert s2.input_settings == s1.input_settings assert s2.pipeline_id == s1.pipeline_id - assert s2.output_dir == s1.output_dir - assert s2.workflows == s1.workflows - else: - assert s2.input_settings == None - assert s2.pipeline_id is None - assert s2.output_dir == s1.output_dir - assert s2.workflows == {} def test_properties_interface(mocker): diff --git a/tests/test_VipSession.py b/tests/test_VipSession.py index 65a1292..7caa9aa 100644 --- a/tests/test_VipSession.py +++ b/tests/test_VipSession.py @@ -64,35 +64,45 @@ def cleanup(): def test_run_and_finish(mocker, nb_runs, pipeline_id): wf_counter = 0 - s1_init = True + processing = 100 + def fake_exists(cls=None, path=None, location="local"): return True with patch.object(VipSession, '_exists', fake_exists): - + def fake_init_exec(pipeline, name, inputValues, resultsLocation): + print("LUCIE: ", inputValues) + x=1/0 nonlocal wf_counter wf_counter += 1 return 'workflow-X' + str(wf_counter) - - def fake_is_file(): - nonlocal s1_init - return not s1_init # If s1 is initialized, return False for not using the backup - + + def fake_execution_info(workflow_id): + nonlocal processing + if not processing: + return {'status': 'Finished', 'returnedFiles': [], 'startDate': 0} + processing -= 1 + return {'status': 'Running', 'returnedFiles': [], 'startDate': 0} + + + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info + + + mocker.patch("vip_client.utils.vip.execution_info").side_effect = fake_execution_info mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec - mocker.patch("pathlib.Path.is_file").side_effect = fake_is_file # Launch a Full Session Run s = VipSession(output_dir="test-VipSession/out", input_dir="test-VipSession/in") s.pipeline_id = pipeline_id s.input_settings = { - "zipped_folder": 'fake_value', - "basis_file": 'fake_value', - "signal_file": ['fake_value', 'fake_value'], - "control_file": ['fake_value'] + "zipped_folder": 'path/on/host/input.zip', + "basis_file": 'path/on/host/fake_value', + "signal_file": ['path/on/host/fake_value', 'path/on/host/fake_value'], + "control_file": ['path/on/host/fake_value'] } - s.run_session(nb_runs=nb_runs) + s.run_session(nb_runs=nb_runs, refresh_time=0) # Check the Results assert s.workflows assert len(s.workflows) == nb_runs @@ -127,16 +137,11 @@ def fake_is_file(): ] ) def test_backup(mocker, backup_location, input_settings, pipeline_id, output_dir): + def fake_pipeline_def(pipeline): return {'identifier': pipeline_id, 'name': 'LCModel', 'description': 'MR spectrosocpy signal quantification software', 'version': '0.1', 'parameters': [{'name': 'zipped_folder', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': 'Archive containing all metabolite & macromolecules in .RAW format', 'isOptional': False, 'isReturnedValue': False}, {'name': 'basis_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.basis' containing information & prior knowledge about the metabolites used for signal fit", 'isOptional': False, 'isReturnedValue': False}, {'name': 'signal_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.RAW' containing the signal to quantify", 'isOptional': False, 'isReturnedValue': False}, {'name': 'control_file', 'type': 'File', 'defaultValue': '$input.getDefaultValue()', 'description': "Text file with extension '.control' setting up constraints, options and prior knowledge used in LCModel algorithm", 'isOptional': False, 'isReturnedValue': False}, {'name': 'script_file', 'type': 'File', 'defaultValue': '/vip/ReproVIP (group)/LCModel/run-lcmodel.sh', 'description': 'Script lauching lcmodel', 'isOptional': False, 'isReturnedValue': False}], 'canExecute': True} - wf_counter = 0 s1_init = True - - def fake_init_exec(pipeline, name, inputValues, resultsLocation): - nonlocal wf_counter - wf_counter += 1 - return 'workflow-X' + str(wf_counter) def fake_is_file(): nonlocal s1_init @@ -146,17 +151,18 @@ def fake_is_file(): mocker.patch.object(VipSession, '_exists', return_value=True) mocker.patch("vip_client.utils.vip.pipeline_def").side_effect = fake_pipeline_def - mocker.patch("vip_client.utils.vip.init_exec").side_effect = fake_init_exec mocker.patch("pathlib.Path.is_file").side_effect = fake_is_file VipSession._BACKUP_LOCATION = backup_location # Return if backup is disabled - if VipSession._BACKUP_LOCATION is None: - return + # Create session s1 = VipSession(output_dir=output_dir) s1.input_settings = input_settings - s1.pipeline_id = pipeline_id + s1.pipeline_id = pipeline_id + + assert s1._save() is not (VipSession._BACKUP_LOCATION is None) # Return False if no backup location + # Backup s1._save() # Set the s1 initialization flag to False @@ -164,10 +170,14 @@ def fake_is_file(): # Load backup s2 = VipSession(output_dir=s1.output_dir) # Check parameters - assert s2.input_settings == s1.input_settings - assert s2.pipeline_id == s1.pipeline_id assert s2.output_dir == s1.output_dir - assert s2.workflows == s1.workflows + if VipSession._BACKUP_LOCATION is None: + assert not s2._load() + assert s2.input_settings != s1.input_settings + assert s2.pipeline_id != s1.pipeline_id + else: + assert s2.input_settings == s1.input_settings + assert s2.pipeline_id == s1.pipeline_id def test_properties_interface(mocker): diff --git a/tests/test_global.py b/tests/test_global.py new file mode 100644 index 0000000..b593a10 --- /dev/null +++ b/tests/test_global.py @@ -0,0 +1,135 @@ +from src.vip_client.classes import VipSession, VipLauncher, VipCI +from mocked_services import mock_vip_api, mock_pathlib, mock_os, mock_girder_client +import pytest + + +test_cases_missing_input_fields = [ + { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'] + }, + { + "zipped_folder": 'fake_value1', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": 'fake_value5' + }, + { + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + }, + { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + }, + { + } +] + +# VipSession trouve pas que l'input est vide quand on a '' et non [] +test_cases_missing_input_values = [ + { + "zipped_folder": 'fake_value1', + "basis_file": '', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": 'fake_value5' + }, + { + "zipped_folder": 'fake_value1', + "basis_file": 'fake_value2', + "signal_file": [], + "control_file": 'fake_value5' + }, + { + "zipped_folder": '', + "basis_file": 'fake_value2', + "signal_file": ['fake_value3', 'fake_value4'], + "control_file": 'fake_value5' + } +] + +test_cases_missing_input_fields = [(input_settings, tested_class) for input_settings in test_cases_missing_input_fields for tested_class in [VipSession, VipLauncher, VipCI]] +test_cases_missing_input_values = [(input_settings, tested_class) for input_settings in test_cases_missing_input_values for tested_class in [VipSession, VipLauncher, VipCI]] + +@pytest.fixture(scope="function", autouse=True) +def setup_teardown_vip_launcher(request, mocker): + # Mock the VIP API + mock_vip_api(mocker, "LCModel/0.1") + mock_pathlib(mocker) + mock_os(mocker) + mock_girder_client(mocker) + + # Setup code before running the tests in the class + print("Handshake with VIP") + VipSession.init(api_key="FAKE_KEY") + VipLauncher.init(api_key="FAKE_KEY") + VipCI.init(vip_key="FAKE_KEY", girder_key="FAKE_KEY") + print("Setup done") + + +@pytest.mark.parametrize( + "input_settings, tested_class", test_cases_missing_input_fields +) +def test_missing_input_settings(input_settings, tested_class): + + VipCI._BACKUP_LOCATION = None + + # Copy the first session + s = VipCI() + s.pipeline_id = "LCModel/0.1" + s.output_dir = "/path/to/output" + s.input_settings = input_settings + + needed_fields = ["zipped_folder", "basis_file", "signal_file"] + missing_fields = [field for field in needed_fields if field not in input_settings] + + if not missing_fields: + s.run_session() + return + + # catch the exception message + with pytest.raises(AttributeError) as e: + s.run_session() + assert str(e.value) == "Missing input parameter(s): " + ", ".join(sorted(missing_fields)) + + + +@pytest.mark.parametrize( + "input_settings, tested_class", test_cases_missing_input_values +) +def test_missing_input_values(mocker, input_settings, tested_class): + + def is_input_full(value): + """ + Returns False if `value` contains an empty string or list. + """ + if isinstance(value, list) and all([isinstance(v, str) for v in value]): # Case: list of strings + return all([(len(v) > 0) for v in value]) + elif isinstance(value, (str, list)): # Case: list or string + return (len(value) > 0) + else: # Case: other + return True + + tested_class._BACKUP_LOCATION = None + + mocker.patch("pathlib.Path.is_file").return_value = True + + # Copy the first session + s = tested_class() + s.pipeline_id = "LCModel/0.1" + if tested_class == VipSession: + mocker.patch.object(VipSession, '_exists', return_value=True) + s.input_dir = "." + else: + s.output_dir = "/path/to/output" + s.input_settings = input_settings + + missing_fields = [field for field in input_settings if not is_input_full(input_settings[field])] + + if not missing_fields: + s.run_session() + return + # Catch the exception message + with pytest.raises(ValueError) as e: + s.run_session() + assert str(e.value) == "Missing input value(s) for parameter(s): " + ", ".join(sorted(missing_fields)) \ No newline at end of file diff --git a/tests/test_vip_utils.py b/tests/test_vip_utils.py new file mode 100644 index 0000000..6ba3b8d --- /dev/null +++ b/tests/test_vip_utils.py @@ -0,0 +1,135 @@ +import pytest +import time +import os + +from src.vip_client.utils.vip import * + +BASE_PATH_VIP = '/vip/Home/API/client_tests/' +BASE_PATH_LOCAL = 'tests/data/' + + +def compare_files(file1, file2): + with open(file1, 'r') as f1, open(file2, 'r') as f2: + return f1.read() == f2.read() + +@pytest.fixture(scope="function", autouse=True) +def setup_teardown_vip_launcher(request, mocker): + assert setApiKey(os.environ['VIP_API_KEY']) + assert new_session() + if not create_dir(BASE_PATH_VIP): + raise Exception("Error creating directory") + assert is_dir(BASE_PATH_VIP) + +@pytest.fixture(scope="function", autouse=True) +def cleanup(): + yield + assert delete_path(BASE_PATH_VIP) + + +def test_upload_download(): + assert upload(BASE_PATH_LOCAL + 'file.txt', BASE_PATH_VIP + 'file.txt') + assert exists(BASE_PATH_VIP + 'file.txt') + assert download(BASE_PATH_VIP + 'file.txt', BASE_PATH_LOCAL + 'file_copy.txt') + assert compare_files(BASE_PATH_LOCAL + 'file.txt', BASE_PATH_LOCAL + 'file_copy.txt') + assert delete_path(BASE_PATH_VIP + 'file.txt') + + +def atest_init_exec(): + input_values = { + 'mand_text': 'value1', + 'mand_file': '/vip/Home/file.txt', + } + exec_id = init_exec('Fake_app_test/0.1', resultsLocation=BASE_PATH_VIP, inputValues=input_values, name='test_init_exec') + exec_info = execution_info(exec_id) + assert exec_info['status'] == 'Running' + + +def test_kill_exec(): + input_values = { + 'mand_text': 'value1', + 'mand_file': '/vip/Home/file.txt', + 'mand_time': 100, + } + exec_id = init_exec('Fake_app_test_delay/0.1', resultsLocation=BASE_PATH_VIP, inputValues=input_values, name='test_kill_exec') + counter = 0 + while execution_info(exec_id)['status'] != 'Running': + time.sleep(5) + if counter > 12: + raise Exception("Execution not started after 60 seconds") + counter += 1 + assert kill_execution(exec_id, deleteFiles=True) + counter = 0 + while execution_info(exec_id)['status'] != 'Killed': + time.sleep(5) + if counter > 12: + raise Exception("Execution not killed after 60 seconds") + +def test_get_exec_stdout(): + input_values = { + 'mand_text': 'value1', + 'mand_file': '/vip/Home/file.txt', + } + exec_id = init_exec('Fake_app_test/0.1', resultsLocation=BASE_PATH_VIP, inputValues=input_values, name='test_get_exec_stdout') + counter = 0 + while execution_info(exec_id)['status'] != 'Finished': + time.sleep(5) + if counter > 12: + raise Exception("Execution not ended after 60 seconds") + counter += 1 + stdout = get_exec_stdout(exec_id) + assert isinstance(stdout, str) + + +def test_get_exec_results(): + input_values = { + 'mand_text': 'value1', + 'mand_file': '/vip/Home/file.txt', + } + exec_id = init_exec('Fake_app_test/0.1', resultsLocation=BASE_PATH_VIP, inputValues=input_values, name='test_get_exec_results') + counter = 0 + while execution_info(exec_id)['status'] != 'Finished': + time.sleep(5) + if counter > 12: + raise Exception("Execution not ended after 60 seconds") + counter += 1 + results = get_exec_results(exec_id) + # assert that this is a list of dictionaries + print(results) + assert isinstance(results, list) + for r in results: + assert isinstance(r, dict) + assert 'executionId' in r + assert 'path' in r + + +def test_list_pipeline(): + pipelines = list_pipeline() + # assert that this is a list of dictionaries + assert isinstance(pipelines, list) + for p in pipelines: + assert isinstance(p, dict) + assert 'identifier' in p + assert 'name' in p + assert 'version' in p + assert 'parameters' in p + assert 'canExecute' in p + assert 'description' in p + + +def test_pipeline_def(): + pipeline = pipeline_def('CQUEST/0.3') + assert isinstance(pipeline, dict) + assert 'identifier' in pipeline + assert 'name' in pipeline + assert 'version' in pipeline + assert 'parameters' in pipeline + assert 'canExecute' in pipeline + assert 'description' in pipeline + + +def test_platform_info(): + info = platform_info() + print(info) + assert isinstance(info, dict) + assert info['platformName'] == 'VIP' + \ No newline at end of file From 046fbb87c657b6c500584584da7e655357e7e30a Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 17 Apr 2024 09:18:56 +0200 Subject: [PATCH 27/33] Fix error division by 0 --- src/vip_client/classes/VipSession.py | 2 -- tests/test_VipSession.py | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/vip_client/classes/VipSession.py b/src/vip_client/classes/VipSession.py index b122e7d..d0bf701 100644 --- a/src/vip_client/classes/VipSession.py +++ b/src/vip_client/classes/VipSession.py @@ -1090,8 +1090,6 @@ def parse_value(input): # Return the part of `input_path` that is relative to `input_dir` (if relevant) try: # No condition since PurePath.is_relative_to() is unavailable for Python <3.9 if len(str(input)) > 0: - print("ALICE: ", PurePosixPath(input_path.relative_to(input_dir))) - x=1/0 return PurePosixPath( # Force Posix flavor to avoid conflicts with Windows paths when checking equality input_path.relative_to(input_dir)) # Relative part of `input_path` return input diff --git a/tests/test_VipSession.py b/tests/test_VipSession.py index 7caa9aa..cc4fdaf 100644 --- a/tests/test_VipSession.py +++ b/tests/test_VipSession.py @@ -73,8 +73,6 @@ def fake_exists(cls=None, path=None, location="local"): with patch.object(VipSession, '_exists', fake_exists): def fake_init_exec(pipeline, name, inputValues, resultsLocation): - print("LUCIE: ", inputValues) - x=1/0 nonlocal wf_counter wf_counter += 1 return 'workflow-X' + str(wf_counter) From 33093761d7934648d2832ea1762bf05f2796b603 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 17 Apr 2024 09:31:18 +0200 Subject: [PATCH 28/33] Removing print --- tests/test_vip_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_vip_utils.py b/tests/test_vip_utils.py index 6ba3b8d..6d7ac2b 100644 --- a/tests/test_vip_utils.py +++ b/tests/test_vip_utils.py @@ -94,7 +94,6 @@ def test_get_exec_results(): counter += 1 results = get_exec_results(exec_id) # assert that this is a list of dictionaries - print(results) assert isinstance(results, list) for r in results: assert isinstance(r, dict) @@ -129,7 +128,6 @@ def test_pipeline_def(): def test_platform_info(): info = platform_info() - print(info) assert isinstance(info, dict) assert info['platformName'] == 'VIP' \ No newline at end of file From 30ddfe17e08a7b2546701cab3844b0d6b88d6b35 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 17 Apr 2024 09:40:17 +0200 Subject: [PATCH 29/33] Adding a delay for creating the tests dir --- tests/test_vip_utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_vip_utils.py b/tests/test_vip_utils.py index 6d7ac2b..dbae33d 100644 --- a/tests/test_vip_utils.py +++ b/tests/test_vip_utils.py @@ -18,6 +18,12 @@ def setup_teardown_vip_launcher(request, mocker): assert new_session() if not create_dir(BASE_PATH_VIP): raise Exception("Error creating directory") + counter = 0 + while not exists(BASE_PATH_VIP): + time.sleep(5) + if counter > 12: + raise Exception("Directory not created after 60 seconds") + counter += 1 assert is_dir(BASE_PATH_VIP) @pytest.fixture(scope="function", autouse=True) From 74feba166185f0fb42dd2aee44cff8958f99bad0 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Wed, 17 Apr 2024 13:45:23 +0200 Subject: [PATCH 30/33] Fix vip utils tests --- src/vip_client/utils/vip.py | 1 - tests/test_vip_utils.py | 15 +++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/vip_client/utils/vip.py b/src/vip_client/utils/vip.py index acc3d47..f8f144e 100644 --- a/src/vip_client/utils/vip.py +++ b/src/vip_client/utils/vip.py @@ -401,7 +401,6 @@ def kill_execution(exec_id, deleteFiles=False) -> bool: if deleteFiles: url += '?deleteFiles=true' rq = SESSION.put(url, headers=__headers) - print("req is: ", url) try: manage_errors(rq) except RuntimeError: diff --git a/tests/test_vip_utils.py b/tests/test_vip_utils.py index dbae33d..af8468a 100644 --- a/tests/test_vip_utils.py +++ b/tests/test_vip_utils.py @@ -12,8 +12,8 @@ def compare_files(file1, file2): with open(file1, 'r') as f1, open(file2, 'r') as f2: return f1.read() == f2.read() -@pytest.fixture(scope="function", autouse=True) -def setup_teardown_vip_launcher(request, mocker): +@pytest.fixture(scope="session", autouse=True) +def setup_teardown_vip_launcher(): assert setApiKey(os.environ['VIP_API_KEY']) assert new_session() if not create_dir(BASE_PATH_VIP): @@ -21,16 +21,11 @@ def setup_teardown_vip_launcher(request, mocker): counter = 0 while not exists(BASE_PATH_VIP): time.sleep(5) - if counter > 12: - raise Exception("Directory not created after 60 seconds") + if counter > 24: + raise Exception("Directory not created after 120 seconds") counter += 1 - assert is_dir(BASE_PATH_VIP) - -@pytest.fixture(scope="function", autouse=True) -def cleanup(): yield assert delete_path(BASE_PATH_VIP) - def test_upload_download(): assert upload(BASE_PATH_LOCAL + 'file.txt', BASE_PATH_VIP + 'file.txt') @@ -40,7 +35,7 @@ def test_upload_download(): assert delete_path(BASE_PATH_VIP + 'file.txt') -def atest_init_exec(): +def test_init_exec(): input_values = { 'mand_text': 'value1', 'mand_file': '/vip/Home/file.txt', From 73c394253ee878f15009044f8965225825b56d1d Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Thu, 25 Apr 2024 17:19:11 +0200 Subject: [PATCH 31/33] Fix tests and support for executions with only default values --- src/vip_client/classes/VipLauncher.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index f995277..0324fce 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -137,7 +137,6 @@ def input_settings(self) -> dict: @input_settings.setter def input_settings(self, input_settings: dict): - print("Setting input settings") # Call deleter if agument is None if input_settings is None: del self.input_settings @@ -490,10 +489,7 @@ def launch_pipeline( self._print("OK") # Check the input parameters self._print("Input settings: ", end="", flush=True) - # Check existence - if not self._is_defined("_input_settings"): - raise TypeError("Please provide input parameters for Session: %s" %self._session_name) - # Check content + # Check content self._check_input_settings(location=self._SERVER_NAME) self._print("OK") # End parameters checks @@ -603,11 +599,11 @@ def run_session( self, nb_runs=1, refresh_time=30) -> VipLauncher: 1. Launches pipeline executions on VIP; 2. Monitors pipeline executions until they are all over. - |!| This function assumes that all session properties are already set. + /!\ This function assumes that all session properties are already set. Optional arguments can be provided: - Increase `nb_runs` to run more than 1 execution at once; - Set `refresh_time` to modify the default refresh time; - """ + """ # Run the pipeline return ( # 1. Launch `nb_runs` pipeline executions on VIP @@ -1194,7 +1190,6 @@ def _get_exec_infos(cls, workflow_id: str) -> dict: # Get execution infos try : infos = vip.execution_info(workflow_id) - print(infos) except RuntimeError as vip_error: cls._handle_vip_error(vip_error) # Return filtered information @@ -1564,7 +1559,7 @@ def _check_input_settings(self, input_settings: dict=None, location: str=None) - if self._is_defined("_input_settings"): input_settings = self._get_input_settings(location) else: - raise AttributeError("Input settings are missing") + input_settings = {} # Check the pipeline identifier if not self._is_defined("_pipeline_id"): raise AttributeError("Input settings could not be checked without a pipeline identifier.") @@ -1586,7 +1581,7 @@ def _check_input_keys(self, input_settings: dict) -> None: # required parameters without a default value { param["name"] for param in self._pipeline_def['parameters'] - if not param["isOptional"] and param["defaultValue"] is None + if not param["isOptional"] and param["defaultValue"] == None } # current parameters - set(input_settings.keys()) @@ -1618,7 +1613,6 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: invalid_chars = [] wrong_types = [] for param in self._pipeline_def['parameters']: - print("CHECKING PARAMETER: ", param['name']) # Get parameter name name = param['name'] # Skip irrelevant inputs (this should not happen after self._check_input_keys()) @@ -1626,7 +1620,6 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: continue # Get input value value = input_settings[name] - print("VALUE: ", value) # `request` will send only strings if not self._isinstance(value, str): # This should not happen raise ValueError( # Parameter could not be parsed correctly From 24f25fd793d5683f6b657bf0ec9717e5c9205581 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Fri, 21 Jun 2024 15:21:49 +0200 Subject: [PATCH 32/33] fix --- .github/workflows/main.yml | 2 +- src/vip_client/classes/VipLauncher.py | 27 +++++++++++++-------------- tests/test_global.py | 12 +++++------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bafb1bb..a7a9980 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -5,7 +5,7 @@ on: - develop pull_request: branches: - - main + - develop jobs: test: diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index 0324fce..4e92d0f 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -148,6 +148,7 @@ def input_settings(self, input_settings: dict): raise TypeError("`input_settings` should be a dictionary") # Check if each input can be converted to a string with valid characters for VIP for param, value in input_settings.items(): + invalid = self._is_input_full(value) invalid = self._invalid_chars_for_vip(value) # if not (set(invalid) <= {'\\'}): # '\\' is OK for Windows paths #EDIT: Corrected in _invalid_chars_for_vip if invalid: @@ -1610,8 +1611,8 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: """ # Browse the input parameters missing_inputs = [] - invalid_chars = [] - wrong_types = [] + invalid_chars_inputs = [] + wrong_type_inputs = [] for param in self._pipeline_def['parameters']: # Get parameter name name = param['name'] @@ -1633,7 +1634,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: # Check invalid characters for VIP invalid = self._invalid_chars_for_vip(value) if invalid: - invalid_chars.append((name, invalid)) + invalid_chars_inputs.append((name, invalid)) continue # If input is a File, check file(s) existence if param["type"] == "File": @@ -1645,7 +1646,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: ) if param["type"] == "Boolean": if value not in ["true", "false"]: - wrong_types.append(name) + wrong_type_inputs.append(name) continue # Check other input formats ? else: pass # TODO @@ -1653,13 +1654,13 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: raise ValueError( f"Missing input value(s) for parameter(s): {', '.join(sorted(missing_inputs))}" ) - if invalid_chars: + if invalid_chars_inputs: raise ValueError( - f"Invalid character(s) in input value(s) for parameter(s): {', '.join(sorted(invalid_chars))}" + f"Invalid character(s) in input value(s) for parameter(s): {', '.join(sorted(invalid_chars_inputs))}" ) - if wrong_types: + if wrong_type_inputs: raise ValueError( - f"Wrong type(s) for parameter(s): {', '.join(sorted(wrong_types))}" + f"Wrong type(s) for parameter(s): {', '.join(sorted(wrong_type_inputs))}" ) # raise RuntimeError("An error should have been raised before this point.") @@ -1671,12 +1672,10 @@ def _is_input_full(cls, value): """ Returns False if `value` contains an empty string or list. """ - if isinstance(value, list) and cls._isinstance(value, str): # Case: list of strings - return all([(len(v) > 0) for v in value]) - elif isinstance(value, (str, list)): # Case: list or string - return (len(value) > 0) - else: # Case: other - return True + if isinstance(value, list): # Case: list + return len(value) > 0 and all([cls._is_input_full(v) for v in value]) + else: + return (len(str(value)) > 0) # ------------------------------------------------ # Function to assert the input contains only a certain Python type diff --git a/tests/test_global.py b/tests/test_global.py index b593a10..e71e2d3 100644 --- a/tests/test_global.py +++ b/tests/test_global.py @@ -103,12 +103,10 @@ def is_input_full(value): """ Returns False if `value` contains an empty string or list. """ - if isinstance(value, list) and all([isinstance(v, str) for v in value]): # Case: list of strings - return all([(len(v) > 0) for v in value]) - elif isinstance(value, (str, list)): # Case: list or string - return (len(value) > 0) - else: # Case: other - return True + if isinstance(value, list): # Case: list + return len(value) > 0 and all([is_input_full(v) for v in value]) + else: + return (len(str(value)) > 0) tested_class._BACKUP_LOCATION = None @@ -132,4 +130,4 @@ def is_input_full(value): # Catch the exception message with pytest.raises(ValueError) as e: s.run_session() - assert str(e.value) == "Missing input value(s) for parameter(s): " + ", ".join(sorted(missing_fields)) \ No newline at end of file + assert str(e.value) == "Missing input value(s) for parameter(s): " + ", ".join(sorted(missing_fields)) From 2c978c5dc106b17d862fd306b0e38fb01b96e8f8 Mon Sep 17 00:00:00 2001 From: Hippolyte Blot Date: Fri, 21 Jun 2024 17:54:29 +0200 Subject: [PATCH 33/33] fix --- src/vip_client/classes/VipLauncher.py | 72 +++++++++++++-------------- tests/__init__.py | 10 ---- tests/test_global.py | 3 +- 3 files changed, 38 insertions(+), 47 deletions(-) delete mode 100644 tests/__init__.py diff --git a/src/vip_client/classes/VipLauncher.py b/src/vip_client/classes/VipLauncher.py index e0fa78e..04a9870 100644 --- a/src/vip_client/classes/VipLauncher.py +++ b/src/vip_client/classes/VipLauncher.py @@ -146,15 +146,10 @@ def input_settings(self, input_settings: dict): # Check type if not isinstance(input_settings, dict): raise TypeError("`input_settings` should be a dictionary") - # Check if each input can be converted to a string with valid characters for VIP - for param, value in input_settings.items(): - invalid = self._is_input_full(value) - invalid = self._invalid_chars_for_vip(value) - # if not (set(invalid) <= {'\\'}): # '\\' is OK for Windows paths #EDIT: Corrected in _invalid_chars_for_vip - if invalid: - raise ValueError( - f"Parameter '{param}' contains some invalid character(s): {', '.join(invalid)}" - ) + + # Check if each input can be converted to a string with valid characters and no empty strings + self._get_invalid_input(input_settings) + # Parse the input settings new_settings = self._parse_input_settings(input_settings) self._print("parsed") @@ -600,7 +595,7 @@ def run_session( self, nb_runs=1, refresh_time=30) -> VipLauncher: 1. Launches pipeline executions on VIP; 2. Monitors pipeline executions until they are all over. - /!\ This function assumes that all session properties are already set. + |!| This function assumes that all session properties are already set. Optional arguments can be provided: - Increase `nb_runs` to run more than 1 execution at once; - Set `refresh_time` to modify the default refresh time; @@ -1601,19 +1596,13 @@ def _check_input_keys(self, input_settings: dict) -> None: + f"'] are useless for pipeline '{self._pipeline_id}'. This may throw RuntimeError later.") # ------------------------------------------------ - # Check the parameter values according to pipeline descriptor - def _check_input_values(self, input_settings: dict, location: str) -> None: - """ - Checks if each parameter value in `input_settings` matches its pipeline description in `parameters`. - `location` refers to the storage infrastructure (e.g., VIP) to scan for missing files. - - Prerequisite: input_settings is defined and contains only strings, or lists of strings. - """ - # Browse the input parameters + + def _get_invalid_input(self, input_settings: dict, parameters_ref=None): missing_inputs = [] invalid_chars_inputs = [] - wrong_type_inputs = [] - for param in self._pipeline_def['parameters']: + if not parameters_ref: # Check from itself if no parameters_ref provided + parameters_ref = [{"name": param} for param in input_settings.keys()] + for param in parameters_ref: # Get parameter name name = param['name'] # Skip irrelevant inputs (this should not happen after self._check_input_keys()) @@ -1621,12 +1610,7 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: continue # Get input value value = input_settings[name] - # `request` will send only strings - if not self._isinstance(value, str): # This should not happen - raise ValueError( # Parameter could not be parsed correctly - f"Parameter: '{name}' \n\twith value: '{value}' \n\twith type: '{type(value)}')\ncould not be parsed."\ - +"Please double check the value; if correct, try converting it to `str` in the `input_settings`." - ) + # Check the input has no empty values if not self._is_input_full(value): missing_inputs.append(name) @@ -1636,6 +1620,30 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: if invalid: invalid_chars_inputs.append((name, invalid)) continue + + if missing_inputs: + raise ValueError( + f"Missing input value(s) for parameter(s): {', '.join(sorted(missing_inputs))}" + ) + if invalid_chars_inputs: + raise ValueError( + f"Invalid character(s) in input value(s) for parameter(s): {', '.join(sorted(invalid_chars_inputs))}" + ) + + # Check the parameter values according to pipeline descriptor + def _check_input_values(self, input_settings: dict, location: str) -> None: + """ + Checks if each parameter value in `input_settings` matches its pipeline description in `parameters`. + `location` refers to the storage infrastructure (e.g., VIP) to scan for missing files. + + Prerequisite: input_settings is defined and contains only strings, or lists of strings. + """ + self._get_invalid_input(input_settings, self._pipeline_def['parameters']) + + wrong_type_inputs = [] + for param in self._pipeline_def['parameters']: + name = param['name'] + value = input_settings.get(name) # If input is a File, check file(s) existence if param["type"] == "File": # Ensure every file exists at `location` @@ -1650,19 +1658,11 @@ def _check_input_values(self, input_settings: dict, location: str) -> None: continue # Check other input formats ? else: pass # TODO - if missing_inputs: - raise ValueError( - f"Missing input value(s) for parameter(s): {', '.join(sorted(missing_inputs))}" - ) - if invalid_chars_inputs: - raise ValueError( - f"Invalid character(s) in input value(s) for parameter(s): {', '.join(sorted(invalid_chars_inputs))}" - ) + if wrong_type_inputs: raise ValueError( f"Wrong type(s) for parameter(s): {', '.join(sorted(wrong_type_inputs))}" ) - # raise RuntimeError("An error should have been raised before this point.") # ------------------------------------------------ diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index 3588236..0000000 --- a/tests/__init__.py +++ /dev/null @@ -1,10 +0,0 @@ -""" -Tests for the VIP Python Client -""" - -# Import classes and packages to secure the namespace -import sys -from pathlib import Path -SOURCE_ROOT = str(Path(__file__).parents[1] / "src") # <=> /src/ -sys.path.append(SOURCE_ROOT) -import vip_client \ No newline at end of file diff --git a/tests/test_global.py b/tests/test_global.py index e71e2d3..1784bfd 100644 --- a/tests/test_global.py +++ b/tests/test_global.py @@ -120,14 +120,15 @@ def is_input_full(value): s.input_dir = "." else: s.output_dir = "/path/to/output" - s.input_settings = input_settings missing_fields = [field for field in input_settings if not is_input_full(input_settings[field])] if not missing_fields: + s.input_settings = input_settings s.run_session() return # Catch the exception message with pytest.raises(ValueError) as e: + s.input_settings = input_settings s.run_session() assert str(e.value) == "Missing input value(s) for parameter(s): " + ", ".join(sorted(missing_fields))