Improved tool test API.

Cleanup up internal representations used to bridge the tool parser and the tool testing modules to produce more sensible dictified representations when exported via the API. This is mostly replacing tuples (which get converted to JSON lists) to dicts but there are some other small cleanups.

lib/galaxy/tools/parser/interface.py

@@ -375,9 +375,10 @@ class TestCollectionDef(object):
             element_identifier = element_attrib["name"]
             nested_collection_elem = element.find("collection")
             if nested_collection_elem is not None:
-                elements.append((element_identifier, TestCollectionDef.from_xml(nested_collection_elem, parse_param_elem)))
+                element_definition = TestCollectionDef.from_xml(nested_collection_elem, parse_param_elem)
             else:
-                elements.append((element_identifier, parse_param_elem(element)))
+                element_definition = parse_param_elem(element)
+            elements.append({"element_identifier": element_identifier, "element_definition": element_definition})
 
         return TestCollectionDef(
             attrib=attrib,
@@ -387,18 +388,18 @@ class TestCollectionDef(object):
         )
 
     def to_dict(self):
-        def element_to_dict(element_pair):
-            element_identifier, element_def = element_pair
+        def element_to_dict(element_dict):
+            element_identifier, element_def = element_dict["element_identifier"], element_dict["element_definition"]
             if isinstance(element_def, TestCollectionDef):
                 element_def = element_def.to_dict()
             return {
                 "element_identifier": element_identifier,
-                "element_def": element_def,
+                "element_definition": element_def,
             }
 
         return {
             "model_class": "TestCollectionDef",
-            "attrib": self.attrib,
+            "attributes": self.attrib,
             "collection_type": self.collection_type,
             "elements": map(element_to_dict, self.elements or []),
             "name": self.name,
@@ -409,27 +410,24 @@ class TestCollectionDef(object):
         assert as_dict["model_class"] == "TestCollectionDef"
 
         def element_from_dict(element_dict):
-            if "element_def" not in element_dict:
+            if "element_definition" not in element_dict:
                 raise Exception("Invalid element_dict %s" % element_dict)
-            element_def = element_dict["element_def"]
-            # TODO: stop using tuples and use dicts internally to eliminate this check
-            if not isinstance(element_def, dict):
-                pass
-            elif element_def.get("model_class", None) == "TestCollectionDef":
+            element_def = element_dict["element_definition"]
+            if element_def.get("model_class", None) == "TestCollectionDef":
                 element_def = TestCollectionDef.from_dict(element_def)
-            return (element_dict["element_identifier"], element_def)
+            return {"element_identifier": element_dict["element_identifier"], "element_definition": element_def}
 
         return TestCollectionDef(
-            attrib=as_dict["attrib"],
+            attrib=as_dict["attributes"],
             name=as_dict["name"],
-            elements=map(element_from_dict, as_dict["elements"] or []),
+            elements=list(map(element_from_dict, as_dict["elements"] or [])),
             collection_type=as_dict["collection_type"],
         )
 
     def collect_inputs(self):
         inputs = []
         for element in self.elements:
-            value = element[1]
+            value = element["element_definition"]
             if isinstance(value, TestCollectionDef):
                 inputs.extend(value.collect_inputs())
             else:
@@ -451,13 +449,13 @@ class TestCollectionOutputDef(object):
     def from_dict(as_dict):
         return TestCollectionOutputDef(
             name=as_dict["name"],
-            attrib=as_dict["attrib"],
+            attrib=as_dict["attributes"],
             element_tests=as_dict["element_tests"],
         )
 
     def to_dict(self):
         return dict(
             name=self.name,
-            attrib=self.attrib,
+            attributes=self.attrib,
             element_tests=self.element_tests
         )

lib/galaxy/tools/parser/xml.py

@@ -416,7 +416,7 @@ def _test_elem_to_dict(test_elem, i):
         expect_failure=string_as_bool(test_elem.get("expect_failure", False)),
         maxseconds=test_elem.get("maxseconds", None),
     )
-    _copy_to_dict_if_present(test_elem, rval, ["interactor", "num_outputs"])
+    _copy_to_dict_if_present(test_elem, rval, ["num_outputs"])
     return rval
 
 
@@ -429,7 +429,7 @@ def __parse_output_elems(test_elem):
     outputs = []
     for output_elem in test_elem.findall("output"):
         name, file, attributes = __parse_output_elem(output_elem)
-        outputs.append([name, file, attributes])
+        outputs.append({"name": name, "value": file, "attributes": attributes})
     return outputs
 
 
@@ -563,7 +563,12 @@ def __parse_extra_files_elem(extra):
     assert extra_type == 'directory' or extra_name is not None, \
         'extra_files type (%s) requires a name attribute' % extra_type
     extra_value, extra_attributes = __parse_test_attributes(extra, attrib)
-    return extra_type, extra_value, extra_name, extra_attributes
+    return {
+        "value": extra_value,
+        "name": extra_name,
+        "type": extra_type,
+        "attributes": extra_attributes
+    }
 
 
 def __expand_input_elems(root_elem, prefix=""):
@@ -626,8 +631,8 @@ def _copy_to_dict_if_present(elem, rval, attributes):
 def __parse_inputs_elems(test_elem, i):
     raw_inputs = []
     for param_elem in test_elem.findall("param"):
-        name, value, attrib = __parse_param_elem(param_elem, i)
-        raw_inputs.append((name, value, attrib))
+        raw_inputs.append(__parse_param_elem(param_elem, i))
+
     return raw_inputs
 
 
@@ -671,7 +676,11 @@ def __parse_param_elem(param_elem, i=0):
             # take precedence
             attrib['edit_attributes'].insert(0, {'type': 'name', 'value': composite_data_name})
     name = attrib.pop('name')
-    return (name, value, attrib)
+    return {
+        "name": name,
+        "value": value,
+        "attributes": attrib
+    }
 
 
 class StdioParser(object):

lib/galaxy/tools/parser/yaml.py

@@ -188,7 +188,7 @@ def _parse_test(i, test_dict):
     if _is_dict(inputs):
         new_inputs = []
         for key, value in inputs.items():
-            new_inputs.append((key, value, {}))
+            new_inputs.append({"name": key, "value": value, "attributes": {}})
         test_dict["inputs"] = new_inputs
 
     outputs = test_dict["outputs"]
@@ -202,7 +202,11 @@ def _parse_test(i, test_dict):
             else:
                 file = value
                 attributes = {}
-            new_outputs.append((key, file, attributes))
+            new_outputs.append({
+                "name": key,
+                "value": file,
+                "attributes": attributes
+            })
     else:
         for output in outputs:
             name = output["name"]
@@ -211,7 +215,7 @@ def _parse_test(i, test_dict):
             new_outputs.append((name, value, attributes))
 
     for output in new_outputs:
-        attributes = output[2]
+        attributes = output["attributes"]
         defaults = {
             'compare': 'diff',
             'lines_diff': 0,

lib/galaxy/tools/test.py

@@ -40,8 +40,9 @@ def description_from_tool_object(tool, test_index, raw_test_dict):
         num_outputs = int(num_outputs)
 
     try:
+        processed_inputs = _process_raw_inputs(tool, tool.inputs, raw_test_dict["inputs"], required_files)
         processed_test_dict = {
-            "inputs": _process_raw_inputs(tool, tool.inputs, raw_test_dict["inputs"], required_files),
+            "inputs": processed_inputs,
             "outputs": raw_test_dict["outputs"],
             "output_collections": raw_test_dict["output_collections"],
             "num_outputs": num_outputs,
@@ -50,13 +51,13 @@ def description_from_tool_object(tool, test_index, raw_test_dict):
             "stderr": raw_test_dict.get("stderr", None),
             "expect_exit_code": raw_test_dict.get("expect_exit_code", None),
             "expect_failure": raw_test_dict.get("expect_failure", False),
-            "md5": raw_test_dict.get("md5", None),
             "required_files": required_files,
             "tool_id": tool.id,
             "test_index": test_index,
             "error": False,
         }
     except Exception as e:
+        log.exception("Failed to load tool test number [%d] for %s" % (test_index, tool.id))
         processed_test_dict = {
             "tool_id": tool.id,
             "test_index": test_index,
@@ -80,8 +81,8 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, parent_co
         if isinstance(value, galaxy.tools.parameters.grouping.Conditional):
             cond_context = ParamContext(name=value.name, parent_context=parent_context)
             case_context = ParamContext(name=value.test_param.name, parent_context=cond_context)
-            raw_input = case_context.extract_value(raw_inputs)
-            case_value = raw_input[1] if raw_input else None
+            raw_input_dict = case_context.extract_value(raw_inputs)
+            case_value = raw_input_dict["value"] if raw_input_dict else None
             case = _matching_case_for_value(tool, value, case_value)
             if case:
                 for input_name, input_value in case.inputs.items():
@@ -120,9 +121,11 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, parent_co
                 repeat_index += 1
         else:
             context = ParamContext(name=value.name, parent_context=parent_context)
-            raw_input = context.extract_value(raw_inputs)
-            if raw_input:
-                (name, param_value, param_extra) = raw_input
+            raw_input_dict = context.extract_value(raw_inputs)
+            if raw_input_dict:
+                name = raw_input_dict["name"]
+                param_value = raw_input_dict["value"]
+                param_extra = raw_input_dict["attributes"]
                 if not value.type == "text":
                     param_value = _split_if_str(param_value)
                 if isinstance(value, galaxy.tools.parameters.basic.DataToolParameter):
@@ -133,8 +136,11 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, parent_co
                 elif isinstance(value, galaxy.tools.parameters.basic.DataCollectionToolParameter):
                     assert 'collection' in param_extra
                     collection_def = param_extra['collection']
-                    for (name, value, extra) in collection_def.collect_inputs():
-                        require_file(name, value, extra, required_files)
+                    for input_dict in collection_def.collect_inputs():
+                        name = input_dict["name"]
+                        value = input_dict["value"]
+                        attributes = input_dict["attributes"]
+                        require_file(name, value, attributes, required_files)
                     processed_value = collection_def
                 else:
                     processed_value = _process_simple_value(value, param_value)
@@ -309,13 +315,13 @@ class ParamContext(object):
 
     def __raw_param_found(self, param_name, raw_inputs):
         index = None
-        for i, raw_input in enumerate(raw_inputs):
-            if raw_input[0] == param_name:
+        for i, raw_input_dict in enumerate(raw_inputs):
+            if raw_input_dict["name"] == param_name:
                 index = i
         if index is not None:
-            raw_input = raw_inputs[index]
+            raw_input_dict = raw_inputs[index]
             del raw_inputs[index]
-            return raw_input
+            return raw_input_dict
         else:
             return None
 

lib/galaxy/tools/verify/interactor.py

@@ -342,19 +342,20 @@ class GalaxyInteractorApi(object):
 
     def _element_identifiers(self, collection_def):
         element_identifiers = []
-        for (element_identifier, element) in collection_def.elements:
-            if isinstance(element, TestCollectionDef):
-                subelement_identifiers = self._element_identifiers(element)
+        for element_dict in collection_def.elements:
+            element_identifier = element_dict["element_identifier"]
+            element_def = element_dict["element_definition"]
+            if isinstance(element_def, TestCollectionDef):
+                subelement_identifiers = self._element_identifiers(element_def)
                 element = dict(
                     name=element_identifier,
                     src="new_collection",
-                    collection_type=element.collection_type,
+                    collection_type=element_def.collection_type,
                     element_identifiers=subelement_identifiers
                 )
             else:
-                element_name = element[0]
-                element = self.uploads[element[1]].copy()
-                element["name"] = element_name
+                element = self.uploads[element_def["value"]].copy()
+                element["name"] = element_identifier
             element_identifiers.append(element)
         return element_identifiers
 
@@ -621,14 +622,19 @@ def _verify_composite_datatype_file_content(file_name, hda_id, base_name=None, a
 
 def _verify_extra_files_content(extra_files, hda_id, dataset_fetcher, test_data_path_builder, keep_outputs_dir):
     files_list = []
-    for extra_type, extra_value, extra_name, extra_attributes in extra_files:
-        if extra_type == 'file':
-            files_list.append((extra_name, extra_value, extra_attributes))
-        elif extra_type == 'directory':
-            for filename in os.listdir(test_data_path_builder(extra_value)):
-                files_list.append((filename, os.path.join(extra_value, filename), extra_attributes))
+    for extra_file_dict in extra_files:
+        extra_file_type = extra_file_dict["type"]
+        extra_file_name = extra_file_dict["name"]
+        extra_file_attributes = extra_file_dict["attributes"]
+        extra_file_value = extra_file_dict["value"]
+
+        if extra_file_type == 'file':
+            files_list.append((extra_file_name, extra_file_value, extra_file_attributes))
+        elif extra_file_type == 'directory':
+            for filename in os.listdir(test_data_path_builder(extra_file_value)):
+                files_list.append((filename, os.path.join(extra_file_value, filename), extra_file_attributes))
         else:
-            raise ValueError('unknown extra_files type: %s' % extra_type)
+            raise ValueError('unknown extra_files type: %s' % extra_file_type)
     for filename, filepath, attributes in files_list:
         _verify_composite_datatype_file_content(filepath, hda_id, base_name=filename, attributes=attributes, dataset_fetcher=dataset_fetcher, test_data_path_builder=test_data_path_builder, keep_outputs_dir=keep_outputs_dir)
 
@@ -758,9 +764,11 @@ def _verify_outputs(testdef, history, jobs, tool_id, data_list, data_collection_
             error = AssertionError("Expected job to complete with exit code %s, found %s" % (expect_exit_code, exit_code))
             register_exception(error)
 
-    for output_index, output_tuple in enumerate(testdef.outputs):
+    for output_index, output_dict in enumerate(testdef.outputs):
         # Get the correct hid
-        name, outfile, attributes = output_tuple
+        name = output_dict["name"]
+        outfile = output_dict["value"]
+        attributes = output_dict["attributes"]
         output_testdef = Bunch(name=name, outfile=outfile, attributes=attributes)
         try:
             output_data = data_list[name]
@@ -922,7 +930,6 @@ class ToolTestDescription(object):
         self.stderr = processed_test_dict.get("stderr", None)
         self.expect_exit_code = processed_test_dict.get("expect_exit_code", None)
         self.expect_failure = processed_test_dict.get("expect_failure", False)
-        self.md5 = processed_test_dict.get("md5", None)
 
     def test_data(self):
         """
@@ -948,7 +955,6 @@ class ToolTestDescription(object):
             "stderr": self.stderr,
             "expect_exit_code": self.expect_exit_code,
             "expect_failure": self.expect_failure,
-            "md5": self.md5,
             "name": self.name,
             "test_index": self.test_index,
             "tool_id": self.tool_id,

test/unit/tools/test_parsing.py

@@ -201,17 +201,17 @@ class XmlLoaderTestCase(BaseLoaderTestCase):
         assert len(tests) == 2
         test_dict = tests[0]
         inputs = test_dict["inputs"]
-        assert len(inputs) == 1
+        assert len(inputs) == 1, test_dict
         input1 = inputs[0]
-        assert input1[0] == "foo"
-        assert input1[1] == "5"
+        assert input1["name"] == "foo", input1
+        assert input1["value"] == "5"
 
         outputs = test_dict["outputs"]
         assert len(outputs) == 1
         output1 = outputs[0]
-        assert output1[0] == 'out1'
-        assert output1[1] == 'moo.txt'
-        attributes1 = output1[2]
+        assert output1["name"] == 'out1'
+        assert output1["value"] == 'moo.txt'
+        attributes1 = output1["attributes"]
         assert attributes1["compare"] == "diff"
         assert attributes1["lines_diff"] == 0
 
@@ -219,9 +219,9 @@ class XmlLoaderTestCase(BaseLoaderTestCase):
         outputs = test2["outputs"]
         assert len(outputs) == 1
         output2 = outputs[0]
-        assert output2[0] == 'out1'
-        assert output2[1] is None
-        attributes1 = output2[2]
+        assert output2["name"] == 'out1'
+        assert output2["value"] is None
+        attributes1 = output2["attributes"]
         assert attributes1["compare"] == "sim_size"
         assert attributes1["lines_diff"] == 4
 
@@ -350,15 +350,15 @@ class YamlLoaderTestCase(BaseLoaderTestCase):
         inputs = test_dict["inputs"]
         assert len(inputs) == 1
         input1 = inputs[0]
-        assert input1[0] == "foo"
-        assert input1[1] == 5
+        assert input1["name"] == "foo"
+        assert input1["value"] == 5
 
         outputs = test_dict["outputs"]
         assert len(outputs) == 1
         output1 = outputs[0]
-        assert output1[0] == 'out1'
-        assert output1[1] == 'moo.txt'
-        attributes1 = output1[2]
+        assert output1["name"] == 'out1'
+        assert output1["value"] == 'moo.txt'
+        attributes1 = output1["attributes"]
         assert attributes1["compare"] == "diff"
         assert attributes1["lines_diff"] == 0
 
@@ -366,9 +366,9 @@ class YamlLoaderTestCase(BaseLoaderTestCase):
         outputs = test2["outputs"]
         assert len(outputs) == 1
         output2 = outputs[0]
-        assert output2[0] == 'out1'
-        assert output2[1] is None
-        attributes1 = output2[2]
+        assert output2["name"] == 'out1'
+        assert output2["value"] is None
+        attributes1 = output2["attributes"]
         assert attributes1["compare"] == "sim_size"
         assert attributes1["lines_diff"] == 4
 
@@ -448,3 +448,14 @@ class SpecialToolLoaderTestCase(BaseLoaderTestCase):
         action = self._tool_source.parse_action_module()
         assert action[0] == "galaxy.tools.actions.history_imp_exp"
         assert action[1] == "ExportHistoryToolAction"
+
+
+class CollectionTestCase(BaseLoaderTestCase):
+    source_file_name = os.path.join(os.getcwd(), "test/functional/tools/collection_two_paired.xml")
+    source_contents = None
+
+    def test_tests(self):
+        tests_dict = self._tool_source.parse_tests_to_dict()
+        tests = tests_dict["tests"]
+        assert len(tests) == 2
+        assert len(tests[0]["inputs"]) == 3, tests[0]