account for possibility of null return value being introduced into th…

…e protected import function by guarding with a final default Sample fallback; add tests for protocol matching

looper/models.py

@@ -865,7 +865,7 @@ def build_submission_bundles(self, protocol, priority=True):
             if priority and len(job_submission_bundles) > 0:
                 return job_submission_bundles[0]
 
-            this_protocol_pipelines = proto_iface.fetch(protocol)
+            this_protocol_pipelines = proto_iface.fetch_pipelines(protocol)
             if not this_protocol_pipelines:
                 _LOGGER.warn("No mapping for protocol '%s' in %s", 
                              protocol, proto_iface)
@@ -2465,6 +2465,18 @@ def __repr__(self):
         return "ProtocolInterface from '{}'".format(self.source or "Mapping")
 
 
+    def fetch_pipelines(self, protocol):
+        """
+        Fetch the mapping for a particular protocol, null if unmapped.
+
+        :param str protocol: name/key for the protocol for which to fetch the
+            pipeline(s)
+        :return str | Iterable[str] | NoneType: pipeline(s) to which the given
+            protocol is mapped, otherwise null
+        """
+        return self.protomap.mappings.get(alpha_cased(protocol))
+
+
     def fetch_sample_subtype(
             self, protocol, strict_pipe_key, full_pipe_path):
         """
@@ -2499,21 +2511,26 @@ def fetch_sample_subtype(
             subtype_name = None
         else:
             if subtypes is None:
-                _LOGGER.debug("Null Sample subtypes specified for pipeline: "
-                              "'%s'; using base Sample type", strict_pipe_key)
-                # Designate lack of need to attempt pipeline module import.
+                # Designate lack of need for import attempt and provide
+                # class with name to format message below.
                 subtype = Sample
+                _LOGGER.debug("Null %s subtype(s) section specified for "
+                              "pipeline: '%s'; using base %s type",
+                              subtype.__name__, strict_pipe_key,
+                              subtype.__name__)
             elif isinstance(subtypes, str):
                 subtype_name = subtypes
                 _LOGGER.debug("Single subtype name for pipeline '%s' "
                               "in interface from '%s': '%s'", subtype_name,
                               strict_pipe_key, self.source)
             else:
+                temp_subtypes = {
+                        alpha_cased(p): st for p, st in subtypes.items()}
                 try:
-                    temp_subtypes = {alpha_cased(p): st
-                                     for p, st in subtypes.items()}
                     subtype_name = temp_subtypes[alpha_cased(protocol)]
                 except KeyError:
+                    # Designate lack of need for import attempt and provide
+                    # class with name to format message below.
                     subtype = Sample
                     _LOGGER.debug("No %s subtype specified in interface from "
                                   "'%s': '%s', '%s'; known: %s",
@@ -2523,24 +2540,32 @@ def fetch_sample_subtype(
 
         # subtype_name is defined if and only if subtype remained null.
         subtype = subtype or \
-                  _import_sample_subtype(full_pipe_path, subtype_name)
+                  _import_sample_subtype(full_pipe_path, subtype_name) or \
+                  Sample
         _LOGGER.debug("Using Sample subtype: %s", subtype.__name__)
         return subtype
 
 
-    def fetch(self, protocol):
-        """
-        Fetch the mapping for a particular protocol, null if unmapped.
-
-        :param str protocol:
-        :return str | Iterable[str] | NoneType: pipeline(s) to which the given
-            protocol is mapped, otherwise null
+    @classmethod
+    def _parse_iface_data(cls, pipe_iface_data):
         """
-        return self.protomap.mappings.get(alpha_cased(protocol))
+        Parse data from mappings to set instance attributes.
 
+        The data that define a ProtocolInterface are a "protocol_mapping"
+        Mapping and a "pipelines" Mapping, which are used to create a
+        ProtocolMapper and a PipelineInterface, representing the configuration
+        data for pipeline(s) from a single location. There are a couple of
+        different ways (file, folder, and eventually, raw Mapping) to provide
+        this data, and this function provides some standardization to how
+        those data are processed, independent of input type/format.
 
-    @classmethod
-    def _parse_iface_data(cls, pipe_iface_data):
+        :param Mapping[str, Mapping] pipe_iface_data: mapping from section
+            name to section data mapping; more specifically, the protocol
+            mappings Mapping and the PipelineInterface mapping
+        :return list[(str, ProtocolMapper | PipelineInterface)]: pairs of
+            attribute name for the ProtocolInterface being created, and the
+            value for that attribute,
+        """
         assignments = [("protocol_mapping", ProtocolMapper, "protomap"),
                        ("pipelines", PipelineInterface, "pipe_iface")]
         attribute_values = []

tests/models/independent/test_ProtocolInterface.py

@@ -2,14 +2,15 @@
 
 import __builtin__
 import inspect
+import itertools
 import logging
 import mock
 import os
 import pytest
 import yaml
 import looper
 from looper import models, DEV_LOGGING_FMT
-from looper.models import ProtocolInterface
+from looper.models import ProtocolInterface, Sample
 
 
 __author__ = "Vince Reuter"
@@ -192,6 +193,12 @@ class SampleSubtypeTests:
     # 2b -- named class is in module but isn't defined
     #
 
+    PROTOCOL_NAME_VARIANTS = [
+            "ATAC-Seq", "ATACSeq", "ATACseq", "ATAC-seq", "ATAC",
+            "ATACSEQ", "ATAC-SEQ", "atac", "atacseq", "atac-seq"]
+
+
+
     @pytest.fixture(scope="function")
     def subtypes_section_single(self, atac_pipe_name):
         pass
@@ -204,33 +211,86 @@ def subtypes_section_multiple(self, atac_pipe_name):
 
     @pytest.mark.parametrize(
             argnames="pipe_key",
-            argvalues=["ATAC-Seq.py", "atacseq.py", "ATACSEQ.py", "ATACSEQ",
-                       "atacseq", "ATAC-seq.py", "ATACseq.py"])
+            argvalues=["{}.py".format(proto) for proto
+                       in PROTOCOL_NAME_VARIANTS])
     @pytest.mark.parametrize(
             argnames="protocol",
-            argvalues=["ATAC-Seq", "ATACSeq", "ATACseq", "ATAC-seq", "ATAC",
-                       "ATACSEQ", "ATAC-SEQ", "atac", "atacseq", "atac-seq"])
-    def test_pipeline_key_close_matches_dont_count(
+            argvalues=PROTOCOL_NAME_VARIANTS)
+    def test_pipeline_key_match_is_strict(
             self, tmpdir, pipe_key, protocol, atac_pipe_name,
             atacseq_iface_with_resources):
         """ Request for Sample subtype for unmapped pipeline is KeyError. """
+
+        # Create the ProtocolInterface.
         strict_pipe_key = atac_pipe_name
         protocol_mapping = {protocol: strict_pipe_key}
-        path_config_file = _write_config_data(
-                protomap=protocol_mapping,
-                conf_data={strict_pipe_key: atacseq_iface_with_resources},
-                dirpath=tmpdir.strpath)
-        piface = ProtocolInterface(path_config_file)
+        confpath = _write_config_data(
+                protomap=protocol_mapping, dirpath=tmpdir.strpath,
+                conf_data={strict_pipe_key: atacseq_iface_with_resources})
+        piface = ProtocolInterface(confpath)
+
+        # The absolute pipeline path is the pipeline name, joined to the
+        # ProtocolInterface's pipelines location. This location is the
+        # location from which a Sample subtype import is attempted.
         full_pipe_path = os.path.join(tmpdir.strpath, atac_pipe_name)
-        with pytest.raises(KeyError):
-            # Mismatch between pipeline key arg and strict key --> KeyError.
+
+        # TODO: update to pytest.raises(None) if/when 3.1 adoption.
+        # Match between pipeline key specified and the strict key used in
+        # the mapping --> no error while mismatch --> error.
+        if pipe_key == atac_pipe_name:
             piface.fetch_sample_subtype(
-                    protocol, pipe_key, full_pipe_path=full_pipe_path)
+                protocol, pipe_key, full_pipe_path=full_pipe_path)
+        else:
+            with pytest.raises(KeyError):
+                piface.fetch_sample_subtype(
+                        protocol, pipe_key, full_pipe_path=full_pipe_path)
 
 
-    def test_protocol_match_is_fuzzy(self):
+    @pytest.mark.parametrize(
+            argnames=["mapped_protocol", "requested_protocol"],
+            argvalues=itertools.combinations(PROTOCOL_NAME_VARIANTS, 2))
+    def test_protocol_match_is_fuzzy(
+            self, tmpdir, mapped_protocol, atac_pipe_name,
+            requested_protocol, atacseq_piface_data):
         """ Punctuation and case mismatches are tolerated in protocol name. """
-        pass
+
+        # Needed to create the ProtocolInterface.
+        protomap = {mapped_protocol: atac_pipe_name}
+        # Needed to invoke the function under test.
+        full_pipe_path = os.path.join(tmpdir.strpath, atac_pipe_name)
+
+        # PipelineInterface data provided maps name to actual interface data
+        # Mapping, so modify the ATAC-Seq mapping within that.
+        # In this test, we're interested in the resolution of the protocol
+        # name, that with it we can grab the name of a class. Thus, we
+        # need only an arbitrary class name about which we can make the
+        # relevant assertion(s).
+        test_class_name = "TotallyArbitrary"
+        atacseq_piface_data[atac_pipe_name]["sample_subtypes"] = \
+                test_class_name
+
+        # Write out configuration data and create the ProtocolInterface.
+        conf_path = _write_config_data(
+                protomap=protomap, conf_data=atacseq_piface_data,
+                dirpath=tmpdir.strpath)
+        piface = ProtocolInterface(conf_path)
+
+        # Make the call under test, patching the function protected
+        # function that's called iff the protocol name match succeeds.
+        with mock.patch("looper.models._import_sample_subtype",
+                        return_value=None) as mocked_import:
+            # Return value is irrelevant; the effect of the protocol name
+            # match/resolution is entirely observable via the argument to the
+            # protected import function.
+            piface.fetch_sample_subtype(
+                    protocol=requested_protocol,
+                    strict_pipe_key=atac_pipe_name,
+                    full_pipe_path=full_pipe_path)
+        # When the protocol name match/resolution succeeds, the name of the
+        # Sample subtype class to which it was mapped is passed as an
+        # argument to the protected import function.
+        mocked_import.assert_called_with(full_pipe_path, test_class_name)
+
 
 
     @pytest.mark.parametrize(
@@ -239,6 +299,7 @@ def test_protocol_match_is_fuzzy(self):
                     __builtin__, lambda o: inspect.isclass(o) and
                                            issubclass(o, BaseException)))[1])
     def test_problematic_import_builtin_exception(self, error_type):
+        """ Base Sample is used if builtin exception on pipeline import. """
         pass
 
 
@@ -248,10 +309,12 @@ def test_problematic_import_builtin_exception(self, error_type):
                     looper.models, lambda o: inspect.isclass(o) and
                                              issubclass(o, Exception)))[1])
     def test_problematic_import_custom_exception(self, error_type):
+        """ Base Sample is used if custom exception on pipeline import. """
         pass
 
 
     def test_no_subtypes_section(self):
+        """  """
         pass