some refactoring to the unit tests, adding cases for slurm

babs/constants.py

@@ -1,3 +1,4 @@
 MSG_NO_ALERT_IN_LOGS = "BABS: No alert message found in log files."
 CHECK_MARK = u'\N{check mark}'  # can be used by print(CHECK_MARK)
 PATH_FS_LICENSE_IN_CONTAINER = "/SGLR/FREESURFER_HOME/license.txt"
+TYPE_SYSTEM_SUPPORTED = ['sge', 'slurm']

babs/utils.py

@@ -16,6 +16,9 @@
 from datetime import datetime
 import re
 
+from babs.constants import TYPE_SYSTEM_SUPPORTED
+
+
 # Disable the behavior of printing messages:
 def blockPrint():
     sys.stdout = open(os.devnull, 'w')
@@ -155,13 +158,12 @@ def validate_type_system(type_system):
     For valid ones, the type string will be changed to lower case.
     If not valid, raise error message.
     """
-    list_supported = ['sge', 'slurm']
-    if type_system.lower() in list_supported:
+    if type_system.lower() in TYPE_SYSTEM_SUPPORTED:
         type_system = type_system.lower()   # change to lower case, if needed
     else:
         raise Exception("Invalid cluster system type: '" + type_system + "'!"
                         + " Currently BABS only support one of these: "
-                        + ', '.join(list_supported))   # names separated by ', '
+                        + ', '.join(TYPE_SYSTEM_SUPPORTED))   # names separated by ', '
     return type_system
 
 

tests/get_data.py

@@ -107,9 +107,12 @@ def where_now(if_circleci):
     elif if_singularity_installed:
         where_now = "on_cluster"
     elif if_docker_installed:
-        where_now = "on_local"
+        if subprocess.call(["docker", "info"]) == 0:
+            where_now = "on_local"
+        else:
+            raise Exception("Docker is installed, but not running!"
+                            + " Pytest cannot proceed.")
     else:
-        where_now = ""
         raise Exception("Not on CircleCI, and neither singularity nor docker is installed!"
                         + " Pytest cannot proceed.")
     return where_now
@@ -192,6 +195,10 @@ def container_ds_path(where_now, tmp_path_factory):
                 url="dhub://"+docker_addr   # e.g., "dhub://pennlinc/toy_bids_app:0.0.7"
             )
 
+    # Container dataset - has been set up by fixture `prep_container_ds_toybidsapp()`
+    assert op.exists(origin_container_ds)
+    assert op.exists(op.join(origin_container_ds, ".datalad/config"))
+
     return origin_container_ds
 
 def get_container_config_yaml_filename(which_bidsapp,
@@ -245,7 +252,13 @@ def get_container_config_yaml_filename(which_bidsapp,
     container_config_yaml_filename += "_" + type_system + "_" \
         + dict_cluster_name[type_system] + ".yaml"
 
-    return container_config_yaml_filename
+    notebooks_dir = op.join(op.dirname(__location__), "notebooks")
+    container_config_yaml_file = op.join(notebooks_dir, container_config_yaml_filename)
+
+    if not op.exists(container_config_yaml_file):
+        pytest.skip(f"container config yaml file doesn't exist in {notebooks_dir}")
+
+    return container_config_yaml_file
 
 
 def if_command_installed(cmd):

tests/test_babs_check_setup.py

@@ -26,15 +26,21 @@
 
 @pytest.mark.order(index=2)
 @pytest.mark.parametrize(
-    "which_case",
-    [("not_to_keep_failed"),
-     ("wrong_container_ds"),
-     ("wrong_input_ds")]
+    "which_case, error_type, error_msg",[
+        # container_ds` has wrong path; not to `--keep-if-failed`
+        ("not_to_keep_failed", Exception, "`--project-root` does not exist!"),
+        # container_ds` has wrong path
+        ("wrong_container_ds", AssertionError, "There is no containers DataLad dataset in folder:"),
+        # `input ds` has wrong path
+        ("wrong_input_ds", FileNotFoundError, "No such file or directory:")
+    ]
 )
 def test_babs_check_setup(
-        which_case,
+        which_case, error_type, error_msg,
         tmp_path, tmp_path_factory,
-        container_ds_path, if_circleci):
+        container_ds_path, if_circleci,
+        type_system="sge"
+):
     """
     This is to test `babs-check-setup` in different failed `babs-init` cases.
     Successful `babs-init` has been tested in `test_babs_init.py`.
@@ -49,10 +55,16 @@ def test_babs_check_setup(
         All cases have something going wrong, leading to `babs-init` failure;
         Only in case `not_to_keep_failed`, flag `--keep-if-failed` in `babs-init` won't turn on,
         so expected error will be: BABS project does not exist.
+    error_type: Exception
+        The type of expected error raised by `babs-check-setup`
+    error_msg: str
+        The expected error message raised by `babs-check-setup`
     tmp_path: fixture from pytest
     tmp_path_factory: fixture from pytest
     container_ds_path: fixture; str
         Path to the container datalad dataset
+    type_system: str
+         the type of job scheduling system, "sge" or "slurm" (shouldn't make a difference here)
     """
     # fixed variables:
     which_bidsapp = "toybidsapp"
@@ -64,56 +76,48 @@ def test_babs_check_setup(
     # Get the path to input dataset:
     path_in = get_input_data(which_input, type_session, if_input_local, tmp_path_factory)
     input_ds_cli = [[which_input, path_in]]
-    input_ds_cli_wrong = [[which_input, "/random/path/to/input_ds"]]
-
-    # Container dataset - has been set up by fixture `prep_container_ds_toybidsapp()`
-    assert op.exists(container_ds_path)
-    assert op.exists(op.join(container_ds_path, ".datalad/config"))
-    container_ds_path_wrong = "/random/path/to/container_ds"
 
     # Preparation of env variable `TEMPLATEFLOW_HOME`:
     os.environ["TEMPLATEFLOW_HOME"] = TEMPLATEFLOW_HOME
     assert os.getenv('TEMPLATEFLOW_HOME') is not None    # assert env var has been set
 
     # Get the cli of `babs-init`:
-    where_project = tmp_path.absolute().as_posix()   # turn into a string
     project_name = "my_babs_project"
-    project_root = op.join(where_project, project_name)
+    project_root = str(tmp_path / project_name)
     container_name = which_bidsapp + "-" + TOYBIDSAPP_VERSION_DASH
-    container_config_yaml_filename = "example_container_" + which_bidsapp + ".yaml"
-    container_config_yaml_filename = \
+
+
+    container_config_yaml_file = \
         get_container_config_yaml_filename(which_bidsapp, which_input, if_two_input=False,
-                                           type_system="sge")  # TODO: also test slurm!
-    container_config_yaml_file = op.join(op.dirname(__location__), "notebooks",
-                                         container_config_yaml_filename)
-    assert op.exists(container_config_yaml_file)
+                                           type_system=type_system)
 
     # below are all correct options:
     babs_init_opts = argparse.Namespace(
-        where_project=where_project,
+        where_project=str(tmp_path),
         project_name=project_name,
         input=input_ds_cli,
         list_sub_file=None,
         container_ds=container_ds_path,
         container_name=container_name,
         container_config_yaml_file=container_config_yaml_file,
         type_session=type_session,
-        type_system="sge",
+        type_system=type_system,
         keep_if_failed=True
     )
 
     # inject something wrong --> `babs-init` will fail:
-    babs_init_opts.container_ds = container_ds_path_wrong
-    # `--keep-if-failed`:
-    if which_case == "not_to_keep_failed":
-        babs_init_opts.keep_if_failed = False
-    # each case, what went wrong:
+    container_ds_path_wrong = "/random/path/to/container_ds"
+    input_ds_cli_wrong = [[which_input, "/random/path/to/input_ds"]]
     if which_case == "not_to_keep_failed":
         babs_init_opts.container_ds = container_ds_path_wrong
+        # `--keep-if-failed`:
+        babs_init_opts.keep_if_failed = False
     elif which_case == "wrong_container_ds":
         babs_init_opts.container_ds = container_ds_path_wrong
     elif which_case == "wrong_input_ds":
         babs_init_opts.input = input_ds_cli_wrong
+    else:
+        pytest.skip("No rules provided for this case!")
 
     # run `babs-init`:
     with mock.patch.object(
@@ -125,18 +129,6 @@ def test_babs_check_setup(
         project_root=project_root,
         job_test=False
     )
-    # Set up expected error message from `babs-check-setup`:
-    if which_case == "not_to_keep_failed":
-        error_type = Exception   # what's after `raise` in the source code
-        error_msg = "`--project-root` does not exist!"
-        # ^^ see `get_existing_babs_proj()` in CLI
-    elif which_case == "wrong_container_ds":
-        error_type = AssertionError   # error from `assert`
-        error_msg = "There is no containers DataLad dataset in folder:"
-    elif which_case == "wrong_input_ds":
-        error_type = FileNotFoundError
-        error_msg = "No such file or directory:"
-        # ^^ No such file or directory: '/path/to/my_babs_project/analysis/inputs/data'
 
     # Run `babs-check-setup`:
     with mock.patch.object(

tests/test_babs_init.py

@@ -8,6 +8,7 @@
 sys.path.append("..")
 sys.path.append("../babs")
 from babs.utils import (read_yaml)   # noqa
+from babs.constants import (TYPE_SYSTEM_SUPPORTED)   # noqa
 from babs.cli import (    # noqa
     babs_init_main,
     babs_check_setup_main)
@@ -25,11 +26,13 @@
 )
 
 @pytest.mark.order(index=1)
+@pytest.mark.parametrize("type_system", ["sge", "slurm"])
 @pytest.mark.parametrize(
-    "which_bidsapp,which_input,type_session,if_input_local,if_two_input",
+    "which_bidsapp, which_input, type_session, if_input_local, if_two_input",
     #  test toybidsapp: BIDS/zipped x single/multi-ses:
     #    the input data will also be remote by default:
-    [("toybidsapp", "BIDS", "single-ses", False, False),
+    [
+     ("toybidsapp", "BIDS", "single-ses", False, False),
      ("toybidsapp", "BIDS", "multi-ses", False, False),
      ("toybidsapp", "fmriprep", "single-ses", False, False),
      ("toybidsapp", "fmriprep", "multi-ses", False, False),
@@ -44,9 +47,11 @@
      ("fmriprep", "BIDS", "single-ses", False, True),
      ("fmriprep", "BIDS", "multi-ses", False, True),
      ])
-def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_two_input,
+def test_babs_init(which_bidsapp, which_input, type_session,
+                   if_input_local, if_two_input,
                    tmp_path, tmp_path_factory,
                    container_ds_path, if_circleci,
+                   type_system
                    ):
     """
     This is to test `babs-init` in different cases.
@@ -72,11 +77,14 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
         Path to the container datalad dataset
     if_circleci: fixture; bool
         Whether currently in CircleCI
-
-    TODO: add `type_system` and to test out Slurm version!
+    type_system: str
+         the type of job scheduling system, "sge" or "slurm"
     """
     # Sanity checks:
-    assert which_bidsapp in LIST_WHICH_BIDSAPP
+    if which_bidsapp not in LIST_WHICH_BIDSAPP:
+        pytest.skip(f"which_bidsapp must be one of {LIST_WHICH_BIDSAPP}, test can't be run")
+    if type_system not in TYPE_SYSTEM_SUPPORTED:
+        pytest.skip(f"type_system must be one of {TYPE_SYSTEM_SUPPORTED}, test can't be run")
 
     # Get the path to input dataset:
     path_in = get_input_data(which_input, type_session, if_input_local, tmp_path_factory)
@@ -91,20 +99,14 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
             tmp_path_factory)
         input_ds_cli.append([INFO_2ND_INPUT_DATA["which_input"], path_in_2nd])
 
-    # Container dataset - has been set up by fixture `prep_container_ds_toybidsapp()`
-    assert op.exists(container_ds_path)
-    assert op.exists(op.join(container_ds_path, ".datalad/config"))
 
     # Preparation of freesurfer: for fmriprep and qsiprep:
-    # check if `--fs-license-file` is included in YAML file:
-    container_config_yaml_filename = \
+    container_config_yaml_file = \
         get_container_config_yaml_filename(which_bidsapp, which_input, if_two_input,
-                                           type_system="sge")  # TODO: also test slurm!
-    container_config_yaml_file = op.join(op.dirname(__location__), "notebooks",
-                                         container_config_yaml_filename)
-    assert op.exists(container_config_yaml_file)
+                                           type_system=type_system)
     container_config_yaml = read_yaml(container_config_yaml_file)
 
+    # check if `--fs-license-file` is included in YAML file:
     if "--fs-license-file" in container_config_yaml["singularity_run"]:
         # ^^ this way is consistent with BABS re: how to determine if fs license is needed;
         flag_requested_fs_license = True
@@ -133,7 +135,7 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
         container_name=container_name,
         container_config_yaml_file=container_config_yaml_file,
         type_session=type_session,
-        type_system="sge",
+        type_system=type_system,
         keep_if_failed=False
     )
 
@@ -162,11 +164,11 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
     # 3) for freesurfer: flag `--fs-license-file`
 
     # first, read in `<container_name>-0-0-0_zip.sh`:
-    fn_bash_container_zip = op.join(project_root, "analysis/code", container_name + "_zip.sh")
-    file_bash_container_zip = open(fn_bash_container_zip, 'r')
-    lines_bash_container_zip = file_bash_container_zip.readlines()
-    file_bash_container_zip.close()
-    # check:
+    with open(op.join(project_root, "analysis/code", container_name + "_zip.sh")) as f:
+        bash_container_zip = f.read()
+
+    # checking the content of files created by BABS init
+    # setting initial values for flags:
     if_bind_templateflow = False   # `singularity run -B` to bind a path to container
     if_bind_freesurfer = False
     str_bind_freesurfer = "-B " + str_fs_license_file \
@@ -178,21 +180,22 @@ def test_babs_init(which_bidsapp, which_input, type_session, if_input_local, if_
     if_flag_bidsfilterfile = False
     if_flag_fs_license = False
     flag_fs_license = '--fs-license-file /SGLR/FREESURFER_HOME/license.txt'
-    for line in lines_bash_container_zip:
-        if "--env TEMPLATEFLOW_HOME=/SGLR/TEMPLATEFLOW_HOME" in line:
-            if_set_singu_templateflow = True
-        if all(ele in line for ele in ["-B",
-                                       TEMPLATEFLOW_HOME + ":/SGLR/TEMPLATEFLOW_HOME"]):
-            # e.g., `-B /test/templateflow_home:/SGLR/TEMPLATEFLOW_HOME \`
-            if_bind_templateflow = True
-        if str_bind_freesurfer in line:
-            if_bind_freesurfer = True
-        if "filterfile=${PWD}/${sesid}_filter.json" in line:
-            if_generate_bidsfilterfile = True
-        if '--bids-filter-file "${filterfile}"' in line:
-            if_flag_bidsfilterfile = True
-        if flag_fs_license in line:
-            if_flag_fs_license = True
+
+    if "--env TEMPLATEFLOW_HOME=/SGLR/TEMPLATEFLOW_HOME" in bash_container_zip:
+        if_set_singu_templateflow = True
+    if all(ele in bash_container_zip for ele in ["-B",
+                                   TEMPLATEFLOW_HOME + ":/SGLR/TEMPLATEFLOW_HOME"]):
+        # e.g., `-B /test/templateflow_home:/SGLR/TEMPLATEFLOW_HOME \`
+        if_bind_templateflow = True
+    if str_bind_freesurfer in bash_container_zip:
+        if_bind_freesurfer = True
+    if "filterfile=${PWD}/${sesid}_filter.json" in bash_container_zip:
+        if_generate_bidsfilterfile = True
+    if '--bids-filter-file "${filterfile}"' in bash_container_zip:
+        if_flag_bidsfilterfile = True
+    if flag_fs_license in bash_container_zip:
+        if_flag_fs_license = True
+
     # assert they are found:
     # 1) TemplateFlow: should be found in all cases:
     assert if_bind_templateflow, \