mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Merge pull request #18091 from bernt-matthias/skip-name-restore
[24.0] Tool linters: allow to skip by old linter names (by allowing to skip linter modules)
This commit is contained in:
@@ -212,11 +212,19 @@ class LintContext:
|
||||
def found_warns(self) -> bool:
|
||||
return len(self.warn_messages) > 0
|
||||
|
||||
def lint(self, name: str, lint_func: Callable[[LintTargetType, "LintContext"], None], lint_target: LintTargetType):
|
||||
def lint(
|
||||
self,
|
||||
name: str,
|
||||
lint_func: Callable[[LintTargetType, "LintContext"], None],
|
||||
lint_target: LintTargetType,
|
||||
module_name: Optional[str] = None,
|
||||
):
|
||||
if name.startswith("lint_"):
|
||||
name = name[len("lint_") :]
|
||||
if name in self.skip_types:
|
||||
return
|
||||
if module_name and module_name in self.skip_types:
|
||||
return
|
||||
|
||||
if self.level < LintLevel.SILENT:
|
||||
# this is a relict from the past where the lint context
|
||||
@@ -355,6 +363,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
|
||||
tool_type = tool_source.parse_tool_type() or "default"
|
||||
|
||||
for module in linter_modules:
|
||||
module_name = module.__name__.split(".")[-1]
|
||||
lint_tool_types = getattr(module, "lint_tool_types", ["default", "manage_data"])
|
||||
if not ("*" in lint_tool_types or tool_type in lint_tool_types):
|
||||
continue
|
||||
@@ -374,7 +383,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
|
||||
else:
|
||||
lint_context.lint(name, value, tool_source)
|
||||
elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
|
||||
lint_context.lint(name, value.lint, tool_source)
|
||||
lint_context.lint(name, value.lint, tool_source, module_name=module_name)
|
||||
return lint_context
|
||||
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import inspect
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
import galaxy.tool_util.linters
|
||||
from galaxy.tool_util.lint import (
|
||||
lint_tool_source_with,
|
||||
lint_tool_source_with_modules,
|
||||
@@ -18,7 +20,7 @@ from galaxy.tool_util.linters import (
|
||||
general,
|
||||
help,
|
||||
inputs,
|
||||
outputs,
|
||||
output,
|
||||
stdio,
|
||||
tests,
|
||||
xml_order,
|
||||
@@ -29,6 +31,7 @@ from galaxy.tool_util.parser.xml import XmlToolSource
|
||||
from galaxy.util import (
|
||||
ElementTree,
|
||||
parse_xml,
|
||||
submodules,
|
||||
)
|
||||
from galaxy.util.xml_macros import load_with_references
|
||||
|
||||
@@ -1508,7 +1511,7 @@ def test_inputs_repeats(lint_ctx):
|
||||
|
||||
def test_outputs_missing(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_MISSING)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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
|
||||
@@ -1518,7 +1521,7 @@ def test_outputs_missing(lint_ctx):
|
||||
|
||||
def test_outputs_multiple(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_MULTIPLE)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, output, tool_source)
|
||||
assert "0 outputs found." in lint_ctx.info_messages
|
||||
assert "Invalid XML: Element 'outputs': This element is not expected." in lint_ctx.error_messages
|
||||
assert len(lint_ctx.info_messages) == 1
|
||||
@@ -1535,7 +1538,7 @@ def test_outputs_unknown_tag(lint_ctx):
|
||||
probably be avoided.
|
||||
"""
|
||||
tool_source = get_xml_tool_source(OUTPUTS_UNKNOWN_TAG)
|
||||
lint_tool_source_with_modules(lint_ctx, tool_source, [outputs, xsd])
|
||||
lint_tool_source_with_modules(lint_ctx, tool_source, [output, xsd])
|
||||
assert "0 outputs found." in lint_ctx.info_messages
|
||||
assert "Avoid the use of 'output' and replace by 'data' or 'collection'" in lint_ctx.warn_messages
|
||||
assert len(lint_ctx.info_messages) == 1
|
||||
@@ -1546,7 +1549,7 @@ def test_outputs_unknown_tag(lint_ctx):
|
||||
|
||||
def test_outputs_unnamed_invalid_name(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_UNNAMED_INVALID_NAME)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, output, tool_source)
|
||||
assert "3 outputs found." in lint_ctx.info_messages
|
||||
assert "Invalid XML: Element 'data': The attribute 'name' is required but missing." in lint_ctx.error_messages
|
||||
assert "Invalid XML: Element 'collection': The attribute 'name' is required but missing." in lint_ctx.error_messages
|
||||
@@ -1562,7 +1565,7 @@ def test_outputs_unnamed_invalid_name(lint_ctx):
|
||||
|
||||
def test_outputs_format_input_legacy(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT_LEGACY)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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
|
||||
@@ -1573,7 +1576,7 @@ def test_outputs_format_input_legacy(lint_ctx):
|
||||
|
||||
def test_outputs_format_input(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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.error_messages
|
||||
assert len(lint_ctx.info_messages) == 1
|
||||
@@ -1584,7 +1587,7 @@ def test_outputs_format_input(lint_ctx):
|
||||
|
||||
def test_outputs_collection_format_source(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_COLLECTION_FORMAT_SOURCE)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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
|
||||
@@ -1594,7 +1597,7 @@ def test_outputs_collection_format_source(lint_ctx):
|
||||
|
||||
def test_outputs_format_action(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_FORMAT_ACTION)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, output, tool_source)
|
||||
assert len(lint_ctx.info_messages) == 1
|
||||
assert not lint_ctx.valid_messages
|
||||
assert not lint_ctx.warn_messages
|
||||
@@ -1603,7 +1606,7 @@ def test_outputs_format_action(lint_ctx):
|
||||
|
||||
def test_outputs_discover_tool_provided_metadata(lint_ctx):
|
||||
tool_source = get_xml_tool_source(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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
|
||||
@@ -1614,7 +1617,7 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx):
|
||||
def test_outputs_duplicated_name_label(lint_ctx):
|
||||
""" """
|
||||
tool_source = get_xml_tool_source(OUTPUTS_DUPLICATED_NAME_LABEL)
|
||||
run_lint_module(lint_ctx, outputs, tool_source)
|
||||
run_lint_module(lint_ctx, 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
|
||||
@@ -2075,6 +2078,30 @@ def test_xml_comments_are_ignored(lint_ctx: LintContext):
|
||||
assert "Comment" not in lint_message.message
|
||||
|
||||
|
||||
def test_skip_by_name(lint_ctx):
|
||||
# add a linter class name to the skip list
|
||||
lint_ctx.skip_types = ["CitationsMissing"]
|
||||
|
||||
tool_source = get_xml_tool_source(CITATIONS_ABSENT)
|
||||
run_lint_module(lint_ctx, citations, tool_source)
|
||||
assert not lint_ctx.warn_messages
|
||||
assert not lint_ctx.info_messages
|
||||
assert not lint_ctx.valid_messages
|
||||
assert not lint_ctx.error_messages
|
||||
|
||||
|
||||
def test_skip_by_module(lint_ctx):
|
||||
# add a module name to the skip list -> all linters in this module are skipped
|
||||
lint_ctx.skip_types = ["citations"]
|
||||
|
||||
tool_source = get_xml_tool_source(CITATIONS_ABSENT)
|
||||
run_lint_module(lint_ctx, citations, tool_source)
|
||||
assert not lint_ctx.warn_messages
|
||||
assert not lint_ctx.info_messages
|
||||
assert not lint_ctx.valid_messages
|
||||
assert not lint_ctx.error_messages
|
||||
|
||||
|
||||
def test_list_linters():
|
||||
linter_names = Linter.list_listers()
|
||||
# make sure to add/remove a test for new/removed linters if this number changes
|
||||
@@ -2095,3 +2122,45 @@ def test_list_linters():
|
||||
"XSD",
|
||||
]:
|
||||
assert len([x for x in linter_names if x.startswith(prefix)])
|
||||
|
||||
|
||||
def test_linter_module_list():
|
||||
linter_modules = submodules.import_submodules(galaxy.tool_util.linters)
|
||||
linter_module_names = [m.__name__.split(".")[-1] for m in linter_modules]
|
||||
linter_module_names = [n for n in linter_module_names if not n.startswith("_")]
|
||||
assert len(linter_module_names) >= 11
|
||||
|
||||
# until 23.2 the linters were implemented as functions lint_xyz contained in a module named xyz
|
||||
# with 24.0 the functions were split in multiple classes (1 per linter message)
|
||||
# in order to keep the skip functionality of lint contexts working (which is used eg in planemo)
|
||||
# with the old names, we now also check for module name if a linter should be skipped
|
||||
# therefore we test here that the module names are not changed
|
||||
# the keys of the following dict represent the linter names before the switch and the values give
|
||||
# the number of linter classes when we switched
|
||||
# so adding new functionality to a linter module is fine. But splitting one or removing functionality
|
||||
# should raise an error here to point to the possible consequences of renaming
|
||||
old_linters = {
|
||||
"citations": 4,
|
||||
"command": 5,
|
||||
"cwl": 9,
|
||||
"general": 17,
|
||||
"help": 6,
|
||||
"inputs": 52,
|
||||
"output": 11,
|
||||
"stdio": 3,
|
||||
"tests": 21,
|
||||
"xml_order": 1,
|
||||
}
|
||||
assert len(set(linter_module_names).intersection(set(old_linters.keys()))) == len(old_linters.keys())
|
||||
|
||||
for module in linter_modules:
|
||||
module_name = module.__name__.split(".")[-1]
|
||||
if module_name not in old_linters:
|
||||
continue
|
||||
linter_cnt = 0
|
||||
for name, value in inspect.getmembers(module):
|
||||
if callable(value) and name.startswith("lint_"):
|
||||
continue
|
||||
elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
|
||||
linter_cnt += 1
|
||||
assert linter_cnt >= old_linters[module_name]
|
||||
|
||||
Reference in New Issue
Block a user