Merge pull request #35 from Sage-Bionetworks-Workflows/bwmac/ORCA-231…

…/fix_external_tests

[ORCA-231] Fix External Tests

src/dcqc/tests/base_test.py

@@ -153,6 +153,7 @@ def from_dict(cls, dictionary: SerializedObject) -> Process:
 
 
 class ExternalTestMixin(BaseTest):
+    pass_code: ClassVar[str]
     # Class attributes
     is_external_test = True
 
@@ -195,12 +196,27 @@ def _interpret_process_outputs(self, outputs: dict[str, Path]) -> TestStatus:
         """Interpret the process output files to yield a test status."""
         exit_code = outputs["exit_code"].read_text()
         exit_code = exit_code.strip()
-        if exit_code == "0":
+        if exit_code == self.pass_code:
             status = TestStatus.PASS
         else:
             status = TestStatus.FAIL
         return status
 
+    # TODO: make changes to this package or the nextflow
+    # workflow so that file mounting is handled cleaner
+    @staticmethod
+    def _short_string_path(path: Path, substring: str) -> str:
+        # chech if substring is in path
+        if substring not in path.as_posix():
+            raise ValueError(f"{substring} not in {path}")
+        # get index where staged folder is
+        index = next(i for i, item in enumerate(path.parts) if substring in item)
+        # shorten path starting from staged folder
+        short_path = Path(*path.parts[index:])
+        # wrap path string in quotes
+        quote_path = str(short_path)
+        return f"'{quote_path}'"
+
     # TODO: Include process in serialized test dictionary
     # def to_dict(self):
     #     dictionary = super(ExternalTestMixin, self).to_dict()

src/dcqc/tests/grep_date_test.py

@@ -4,19 +4,21 @@
 
 class GrepDateTest(ExternalBaseTest):
     tier = 4
+    pass_code = "1"
     target: SingleTarget
 
     def generate_process(self) -> Process:
         path = self.target.file.stage()
+        string_path = self._short_string_path(path, "dcqc-staged-")
+
         command_args = [
-            "!",  # negate exit status
             "grep",
             "-E",  # extended regular expression
             "-i",  # case insensitive
             "-a",  # treat input as text
             "-q",  # suppress output
             "'date|time'",  # match date or time
-            path,
+            string_path,
         ]
         process = Process(
             container="quay.io/biocontainers/coreutils:8.30--h14c3975_1000",

src/dcqc/tests/libtiff_info_test.py

@@ -4,13 +4,16 @@
 
 class LibTiffInfoTest(ExternalBaseTest):
     tier = 2
+    pass_code = "0"
     target: SingleTarget
 
     def generate_process(self) -> Process:
         path = self.target.file.stage()
+        string_path = self._short_string_path(path, "dcqc-staged-")
+
         command_args = [
             "tiffinfo",
-            path,
+            string_path,
         ]
         process = Process(
             container="quay.io/sagebionetworks/libtiff:2.0",

src/dcqc/tests/tiff_tag_306_date_time_test.py

@@ -4,15 +4,17 @@
 
 class TiffTag306DateTimeTest(ExternalBaseTest):
     tier = 4
+    pass_code = "1"
     target: SingleTarget
 
     def generate_process(self) -> Process:
         path = self.target.file.stage()
+        string_path = self._short_string_path(path, "dcqc-staged-")
+
         command_args = [
-            "!",  # negate exit status
             "tifftools",
             "dump",
-            path,
+            string_path,
             "|",
             "grep",  # pipe the output
             "-a",  # treat input as text

tests/data/test.external.json

@@ -13,7 +13,7 @@
         },
         "type": "TIFF",
         "name": "test.txt",
-        "local_path": "tests/data/test.txt"
+        "local_path": "tests/data/dcqc-staged-test/test.txt"
       }
     ],
     "type": "SingleTarget"

tests/test_tests.py

@@ -90,23 +90,24 @@ def test_that_all_external_tests_inherit_from_the_mixin_first():
 def test_that_the_libtiff_info_test_correctly_interprets_exit_code_0_and_1(
     test_files, mocker
 ):
+    # 0 is pass, 1 is fail
     tiff_file = test_files["tiff"]
     target = SingleTarget(tiff_file)
     with TemporaryDirectory() as tmp_dir:
         path_0 = Path(tmp_dir, "code_0.txt")
         path_1 = Path(tmp_dir, "code_1.txt")
         path_0.write_text("0")
         path_1.write_text("1")
-        good_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
-        bad_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
+        pass_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
+        fail_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
 
         test = tests.LibTiffInfoTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=good_outputs)
+        mocker.patch.object(test, "_find_process_outputs", return_value=pass_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.PASS
 
         test = tests.LibTiffInfoTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=bad_outputs)
+        mocker.patch.object(test, "_find_process_outputs", return_value=fail_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.FAIL
 
@@ -183,23 +184,24 @@ def test_that_a_process_can_be_serialized_and_deserialized():
 def test_that_the_grep_date_test_correctly_interprets_exit_code_0_and_1(
     test_files, mocker
 ):
+    # 1 is pass, 0 is fail
     tiff_file = test_files["tiff"]
     target = SingleTarget(tiff_file)
     with TemporaryDirectory() as tmp_dir:
         path_0 = Path(tmp_dir, "code_0.txt")
         path_1 = Path(tmp_dir, "code_1.txt")
         path_0.write_text("0")
         path_1.write_text("1")
-        good_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
-        bad_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
+        fail_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
+        pass_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
 
         test = tests.GrepDateTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=good_outputs)
+        mocker.patch.object(test, "_find_process_outputs", return_value=pass_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.PASS
 
-        test = tests.LibTiffInfoTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=bad_outputs)
+        test = tests.GrepDateTest(target)
+        mocker.patch.object(test, "_find_process_outputs", return_value=fail_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.FAIL
 
@@ -221,23 +223,24 @@ def test_that_the_tifftag306datetimetest_command_is_produced(test_targets):
 def test_that_the_tifftag306datetimetest_correctly_interprets_exit_code_0_and_1(
     test_files, mocker
 ):
+    # 1 is pass, 0 is fail
     tiff_file = test_files["tiff"]
     target = SingleTarget(tiff_file)
     with TemporaryDirectory() as tmp_dir:
         path_0 = Path(tmp_dir, "code_0.txt")
         path_1 = Path(tmp_dir, "code_1.txt")
         path_0.write_text("0")
         path_1.write_text("1")
-        good_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
-        bad_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
+        fail_outputs = {"std_out": path_1, "std_err": path_1, "exit_code": path_0}
+        pass_outputs = {"std_out": path_0, "std_err": path_0, "exit_code": path_1}
 
         test = tests.TiffTag306DateTimeTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=good_outputs)
+        mocker.patch.object(test, "_find_process_outputs", return_value=pass_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.PASS
 
-        test = tests.LibTiffInfoTest(target)
-        mocker.patch.object(test, "_find_process_outputs", return_value=bad_outputs)
+        test = tests.TiffTag306DateTimeTest(target)
+        mocker.patch.object(test, "_find_process_outputs", return_value=fail_outputs)
         test_status = test.get_status()
         assert test_status == TestStatus.FAIL
 
@@ -271,3 +274,18 @@ def test_that_paired_fastq_parity_test_correctly_handles_compressed_fastq_files(
     test = tests.PairedFastqParityTest(target)
     test_status = test.get_status()
     assert test_status == TestStatus.PASS
+
+
+def test_that_short_string_path_correctly_shortens_file_paths():
+    substring = "test-substring"
+    long_path = f"path/needs/to/be/shortened/{substring}/file.txt"
+    expected_short_path = f"'{substring}/file.txt'"
+    short_path = ExternalTestMixin._short_string_path(Path(long_path), substring)
+    assert short_path == expected_short_path
+
+
+def test_that_short_string_path_raises_valueerror_if_substring_not_in_path():
+    substring = "test-substring"
+    long_path = "path/needs/to/be/shortened/fail/file.txt"
+    with pytest.raises(ValueError):
+        ExternalTestMixin._short_string_path(Path(long_path), substring)