From edf4b43b5c585b1f5d8fd959c8751f6ca353adc7 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 24 Nov 2021 11:16:56 +0100 Subject: [PATCH 1/3] linter: warn if expect_num_outputs is missing if outputs have filters or are discovered --- lib/galaxy/tool_util/linters/tests.py | 6 ++++ test/unit/tool_util/test_tool_linters.py | 42 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lib/galaxy/tool_util/linters/tests.py b/lib/galaxy/tool_util/linters/tests.py index 1825457ce1c..6f0da33dca4 100644 --- a/lib/galaxy/tool_util/linters/tests.py +++ b/lib/galaxy/tool_util/linters/tests.py @@ -26,6 +26,12 @@ def lint_tsts(tool_xml, lint_ctx): has_test = True break + # check if expect_num_outputs is set if there are outputs with filters or discovered datasets + discover_datasets = tool_xml.findall("./outputs//discover_datasets") + filter = tool_xml.findall("./outputs//filter") + if len(discover_datasets) + len(filter) > 0 and "expect_num_outputs" not in test.attrib: + lint_ctx.warn("Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets") + # really simple test that test parameters are also present in the inputs for param in test.findall("param"): name = param.attrib.get("name", None) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index b5617388704..cab8adb54c4 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -280,6 +280,34 @@ TESTS_EXPECT_FAILURE_OUTPUT = """ """ +TESTS_EXPECT_NUM_OUTPUTS_FILTER = """ + + + + + + + + + + + +""" + +TESTS_EXPECT_NUM_OUTPUTS_DISCOVERED_DATASETS = """ + + + + + + + + + + + +""" + TESTS = [ ( WHITESPACE_IN_VERSIONS_AND_NAMES, general.lint_general, @@ -396,6 +424,18 @@ TESTS = [ lambda x: "Test 1: Cannot specify outputs in a test expecting failure." in x.error_messages and len(x.warn_messages) == 0 and len(x.error_messages) == 1 + ), + ( + TESTS_EXPECT_NUM_OUTPUTS_FILTER, tests.lint_tsts, + lambda x: + "Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets" in x.warn_messages + and len(x.warn_messages) == 1 and len(x.error_messages) == 0 + ), + ( + TESTS_EXPECT_NUM_OUTPUTS_DISCOVERED_DATASETS, tests.lint_tsts, + lambda x: + "Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets" in x.warn_messages + and len(x.warn_messages) == 1 and len(x.error_messages) == 0 ) ] @@ -416,6 +456,8 @@ TEST_IDS = [ 'test without expectations', 'test param missing from inputs', 'test expecting failure with outputs', + 'test missing expect_num_outputs for filtered outputs', + 'test missing expect_num_outputs for discovered outputs', ] From b1de5d29cc0f879efeee2a1285893f23f5265eb4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sun, 16 Jan 2022 17:04:40 +0100 Subject: [PATCH 2/3] linter warn for missing expect_num_outputs only if filters are present discovered datasets are not counted. see: https://github.com/galaxyproject/galaxy/pull/7894 https://github.com/galaxyproject/galaxy/pull/7894/commits/2aa6831c740126bcd3b6e10e9a0d282aca3b4ba7 --- lib/galaxy/tool_util/linters/tests.py | 7 +++---- test/unit/tool_util/test_tool_linters.py | 23 +---------------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/lib/galaxy/tool_util/linters/tests.py b/lib/galaxy/tool_util/linters/tests.py index 6f0da33dca4..2589c8e0760 100644 --- a/lib/galaxy/tool_util/linters/tests.py +++ b/lib/galaxy/tool_util/linters/tests.py @@ -26,11 +26,10 @@ def lint_tsts(tool_xml, lint_ctx): has_test = True break - # check if expect_num_outputs is set if there are outputs with filters or discovered datasets - discover_datasets = tool_xml.findall("./outputs//discover_datasets") + # check if expect_num_outputs is set if there are outputs with filters filter = tool_xml.findall("./outputs//filter") - if len(discover_datasets) + len(filter) > 0 and "expect_num_outputs" not in test.attrib: - lint_ctx.warn("Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets") + if len(filter) > 0 and "expect_num_outputs" not in test.attrib: + lint_ctx.warn("Test should specify 'expect_num_outputs' if outputs have filters") # really simple test that test parameters are also present in the inputs for param in test.findall("param"): diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index cab8adb54c4..93db3b0bde2 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -294,20 +294,6 @@ TESTS_EXPECT_NUM_OUTPUTS_FILTER = """ """ -TESTS_EXPECT_NUM_OUTPUTS_DISCOVERED_DATASETS = """ - - - - - - - - - - - -""" - TESTS = [ ( WHITESPACE_IN_VERSIONS_AND_NAMES, general.lint_general, @@ -428,15 +414,9 @@ TESTS = [ ( TESTS_EXPECT_NUM_OUTPUTS_FILTER, tests.lint_tsts, lambda x: - "Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets" in x.warn_messages + "Test should specify 'expect_num_outputs' if outputs have filters" in x.warn_messages and len(x.warn_messages) == 1 and len(x.error_messages) == 0 ), - ( - TESTS_EXPECT_NUM_OUTPUTS_DISCOVERED_DATASETS, tests.lint_tsts, - lambda x: - "Test should specify 'expect_num_outputs' if outputs have filters or use discover_datasets" in x.warn_messages - and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ) ] TEST_IDS = [ @@ -457,7 +437,6 @@ TEST_IDS = [ 'test param missing from inputs', 'test expecting failure with outputs', 'test missing expect_num_outputs for filtered outputs', - 'test missing expect_num_outputs for discovered outputs', ] From 28b937b1b7f30173af8ab7eefd8a6cbb56b3d121 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 14 Sep 2022 17:16:58 +0200 Subject: [PATCH 3/3] post merge cleanup --- lib/galaxy/tool_util/linters/tests.py | 4 ++-- test/unit/tool_util/test_tool_linters.py | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_util/linters/tests.py b/lib/galaxy/tool_util/linters/tests.py index db15aac2281..4f30343d78a 100644 --- a/lib/galaxy/tool_util/linters/tests.py +++ b/lib/galaxy/tool_util/linters/tests.py @@ -38,7 +38,7 @@ def lint_tsts(tool_xml, lint_ctx): if len(assertions) == 0: continue if len(assertions) > 1: - lint_ctx.error(f"Test {test_idx}: More than one {ta} found. Only the first is considered.") + lint_ctx.error(f"Test {test_idx}: More than one {ta} found. Only the first is considered.", node=test) has_test = True _check_asserts(test_idx, assertions, lint_ctx) _check_asserts(test_idx, test.findall(".//assert_contents"), lint_ctx) @@ -46,7 +46,7 @@ def lint_tsts(tool_xml, lint_ctx): # check if expect_num_outputs is set if there are outputs with filters filter = tool_xml.findall("./outputs//filter") if len(filter) > 0 and "expect_num_outputs" not in test.attrib: - lint_ctx.warn("Test should specify 'expect_num_outputs' if outputs have filters") + lint_ctx.warn("Test should specify 'expect_num_outputs' if outputs have filters", node=test) # really simple test that test parameters are also present in the inputs for param in test.findall("param"): diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index f43aafdbc0b..8e1ff98d50d 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -758,6 +758,7 @@ TESTS_EXPECT_NUM_OUTPUTS_FILTER = """ +""" # tool xml for xml_order linter XML_ORDER = """ @@ -1575,10 +1576,7 @@ def test_tests_discover_outputs(lint_ctx): def test_tests_expect_num_outputs_filter(lint_ctx): tool_source = get_xml_tool_source(TESTS_EXPECT_NUM_OUTPUTS_FILTER) run_lint(lint_ctx, tests.lint_tsts, tool_source) - assert ( - "Test should specify 'expect_num_outputs' if outputs have filters" in - in lint_ctx.warn_messages - ) + assert "Test should specify 'expect_num_outputs' if outputs have filters" in lint_ctx.warn_messages assert len(lint_ctx.warn_messages) == 1 assert len(lint_ctx.error_messages) == 0