From 082d4656c8c72a192cf2cfdd78ee7161f90052ef Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 11 Mar 2024 12:57:19 +0100 Subject: [PATCH 1/7] tool linter: check for lead nodes with unstripped text content --- lib/galaxy/tool_util/linters/general.py | 15 +++++++ test/unit/tool_util/test_tool_linters.py | 53 +++++++++++++++--------- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index f395e3350904..a19cb6e175e1 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -227,3 +227,18 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): lint_ctx.warn( "Expressions in resource requirement not supported yet", linter=cls.name(), node=tool_node ) + + +class TextSpaces(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + if not tool_node: + return + for node in tool_node.iter(): + if len(node) > 0: + continue + if node.text and node.text != node.text.strip(): + lint_ctx.warn( + f"XML node '{node.tag}' has text with leading or trailing spaces ('{node.text}'!='{node.text.strip()}').", linter=cls.name(), node=node + ) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index e553d2c16884..dd1fa868e1b9 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -118,6 +118,16 @@ """ +GENERAL_TEXT_SPACES = """ + + + + bwa + + + +""" + # test tool xml for help linter HELP_MULTIPLE = """ @@ -893,10 +903,12 @@ TOOL_WITH_COMMENTS = """ - + + - + + """ @@ -924,15 +936,6 @@ def get_xml_tool_source(xml_string: str) -> XmlToolSource: return XmlToolSource(get_xml_tree(xml_string)) -def get_tool_xml_exact(xml_string: str): - """Returns the tool XML as it is, without stripping comments or anything else.""" - with tempfile.NamedTemporaryFile(mode="w", suffix="tool.xml") as tmp: - tmp.write(xml_string) - tmp.flush() - tool_path = tmp.name - return parse_xml(tool_path, strip_whitespace=False, remove_comments=False) - - def run_lint_module(lint_ctx, lint_module, lint_target): lint_tool_source_with_modules(lint_ctx, lint_target, list(set([lint_module, xsd]))) @@ -1081,6 +1084,25 @@ def test_general_valid_new_profile_fmt(lint_ctx): assert not lint_ctx.error_messages +def test_general_text_spaces(lint_ctx): + tool_source = get_xml_tool_source(GENERAL_TEXT_SPACES) + run_lint_module(lint_ctx, general, tool_source) + assert "XML node 'xref' has text with leading or trailing spaces ('\n bwa\n '!='bwa')" in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 1 + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_general_text_spaces_comments(lint_ctx): + tool_source = get_xml_tool_source(TOOL_WITH_COMMENTS) + run_lint_module(lint_ctx, general, tool_source) + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 4 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + def test_help_multiple(lint_ctx): tool_source = get_xml_tool_source(HELP_MULTIPLE) run_lint_module(lint_ctx, help, tool_source) @@ -2068,17 +2090,10 @@ def test_linting_cwl_tool(lint_ctx): assert not lint_ctx.error_messages -def test_xml_comments_are_ignored(lint_ctx: LintContext): - tool_xml = get_tool_xml_exact(TOOL_WITH_COMMENTS) - lint_xml_with(lint_ctx, tool_xml) - for lint_message in lint_ctx.message_list: - assert "Comment" not in lint_message.message - - def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 130 + assert len(linter_names) == 131 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From 4003cb4a7450d0ad7ad7f6b2aae029e22f904e0e Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 11 Mar 2024 13:15:56 +0100 Subject: [PATCH 2/7] remove unused imports --- test/unit/tool_util/test_tool_linters.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index dd1fa868e1b9..0614539c616f 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -6,7 +6,6 @@ from galaxy.tool_util.lint import ( lint_tool_source_with, lint_tool_source_with_modules, - lint_xml_with, LintContext, Linter, XMLLintMessageLine, @@ -26,10 +25,7 @@ ) from galaxy.tool_util.loader_directory import load_tool_sources_from_path from galaxy.tool_util.parser.xml import XmlToolSource -from galaxy.util import ( - ElementTree, - parse_xml, -) +from galaxy.util import ElementTree from galaxy.util.xml_macros import load_with_references # TODO tests tool xml for general linter From c51eda696a20c7c91c8a63f8909ca3ba55afab7e Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 11 Mar 2024 15:21:44 +0100 Subject: [PATCH 3/7] apply black --- lib/galaxy/tool_util/linters/general.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index a19cb6e175e1..88d1dcf4735c 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -240,5 +240,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): continue if node.text and node.text != node.text.strip(): lint_ctx.warn( - f"XML node '{node.tag}' has text with leading or trailing spaces ('{node.text}'!='{node.text.strip()}').", linter=cls.name(), node=node + f"XML node '{node.tag}' has text with leading or trailing spaces ('{node.text}'!='{node.text.strip()}').", + linter=cls.name(), + node=node, ) From 5a03b37f0cb3a9bdb1a73d64751950f357ee3d42 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 11 Mar 2024 18:11:02 +0100 Subject: [PATCH 4/7] black --- test/unit/tool_util/test_tool_linters.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 0614539c616f..5a16e06e2391 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1083,7 +1083,10 @@ def test_general_valid_new_profile_fmt(lint_ctx): def test_general_text_spaces(lint_ctx): tool_source = get_xml_tool_source(GENERAL_TEXT_SPACES) run_lint_module(lint_ctx, general, tool_source) - assert "XML node 'xref' has text with leading or trailing spaces ('\n bwa\n '!='bwa')" in lint_ctx.warn_messages + assert ( + "XML node 'xref' has text with leading or trailing spaces ('\n bwa\n '!='bwa')" + in lint_ctx.warn_messages + ) assert not lint_ctx.info_messages assert len(lint_ctx.valid_messages) == 1 assert len(lint_ctx.warn_messages) == 1 From e0e6a6e8257e3d54dac111953fcc76250f8feaf7 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 11 Mar 2024 18:45:55 +0100 Subject: [PATCH 5/7] fix tests --- test/unit/tool_util/test_tool_linters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 5a16e06e2391..c4762cbe97c9 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -897,7 +897,7 @@ """ TOOL_WITH_COMMENTS = """ - + @@ -1088,7 +1088,7 @@ def test_general_text_spaces(lint_ctx): in lint_ctx.warn_messages ) assert not lint_ctx.info_messages - assert len(lint_ctx.valid_messages) == 1 + assert len(lint_ctx.valid_messages) == 4 assert len(lint_ctx.warn_messages) == 1 assert not lint_ctx.error_messages From 55c5d0847c58c73c8d236e9fc593dff0939624f0 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 7 May 2024 17:53:26 +0200 Subject: [PATCH 6/7] remove unused import --- test/unit/tool_util/test_tool_linters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 2e06d12a04b4..27f5bb9f9323 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -29,7 +29,6 @@ from galaxy.tool_util.parser.xml import XmlToolSource from galaxy.util import ( ElementTree, - parse_xml, submodules, ) from galaxy.util.unittest_utils import skip_if_site_down From 74b19f02e57f91e9d49be86cde57b078e8baba00 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 7 May 2024 18:12:40 +0200 Subject: [PATCH 7/7] fix number of tests test bad merge --- test/unit/tool_util/test_tool_linters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 27f5bb9f9323..2db30308b950 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -2168,7 +2168,7 @@ def test_skip_by_module(lint_ctx): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 132 + assert len(linter_names) == 133 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [