Skip to content

Commit

Permalink
run xsd linter in all linter unit tests
Browse files Browse the repository at this point in the history
contrary to the previous linters the xsd linter covers more than one
section of the tool xml (i.e. all of them) and therefor may cover
other linters.

consequently all linters that are redundant with the xsd linter
are removed.
  • Loading branch information
bernt-matthias committed Dec 7, 2023
1 parent 0481e48 commit 8e2cb30
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 302 deletions.
11 changes: 0 additions & 11 deletions lib/galaxy/tool_util/linters/citations.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
lint_ctx.warn("No citations found, consider adding citations to your tool.", node=root)


class CitationsMultiple(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
citations = tool_xml.findall("citations")
if len(citations) > 1:
lint_ctx.error("More than one citation section found, behavior undefined.", node=citations[1])


class CitationsNoText(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down
11 changes: 0 additions & 11 deletions lib/galaxy/tool_util/linters/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,6 @@
from galaxy.tool_util.parser.interface import ToolSource


class CommandMultiple(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
commands = tool_xml.findall("./command")
if len(commands) > 1:
lint_ctx.error("More than one command tag found, behavior undefined.", node=commands[1])


class CommandMissing(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down
11 changes: 0 additions & 11 deletions lib/galaxy/tool_util/linters/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,6 @@
from galaxy.tool_util.parser.interface import ToolSource


class HelpMultiple(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
helps = tool_xml.findall("./help")
if len(helps) > 1:
lint_ctx.error("More than one help section found, behavior undefined.", node=helps[1])


class HelpMissing(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down
48 changes: 1 addition & 47 deletions lib/galaxy/tool_util/linters/inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
PARAM_TYPE_CHILD_COMBINATIONS = [
("./options", ["data", "select", "drill_down"]),
("./options/option", ["drill_down"]),
("./column", ["data_column"]),
("./options/column", ["select"]),
]

# TODO lint for valid param type - attribute combinations
Expand Down Expand Up @@ -1167,38 +1167,6 @@ def _iter_conditional(tool_xml: "ElementTree") -> Iterator[Tuple["Element", Opti
yield conditional, conditional_name, first_param, first_param_type


class ConditionalName(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
conditionals = tool_xml.findall("./inputs//conditional")
for conditional in conditionals:
conditional_name = conditional.get("name")
if not conditional_name:
lint_ctx.error("Conditional without a name", node=conditional)


class ConditionalParamNumber(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
conditionals = tool_xml.findall("./inputs//conditional")
for conditional in conditionals:
conditional_name = conditional.get("name")
if conditional.get("value_from"): # Probably only the upload tool use this, no children elements
continue
first_param = conditional.findall("param")
if len(first_param) != 1:
lint_ctx.error(
f"Conditional [{conditional_name}] needs exactly one child <param> found {len(first_param)}",
node=conditional,
)


class ConditionalParamTypeBool(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down Expand Up @@ -1242,20 +1210,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
)


class ConditionalWhenValue(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
for conditional, conditional_name, _, first_param_type in _iter_conditional(tool_xml):
if first_param_type not in ["boolean", "select"]:
continue
whens = conditional.findall("./when")
if any("value" not in when.attrib for when in whens):
lint_ctx.error(f"Conditional [{conditional_name}] when without value", node=conditional)


class ConditionalWhenMissing(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down
47 changes: 17 additions & 30 deletions lib/galaxy/tool_util/linters/outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@
from packaging.version import Version

from galaxy.tool_util.lint import Linter
from galaxy.util import 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
from galaxy.util.etree import Element
from galaxy.util.etree import Element, ElementTree


class OutputsMissing(Linter):
Expand All @@ -27,17 +26,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", node=tool_node)


class OutputsMultiple(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
outputs = tool_xml.findall("./outputs")
if len(outputs) > 1:
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", node=outputs[1])


class OutputsOutput(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand All @@ -49,18 +37,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
lint_ctx.warn("Avoid the use of 'output' and replace by 'data' or 'collection'", node=output)


class OutputsNameMissing(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
for output in tool_xml.findall("./outputs/data") + tool_xml.findall("./outputs/collection"):
if "name" not in output.attrib:
# TODO make this an error if there is no discover_datasets / from_work_dir (is this then still a problem)
lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", node=output)


class OutputsNameInvalidCheetah(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down Expand Up @@ -187,7 +163,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
if "structured_like" in output.attrib and "inherit_format" in output.attrib:
format_set = True
for sub in output:
if _check_pattern(sub):
if _check_pattern(sub) or _has_tool_provided_metadata(tool_xml):
format_set = True
elif _check_format(sub):
format_set = True
Expand Down Expand Up @@ -241,14 +217,25 @@ def _check_pattern(node):
"""
if node.tag != "discover_datasets":
return False
if "from_tool_provided_metadata" in node.attrib and string_as_bool(
node.attrib.get("from_tool_provided_metadata", "false")
):
return True
if "pattern" not in node.attrib:
return False
pattern = node.attrib["pattern"]
regex_pattern = NAMED_PATTERNS.get(pattern, pattern)
# TODO error on wrong pattern or non-regexp
if "(?P<ext>" in regex_pattern:
return True


def _has_tool_provided_metadata(tool_xml: "ElementTree") -> bool:
outputs = tool_xml.find("./outputs")
if outputs is not None:
if "provided_metadata_file" in outputs.attrib or "provided_metadata_style" in outputs.attrib:
return True
command = tool_xml.find("./command")
if command is not None:
if "galaxy.json" in command.text:
return True
config = tool_xml.find("./configfiles/configfile[@filename='galaxy.json']")
if config is not None:
return True
return False
15 changes: 0 additions & 15 deletions lib/galaxy/tool_util/linters/stdio.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
)


class StdIOMultiple(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
# Can only lint XML tools at this point.
# Should probably use tool_source.parse_stdio() to abstract away XML details
return
stdios = tool_xml.findall("./stdio") if tool_xml else []

if len(stdios) > 1:
lint_ctx.error("More than one stdio tag found, behavior undefined.", node=stdios[1])
return


class StdIORegex(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down
81 changes: 2 additions & 79 deletions lib/galaxy/tool_util/linters/tests.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
"""This module contains a linting functions for tool tests."""
from inspect import (
Parameter,
signature,
)
from typing import (
List,
Optional,
Expand All @@ -12,7 +8,6 @@
from galaxy.tool_util.lint import Linter
from galaxy.util import asbool
from ._util import is_datasource
from ..verify import asserts

if TYPE_CHECKING:
from galaxy.tool_util.lint import LintContext
Expand Down Expand Up @@ -66,66 +61,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
)


class TestsAssertsUnknown(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
tests = tool_xml.findall("./tests/test")
for test_idx, test in enumerate(tests, start=1):
for a in test.xpath(
".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*"
):
assert_function_name = "assert_" + a.tag
if assert_function_name not in asserts.assertion_functions:
lint_ctx.error(f"Test {test_idx}: unknown assertion '{a.tag}'", node=a)


class TestsAssertsUnknownAttrib(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
tests = tool_xml.findall("./tests/test")
for test_idx, test in enumerate(tests, start=1):
for a in test.xpath(
".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*"
):
assert_function_name = "assert_" + a.tag
if assert_function_name not in asserts.assertion_functions:
continue
assert_function_sig = signature(asserts.assertion_functions[assert_function_name])
# check of the attributes
for attrib in a.attrib:
if attrib not in assert_function_sig.parameters:
lint_ctx.error(f"Test {test_idx}: unknown attribute '{attrib}' for '{a.tag}'", node=a)


class TestsAssertsMissingAttrib(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
tests = tool_xml.findall("./tests/test")
for test_idx, test in enumerate(tests, start=1):
for a in test.xpath(
".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*"
):
assert_function_name = "assert_" + a.tag
if assert_function_name not in asserts.assertion_functions:
continue
assert_function_sig = signature(asserts.assertion_functions[assert_function_name])
# check missing required attributes
for p in assert_function_sig.parameters:
if p in ["output", "output_bytes", "verify_assertions_function", "children"]:
continue
if assert_function_sig.parameters[p].default is Parameter.empty and p not in a.attrib:
lint_ctx.error(f"Test {test_idx}: missing attribute '{p}' for '{a.tag}'", node=a)


class TestsAssertsHasNQuant(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
Expand Down Expand Up @@ -181,19 +116,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
)


class TestsParamName(Linter):
@classmethod
def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
tool_xml = getattr(tool_source, "xml_tree", None)
if not tool_xml:
return
tests = tool_xml.findall("./tests/test")
for test_idx, test in enumerate(tests, start=1):
for param in test.findall("param"):
if not param.attrib.get("name", None):
lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", node=param)


class TestsParamInInputs(Linter):
"""
really simple linter that test parameters are also present in the inputs
Expand Down Expand Up @@ -233,7 +155,8 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
return
tests = tool_xml.findall("./tests/test")
for test_idx, test in enumerate(tests, start=1):
for output in test.findall("output") + test.findall("output_collection"):
# note output_collections are covered by xsd, but output is not required to have one by xsd
for output in test.findall("output"):
if not output.attrib.get("name", None):
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", node=output)

Expand Down
Loading

0 comments on commit 8e2cb30

Please sign in to comment.