Updating tests and modifications of errors to standardize their messages

.github/workflows/main.yml

@@ -0,0 +1,34 @@
+on:
+  push:
+    branches:
+      - main
+      - develop
+  pull_request:
+    branches:
+      - develop
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    env:
+      VIP_API_KEY: ${{ secrets.VIP_API_KEY }}
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Set up Python 3.9
+      uses: actions/setup-python@v4
+      with:
+        python-version: 3.9
+
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install pytest pytest_mock requests girder_client
+
+    - name: Run Pytest
+      run: |
+        export PYTHONPATH=src:$PYTHONPATH
+        pytest

.gitignore

@@ -1,11 +1,11 @@
 __pycache__/
+.pytest_cache/
 .vscode/
 vip_outputs/
-perso/
 
 examples/old
 
 publish.sh
 dist/
-tests/old
-tests/__pycache__
+.env
+test_pipedef.py

src/vip_client/classes/VipCI.py

@@ -226,7 +226,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.

src/vip_client/classes/VipLauncher.py

@@ -146,14 +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._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._check_invalid_input(input_settings)
+
         # Parse the input settings
         new_settings = self._parse_input_settings(input_settings)
         self._print("parsed")
@@ -489,10 +485,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
@@ -602,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;
@@ -1562,7 +1555,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")
+                raise AttributeError("Input settings have not been defined.")
         # Check the pipeline identifier
         if not self._is_defined("_pipeline_id"): 
             raise AttributeError("Input settings could not be checked without a pipeline identifier.")
@@ -1584,14 +1577,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 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
@@ -1603,50 +1596,74 @@ 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
-        for param in self._pipeline_def['parameters'] :
+
+    def _check_invalid_input(self, input_settings: dict, parameters_ref=None):
+        missing_inputs = []
+        invalid_chars_inputs = []
+        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())
             if name not in input_settings:
                 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):
-                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_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._check_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`
                 missing_file = self._first_missing_file(value, location)
                 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_type_inputs.append(name)
+                    continue
             # Check other input formats ?
             else: pass # TODO
+
+        if wrong_type_inputs:
+            raise ValueError(
+                f"Wrong type(s) for parameter(s): {', '.join(sorted(wrong_type_inputs))}"
+            )
+
     # ------------------------------------------------
 
     # Function to look for empty values
@@ -1655,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

src/vip_client/classes/VipSession.py

@@ -542,7 +542,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;
@@ -583,7 +583,7 @@ def finish(self, timeout=300) -> VipSession:
             self._print( "    Please run finish() from the original session or remove them manually on the VIP portal:")
             self._print(f"\t{self._VIP_PORTAL}")
         # Save the session
-        self._save()    
+        self._save()
         # Return
         return self
     # ------------------------------------------------
@@ -1076,7 +1076,7 @@ def parse_value(input):
             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 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
@@ -1091,8 +1091,10 @@ 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:
+                        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

src/vip_client/utils/vip.py

@@ -141,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:
@@ -396,10 +397,10 @@ 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)
     try:
         manage_errors(rq)
     except RuntimeError:

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

tests/data/file.txt

@@ -0,0 +1 @@
+This is juste a test file for upload

tests/data/file_copy.txt

@@ -0,0 +1 @@
+This is juste a test file for upload