Merge pull request #19 from Sage-Bionetworks-Workflows/bgrande/tests-suites-cov

Close code coverage for `suites` and `tests` submodules

Author: Bruno Grande

src/dcqc/mixins.py

@@ -108,6 +108,7 @@ def to_dict(self) -> SerializedObject:
         return self.dict_factory(result)
 
     # TODO: Use template method to handle `_serialized_properties`
+    #       as well as `deepcopy()`
     @classmethod
     @abstractmethod
     def from_dict(cls, dictionary: SerializedObject) -> SerializableMixin:

src/dcqc/suites/suite_abc.py

@@ -51,7 +51,9 @@ def __init__(
         test_classes = self.list_test_classes()
         test_names = set(test.__name__ for test in test_classes)
 
-        required_tests = required_tests or self._default_required_tests()
+        # To differentiate between None and []
+        if required_tests is None:
+            required_tests = self._default_required_tests()
         self.required_tests = set(required_tests).intersection(test_names)
 
         skipped_tests = skipped_tests or list()
@@ -216,6 +218,10 @@ def get_subclass_by_file_type(
             return registry["*"]
         return registry[name]
 
+    @property
+    def tests_by_name(self):
+        return {test.type: test for test in self.tests}
+
     def compute_tests(self) -> None:
         """Compute the status for each initialized test."""
         self.target.stage()
@@ -225,8 +231,9 @@ def compute_tests(self) -> None:
     def compute_status(self) -> TestStatus:
         """Compute the overall suite status."""
         self.compute_tests()
-        if self._status is not TestStatus.NONE:
+        if self._status != TestStatus.NONE:
             return self._status
+        self._status = TestStatus.PASS
         for test in self.tests:
             test_name = test.type
             if test_name not in self.required_tests:

src/dcqc/tests/test_abc.py

@@ -3,7 +3,8 @@
 import shlex
 from abc import ABC, abstractmethod
 from collections.abc import Sequence
-from dataclasses import dataclass
+from copy import deepcopy
+from dataclasses import InitVar, dataclass
 from enum import Enum
 from importlib import import_module
 from pathlib import Path
@@ -62,9 +63,9 @@ def skip(self):
         """Force the test to be skipped."""
         self._status = TestStatus.SKIP
 
-    def get_status(self) -> TestStatus:
+    def get_status(self, compute_ok: bool = True) -> TestStatus:
         """Compute (if applicable) and return the test status."""
-        if self._status == TestStatus.NONE:
+        if self._status == TestStatus.NONE and compute_ok:
             self._status = self.compute_status()
         return self._status
 
@@ -141,18 +142,18 @@ def import_module(self, name: str) -> ModuleType:
 @dataclass
 class Process(SerializableMixin):
     container: str
-    command_args: Sequence[str]
+    command_args: InitVar[Sequence[str]]
     cpus: int = 1
     memory: int = 2  # In GB
 
-    def get_command(self) -> str:
-        return shlex.join(self.command_args)
+    _serialized_properties = ["command"]
 
-    def to_dict(self):
-        dictionary = super(Process, self).to_dict()
-        del dictionary["command_args"]
-        dictionary["command"] = self.get_command()
-        return dictionary
+    def __post_init__(self, command_args: Sequence[str]):
+        self._command_args = command_args
+
+    @property
+    def command(self) -> str:
+        return shlex.join(self._command_args)
 
     @classmethod
     def from_dict(cls, dictionary: SerializedObject) -> Process:
@@ -164,6 +165,7 @@ def from_dict(cls, dictionary: SerializedObject) -> Process:
         Returns:
             The reconstructed process object.
         """
+        dictionary = deepcopy(dictionary)
         command = dictionary.pop("command")
         command_args = shlex.split(command)
         dictionary["command_args"] = command_args

tests/conftest.py

@@ -15,6 +15,8 @@
 import pytest
 
 from dcqc.file import File
+from dcqc.suites.suite_abc import SuiteABC
+from dcqc.target import Target
 
 CNFPATH = Path(__file__).resolve()
 TESTDIR = CNFPATH.parent
@@ -52,6 +54,7 @@ def _get_data(filename: str) -> Path:
 @pytest.fixture
 def test_files(get_data):
     txt_path = get_data("test.txt")
+    jsonld_path = get_data("example.jsonld")
     tiff_path = get_data("circuit.tif")
     syn_path = "syn://syn50555279"
     good_metadata = {
@@ -62,6 +65,10 @@ def test_files(get_data):
         "file_type": "tiff",
         "md5_checksum": "definitelynottherightmd5checksum",
     }
+    jsonld_metadata = {
+        "file_type": "JSON-LD",
+        "md5_checksum": "56bb5f34da6d6df2ade3ac37e25586b7",
+    }
     tiff_metadata = {
         "file_type": "tiff",
         "md5_checksum": "c7b08f6decb5e7572efbe6074926a843",
@@ -70,6 +77,7 @@ def test_files(get_data):
         "good": File(txt_path.as_posix(), good_metadata),
         "bad": File(txt_path.as_posix(), bad_metadata),
         "tiff": File(tiff_path.as_posix(), tiff_metadata),
+        "jsonld": File(jsonld_path.as_posix(), jsonld_metadata),
         "synapse": File(syn_path, good_metadata),
     }
 
@@ -81,6 +89,22 @@ def test_files(get_data):
     yield test_files
 
 
+@pytest.fixture
+def test_targets(test_files):
+    test_targets = dict()
+    for name, file in test_files.items():
+        test_targets[name] = Target(file)
+    yield test_targets
+
+
+@pytest.fixture
+def test_suites(test_targets):
+    test_suites = dict()
+    for name, target in test_targets.items():
+        test_suites[name] = SuiteABC.from_target(target)
+    yield test_suites
+
+
 @pytest.fixture
 def get_output():
     def _get_output(filename: str) -> Path:

tests/test_suites.py

@@ -3,18 +3,18 @@
 from dcqc.file import FileType
 from dcqc.suites.suite_abc import SuiteABC
 from dcqc.suites.suites import FileSuite, OmeTiffSuite, TiffSuite
-from dcqc.target import Target
 from dcqc.tests.test_abc import TestABC, TestStatus
-from dcqc.tests.tests import LibTiffInfoTest
+from dcqc.tests.tests import FileExtensionTest, LibTiffInfoTest
+
+FileType("None", ())
+FileType("Unpaired", ())
 
 
 class RedundantFileSuite(TiffSuite):
+    file_type = FileType.get_file_type("None")
     del_tests = (LibTiffInfoTest,)
 
 
-FileType("Unpaired", ())
-
-
 class DummyTest(TestABC):
     def compute_status(self) -> TestStatus:
         return TestStatus.NONE
@@ -69,8 +69,52 @@ def test_that_the_generic_file_suite_is_retrieved_for_an_unpaired_file_type():
     assert actual is FileSuite
 
 
-def test_that_the_default_required_tests_are_only_tiers_1_and_2(test_files):
-    tiff_file = test_files["tiff"]
-    tiff_target = Target(tiff_file)
-    tiff_suite = TiffSuite(tiff_target)
-    assert all(test.tier <= 2 for test in tiff_suite.tests)
+def test_that_the_default_required_tests_are_only_tiers_1_and_2(test_suites):
+    suite = test_suites["tiff"]
+    assert all(test.tier <= 2 for test in suite.tests)
+
+
+def test_that_skipped_tests_are_skipped_when_building_suite_from_tests(test_suites):
+    suite = test_suites["tiff"]
+    tests = suite.tests
+    new_suite = SuiteABC.from_tests(tests, skipped_tests=["LibTiffInfoTest"])
+    skipped_test_before = suite.tests_by_name["LibTiffInfoTest"]
+    skipped_test_after = new_suite.tests_by_name["LibTiffInfoTest"]
+    assert skipped_test_before.get_status(compute_ok=False) != TestStatus.SKIP
+    assert skipped_test_after.get_status(compute_ok=False) == TestStatus.SKIP
+
+
+def test_for_an_error_when_building_suite_from_tests_with_diff_targets(test_targets):
+    target_1 = test_targets["good"]
+    target_2 = test_targets["bad"]
+    test_1 = FileExtensionTest(target_1)
+    test_2 = FileExtensionTest(target_2)
+    tests = [test_1, test_2]
+    with pytest.raises(ValueError):
+        SuiteABC.from_tests(tests)
+
+
+def test_that_a_suite_will_not_consider_unrequired_tests(test_targets):
+    target = test_targets["bad"]
+    required_tests = []
+    skipped_tests = ["LibTiffInfoTest"]
+    suite = SuiteABC.from_target(target, required_tests, skipped_tests)
+    suite_status = suite.compute_status()
+    assert suite_status == TestStatus.PASS
+
+
+def test_that_a_suite_will_consider_required_tests_when_failing(test_targets):
+    target = test_targets["bad"]
+    required_tests = ["FileExtensionTest"]
+    skipped_tests = ["LibTiffInfoTest"]
+    suite = SuiteABC.from_target(target, required_tests, skipped_tests)
+    suite_status = suite.compute_status()
+    assert suite_status == TestStatus.FAIL
+
+
+def test_that_a_suite_will_consider_required_tests_when_passing(test_targets):
+    target = test_targets["good"]
+    required_tests = ["Md5ChecksumTest"]
+    suite = SuiteABC.from_target(target, required_tests)
+    suite_status = suite.compute_status()
+    assert suite_status == TestStatus.PASS