Merge pull request #15482 from bernt-matthias/test/profile-format-input

Add framework test for profile behavior of `format="input"`

lib/galaxy/tool_util/linters/inputs.py

@@ -558,8 +558,11 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"):
                 )
 
 
-def lint_repeats(tool_xml, lint_ctx):
+def lint_repeats(tool_source: "ToolSource", lint_ctx):
     """Lint repeat blocks in tool inputs."""
+    tool_xml = getattr(tool_source, "xml_tree", None)
+    if tool_xml is None:
+        return
     repeats = tool_xml.findall("./inputs//repeat")
     for repeat in repeats:
         if "name" not in repeat.attrib:

lib/galaxy/tool_util/linters/outputs.py

@@ -1,14 +1,27 @@
 """This module contains a linting functions for tool outputs."""
+from typing import TYPE_CHECKING
+
+from packaging.version import Version
+
 from galaxy.util import (
     etree,
     string_as_bool,
 )
 from ._util import is_valid_cheetah_placeholder
 from ..parser.output_collection_def import NAMED_PATTERNS
 
+if TYPE_CHECKING:
+    from galaxy.tool_util.lint import LintContext
+    from galaxy.tool_util.parser import ToolSource
 
-def lint_output(tool_xml, lint_ctx):
+
+def lint_output(tool_source: "ToolSource", lint_ctx: "LintContext"):
     """Check output elements, ensure there is at least one and check attributes."""
+    tool_xml = getattr(tool_source, "xml_tree", None)
+    if tool_xml is None:
+        return
+    profile = tool_source.parse_profile()
+
     outputs = tool_xml.findall("./outputs")
     # determine node to report for general problems with outputs
     tool_node = tool_xml.find("./outputs")
@@ -56,7 +69,7 @@ def lint_output(tool_xml, lint_ctx):
         labels.add(label)
 
         format_set = False
-        if __check_format(output, lint_ctx):
+        if __check_format(output, lint_ctx, profile):
             format_set = True
         if output.tag == "data":
             if "auto_format" in output.attrib and output.attrib["auto_format"]:
@@ -70,7 +83,7 @@ def lint_output(tool_xml, lint_ctx):
         for sub in output:
             if __check_pattern(sub):
                 format_set = True
-            elif __check_format(sub, lint_ctx, allow_ext=True):
+            elif __check_format(sub, lint_ctx, profile, allow_ext=True):
                 format_set = True
 
         if not format_set:
@@ -83,7 +96,7 @@ def lint_output(tool_xml, lint_ctx):
     lint_ctx.info(f"{num_outputs} outputs found.", node=outputs[0])
 
 
-def __check_format(node, lint_ctx, allow_ext=False):
+def __check_format(node, lint_ctx, profile: str, allow_ext=False):
     """
     check if format/ext/format_source attribute is set in a given node
     issue a warning if the value is input
@@ -105,10 +118,12 @@ def __check_format(node, lint_ctx, allow_ext=False):
     if fmt is None:
         fmt = node.attrib.get("format")
     if fmt == "input":
-        lint_ctx.error(
-            f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.",
-            node=node,
-        )
+        message = f"Using format='input' on {node.tag} is deprecated. Use the format_source attribute."
+        if Version(str(profile)) <= Version("16.01"):
+            lint_ctx.warn(message, node=node)
+        else:
+            lint_ctx.error(message, node=node)
+
     return fmt is not None
 
 

test/functional/tools/output_format_input.xml

@@ -0,0 +1,26 @@
+<tool id="output_format_input" name="output_format_input" version="1.0" profile="21.01">
+  <!-- test format="input" for the profile version where the format is set to "input"
+       (for legacy tools, i.e. profile <16.04, the format of a ramdom input is used) -->
+  <description></description>
+  <command>
+    cat '$input' > '$output'
+  </command>
+  <inputs>
+    <param format="tabular" name="input" type="data" label="Select cells from"/>
+  </inputs>
+  <outputs>
+    <data format="input" name="output" metadata_source="input" />
+  </outputs>
+  <tests>
+    <test>
+      <param name="input" value="simple_line.txt" ftype="txt" />
+      <output name="output" file="simple_line.txt" ftype="input"/>
+    </test>
+    <test>
+      <param name="input" value="simple_line.txt" ftype="tabular" />
+      <output name="output" file="simple_line.txt" ftype="input"/>
+    </test>
+  </tests>
+  <help>
+  </help>
+</tool>

test/functional/tools/sample_tool_conf.xml

@@ -8,8 +8,9 @@
     <tool file="param_text_option.xml" />
     <tool file="column_param.xml" />
   </section>
-  <tool file="ucsc_tablebrowser.xml" />
-  <tool file="test_data_source.xml" />
+  <tool file="output_format_input.xml"/>
+  <tool file="ucsc_tablebrowser.xml"/>
+  <tool file="test_data_source.xml"/>
   <tool file="simple_constructs.xml" />
   <tool file="color_param.xml" />
   <tool file="inheritance_simple.xml" />

test/unit/tool_util/test_tool_linters.py

@@ -538,13 +538,20 @@
     </outputs>
 </tool>
 """
-OUTPUTS_FORMAT_INPUT = """
+OUTPUTS_FORMAT_INPUT_LEGACY = """
 <tool>
     <outputs>
         <data name="valid_name" format="input"/>
     </outputs>
 </tool>
 """
+OUTPUTS_FORMAT_INPUT = """
+<tool profile="16.04">
+    <outputs>
+        <data name="valid_name" format="input"/>
+    </outputs>
+</tool>
+"""
 
 # check that linter accepts format source for collection elements as means to specify format
 # and that the linter warns if format and format_source are used
@@ -911,15 +918,6 @@ def get_tool_xml_exact(xml_string: str):
         return parse_xml(tool_path, strip_whitespace=False, remove_comments=False)
 
 
-def failed_assert_print(lint_ctx):
-    return (
-        f"Valid: {lint_ctx.valid_messages}\n"
-        f"Info: {lint_ctx.info_messages}\n"
-        f"Warnings: {lint_ctx.warn_messages}\n"
-        f"Errors: {lint_ctx.error_messages}"
-    )
-
-
 def run_lint(lint_ctx, lint_func, lint_target):
     lint_ctx.lint(name="test_lint", lint_func=lint_func, lint_target=lint_target)
     # check if the lint messages have the line
@@ -1457,8 +1455,8 @@ def test_inputs_duplicate_names(lint_ctx):
 
 
 def test_inputs_repeats(lint_ctx):
-    tool_xml_tree = get_xml_tree(REPEATS)
-    run_lint(lint_ctx, inputs.lint_repeats, tool_xml_tree)
+    tool_source = get_xml_tool_source(REPEATS)
+    run_lint(lint_ctx, inputs.lint_repeats, tool_source)
     assert "Repeat does not specify name attribute." in lint_ctx.error_messages
     assert "Repeat does not specify title attribute." in lint_ctx.error_messages
     assert not lint_ctx.info_messages
@@ -1468,8 +1466,8 @@ def test_inputs_repeats(lint_ctx):
 
 
 def test_outputs_missing(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_MISSING)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_MISSING)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "Tool contains no outputs section, most tools should produce outputs." in lint_ctx.warn_messages
     assert not lint_ctx.info_messages
     assert not lint_ctx.valid_messages
@@ -1478,8 +1476,8 @@ def test_outputs_missing(lint_ctx):
 
 
 def test_outputs_multiple(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_MULTIPLE)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_MULTIPLE)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Tool contains multiple output sections, behavior undefined." in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1489,8 +1487,8 @@ def test_outputs_multiple(lint_ctx):
 
 
 def test_outputs_unknown_tag(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_UNKNOWN_TAG)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_UNKNOWN_TAG)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Unknown element found in outputs [output]" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1500,8 +1498,8 @@ def test_outputs_unknown_tag(lint_ctx):
 
 
 def test_outputs_unnamed_invalid_name(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_UNNAMED_INVALID_NAME)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_UNNAMED_INVALID_NAME)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "2 outputs found." in lint_ctx.info_messages
     assert "Tool output doesn't define a name - this is likely a problem." in lint_ctx.warn_messages
     assert "Tool data output with missing name doesn't define an output format." in lint_ctx.warn_messages
@@ -1514,23 +1512,31 @@ def test_outputs_unnamed_invalid_name(lint_ctx):
     assert not lint_ctx.error_messages
 
 
+def test_outputs_format_input_legacy(lint_ctx):
+    tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT_LEGACY)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
+    assert "1 outputs found." in lint_ctx.info_messages
+    assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.warn_messages
+    assert len(lint_ctx.info_messages) == 1
+    assert not lint_ctx.valid_messages
+    assert len(lint_ctx.warn_messages) == 1
+    assert not lint_ctx.error_messages
+
+
 def test_outputs_format_input(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_FORMAT_INPUT)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
-    assert (
-        "Using format='input' on data, format_source attribute is less ambiguous and should be used instead."
-        in lint_ctx.error_messages
-    )
+    assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.error_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
     assert not lint_ctx.warn_messages
     assert len(lint_ctx.error_messages) == 1
 
 
 def test_outputs_collection_format_source(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_COLLECTION_FORMAT_SOURCE)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_COLLECTION_FORMAT_SOURCE)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "Tool data output 'reverse' should use either format_source or format/ext" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1539,17 +1545,17 @@ def test_outputs_collection_format_source(lint_ctx):
 
 
 def test_outputs_format_action(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_FORMAT_ACTION)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_FORMAT_ACTION)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
     assert not lint_ctx.warn_messages
     assert not lint_ctx.error_messages
 
 
 def test_outputs_discover_tool_provided_metadata(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1558,8 +1564,8 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx):
 
 
 def test_outputs_duplicated_name_label(lint_ctx):
-    tool_xml_tree = get_xml_tree(OUTPUTS_DUPLICATED_NAME_LABEL)
-    run_lint(lint_ctx, outputs.lint_output, tool_xml_tree)
+    tool_source = get_xml_tool_source(OUTPUTS_DUPLICATED_NAME_LABEL)
+    run_lint(lint_ctx, outputs.lint_output, tool_source)
     assert "4 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages