From 6687cdc3cadb9796d45487a5c9feaa8f47b13706 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 15 May 2023 20:57:24 +0200 Subject: [PATCH 01/12] adapt output filter test - remove the output that is always present - add a test that has no outputs --- test/functional/tools/output_filter.xml | 40 ++++++------------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/test/functional/tools/output_filter.xml b/test/functional/tools/output_filter.xml index 2002fee03826..a76688359489 100644 --- a/test/functional/tools/output_filter.xml +++ b/test/functional/tools/output_filter.xml @@ -26,7 +26,6 @@ echo 'test' > p2.reverse filter_text_1 == "foo" - produce_collection is True @@ -37,7 +36,7 @@ echo 'test' > p2.reverse - + @@ -50,13 +49,8 @@ echo 'test' > p2.reverse - - - - - - + @@ -64,22 +58,16 @@ echo 'test' > p2.reverse - - - - - - + + - - - - - + + + - + @@ -93,11 +81,6 @@ echo 'test' > p2.reverse - - - - - @@ -111,7 +94,7 @@ echo 'test' > p2.reverse - + @@ -126,11 +109,6 @@ echo 'test' > p2.reverse - - - - - From f19f4ae0c3cbcf750e39b41d22f570985d50983c Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 15 May 2023 18:53:36 +0200 Subject: [PATCH 02/12] tool verification: move no output assertion from planemo the verify_tool function is called from [within an try-except block](https://github.com/galaxyproject/planemo/blob/1aa3eb05a97ad20c0be6f6560ab5cec090e76612/planemo/engine/galaxy.py#L109) which silently catches any exception. Thus any exception raised from within verify_tool will not be detected, i.e. the assertion needs to be moved into `_verify_outputs` (which also seems to make sense by name) in order to make verify_tool record the problem properly. --- lib/galaxy/tool_util/verify/interactor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index da4bf13baf3b..f4f09bd0403d 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1430,8 +1430,6 @@ def verify_tool( raise e if not expected_failure_occurred: - assert data_list or data_collection_list - try: job_stdio = _verify_outputs( testdef, test_history, jobs, data_list, data_collection_list, galaxy_interactor, quiet=quiet @@ -1487,6 +1485,7 @@ def _handle_def_errors(testdef): def _verify_outputs(testdef, history, jobs, data_list, data_collection_list, galaxy_interactor, quiet=False): + assert data_list or data_collection_list, "Tool produced no output data" assert len(jobs) == 1, "Test framework logic error, somehow tool test resulted in more than one job." job = jobs[0] From 46abd27123cedd6092c3afc5e96b187c624358bb Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 20 Jul 2023 09:38:22 +0200 Subject: [PATCH 03/12] make test a bit more specific --- test/functional/tools/output_filter.xml | 48 ++++++++++++------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/test/functional/tools/output_filter.xml b/test/functional/tools/output_filter.xml index a76688359489..c065d6d4afd3 100644 --- a/test/functional/tools/output_filter.xml +++ b/test/functional/tools/output_filter.xml @@ -1,15 +1,15 @@ test for output filtering and expect_num_outputs 1 && -echo 'test' > 2 && -echo 'test' > 3 && -echo 'test' > 4 && -echo 'test' > 5 && -echo 'test' > p1.forward && -echo 'test' > p1.reverse && -echo 'test' > p2.forward && -echo 'test' > p2.reverse +echo '1' > 1 && +echo '2' > 2 && +echo '3' > 3 && +echo '4' > 4 && +echo '5' > 5 && +echo 'p1.forward' > p1.forward && +echo 'p1.reverse' > p1.reverse && +echo 'p2.forward' > p2.forward && +echo 'p2.reverse' > p2.reverse ]]> @@ -41,12 +41,12 @@ echo 'test' > p2.reverse - + - + @@ -55,7 +55,7 @@ echo 'test' > p2.reverse - + @@ -73,23 +73,23 @@ echo 'test' > p2.reverse - + - + - + - + @@ -101,23 +101,23 @@ echo 'test' > p2.reverse - + - + - + - + @@ -125,24 +125,24 @@ echo 'test' > p2.reverse - + - + - + - + From 6da871971d7aaca8d468409547df9093ec090afc Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 20 Jul 2023 09:53:22 +0200 Subject: [PATCH 04/12] register failure to produce output thereby we implicitly make the assumption that a test needs to produce an output also make message more specific --- lib/galaxy/tool_util/verify/interactor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index f4f09bd0403d..b355f02cf0c3 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1485,7 +1485,6 @@ def _handle_def_errors(testdef): def _verify_outputs(testdef, history, jobs, data_list, data_collection_list, galaxy_interactor, quiet=False): - assert data_list or data_collection_list, "Tool produced no output data" assert len(jobs) == 1, "Test framework logic error, somehow tool test resulted in more than one job." job = jobs[0] @@ -1499,6 +1498,10 @@ def register_exception(e: Exception): print(_format_stream(job_stdio[stream], stream=stream, format=True), file=sys.stderr) found_exceptions.append(e) + if not (data_list or data_collection_list): + error = AssertionError("Tool produced no output datasets or collections") + register_exception(error) + if testdef.expect_failure: if testdef.outputs: raise Exception("Cannot specify outputs in a test expecting failure.") From fb0159075d7778be100187f6fb4f86572f1bfb9d Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 20 Jul 2023 14:02:38 +0200 Subject: [PATCH 05/12] register later --- lib/galaxy/tool_util/verify/interactor.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index b355f02cf0c3..6aad872067fe 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1498,10 +1498,6 @@ def register_exception(e: Exception): print(_format_stream(job_stdio[stream], stream=stream, format=True), file=sys.stderr) found_exceptions.append(e) - if not (data_list or data_collection_list): - error = AssertionError("Tool produced no output datasets or collections") - register_exception(error) - if testdef.expect_failure: if testdef.outputs: raise Exception("Cannot specify outputs in a test expecting failure.") @@ -1519,6 +1515,10 @@ def register_exception(e: Exception): job_stdio = galaxy_interactor.get_job_stdio(job["id"]) + if not (data_list or data_collection_list): + error = AssertionError("Tool produced no output datasets or collections") + register_exception(error) + if testdef.num_outputs is not None: expected = testdef.num_outputs actual = len(data_list) + len(data_collection_list) From b05bfc3265d975bac5209ca32f44845adf344453 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 25 Mar 2024 13:35:05 +0100 Subject: [PATCH 06/12] move assertion instead of removing it otherwise the tool test will just return a list out of bounds exception which is unclear to the user --- lib/galaxy/tool_util/verify/interactor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 6aad872067fe..74e846c34a77 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1487,7 +1487,6 @@ def _handle_def_errors(testdef): def _verify_outputs(testdef, history, jobs, data_list, data_collection_list, galaxy_interactor, quiet=False): assert len(jobs) == 1, "Test framework logic error, somehow tool test resulted in more than one job." job = jobs[0] - found_exceptions: List[Exception] = [] def register_exception(e: Exception): @@ -1502,6 +1501,9 @@ def register_exception(e: Exception): if testdef.outputs: raise Exception("Cannot specify outputs in a test expecting failure.") + if not data_list and not data_collection_list: + Exception("Job did not produce any outputs") + maxseconds = testdef.maxseconds # Wait for the job to complete and register expections if the final # status was not what test was expecting. From 0b082d2cd3e84c4dbc5678bb545d87849bdaf405 Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 25 Mar 2024 13:40:35 +0100 Subject: [PATCH 07/12] Just use the suggestion Co-authored-by: Marius van den Beek --- lib/galaxy/tool_util/verify/interactor.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 74e846c34a77..5f7bbd5264e9 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1501,9 +1501,6 @@ def register_exception(e: Exception): if testdef.outputs: raise Exception("Cannot specify outputs in a test expecting failure.") - if not data_list and not data_collection_list: - Exception("Job did not produce any outputs") - maxseconds = testdef.maxseconds # Wait for the job to complete and register expections if the final # status was not what test was expecting. @@ -1517,9 +1514,7 @@ def register_exception(e: Exception): job_stdio = galaxy_interactor.get_job_stdio(job["id"]) - if not (data_list or data_collection_list): - error = AssertionError("Tool produced no output datasets or collections") - register_exception(error) + assert data_list or data_collection_list, "Tool produced no output datasets or collections" if testdef.num_outputs is not None: expected = testdef.num_outputs From e0fee40053fc6365981942095402aa5d8e1a1aab Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 25 Mar 2024 15:31:20 +0100 Subject: [PATCH 08/12] check only presence of outputs mentioned in the tests --- lib/galaxy/tool_util/verify/interactor.py | 42 +++++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 5f7bbd5264e9..28b590f53bd1 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1514,8 +1514,6 @@ def register_exception(e: Exception): job_stdio = galaxy_interactor.get_job_stdio(job["id"]) - assert data_list or data_collection_list, "Tool produced no output datasets or collections" - if testdef.num_outputs is not None: expected = testdef.num_outputs actual = len(data_list) + len(data_collection_list) @@ -1547,23 +1545,29 @@ def register_exception(e: Exception): # Legacy - fall back on ordered data list access if data_list is # just a list (case with twill variant or if output changes its # name). - if hasattr(data_list, "values"): - output_data = list(data_list.values())[output_index] - else: - output_data = data_list[len(data_list) - len(testdef.outputs) + output_index] - assert output_data is not None - try: - galaxy_interactor.verify_output( - history, - jobs, - output_data, - output_testdef=output_testdef, - tool_id=job["tool_id"], - maxseconds=maxseconds, - tool_version=testdef.tool_version, - ) - except Exception as e: - register_exception(e) + try: + if hasattr(data_list, "values"): + output_data = list(data_list.values())[output_index] + else: + output_data = data_list[len(data_list) - len(testdef.outputs) + output_index] + except IndexError: + pass + if output_data: + try: + galaxy_interactor.verify_output( + history, + jobs, + output_data, + output_testdef=output_testdef, + tool_id=job["tool_id"], + maxseconds=maxseconds, + tool_version=testdef.tool_version, + ) + except Exception as e: + register_exception(e) + else: + error = AssertionError(f"Tool did not produce an output with name '{name}' (or at index {output_index})") + register_exception(error) other_checks = { "command_line": "Command produced by the job", From ae594c18bcdc3406053f108d60625988ee30a8af Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 25 Mar 2024 15:34:57 +0100 Subject: [PATCH 09/12] Add back test output --- test/functional/tools/output_filter.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/functional/tools/output_filter.xml b/test/functional/tools/output_filter.xml index c065d6d4afd3..d81f17cef201 100644 --- a/test/functional/tools/output_filter.xml +++ b/test/functional/tools/output_filter.xml @@ -63,9 +63,16 @@ echo 'p2.reverse' > p2.reverse + + + + + + + From e1ec2c8b28dbd96979b2c1a230797d391fb57edb Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 25 Mar 2024 15:44:11 +0100 Subject: [PATCH 10/12] Update test/functional/tools/output_filter.xml --- test/functional/tools/output_filter.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/functional/tools/output_filter.xml b/test/functional/tools/output_filter.xml index d81f17cef201..4a891399210d 100644 --- a/test/functional/tools/output_filter.xml +++ b/test/functional/tools/output_filter.xml @@ -71,8 +71,6 @@ echo 'p2.reverse' > p2.reverse - - From 1a545ae445a55b9a9302cdc250f3acb34a7a2884 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 26 Mar 2024 08:44:54 +0100 Subject: [PATCH 11/12] move registering the exception then it might be clearer what's going on here --- lib/galaxy/tool_util/verify/interactor.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 28b590f53bd1..9043cf5c885d 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1539,6 +1539,7 @@ def register_exception(e: Exception): outfile = output_dict["value"] attributes = output_dict["attributes"] output_testdef = Bunch(name=name, outfile=outfile, attributes=attributes) + output_data = None try: output_data = data_list[name] except (TypeError, KeyError): @@ -1551,7 +1552,8 @@ def register_exception(e: Exception): else: output_data = data_list[len(data_list) - len(testdef.outputs) + output_index] except IndexError: - pass + error = AssertionError(f"Tool did not produce an output with name '{name}' (or at index {output_index})") + register_exception(error) if output_data: try: galaxy_interactor.verify_output( @@ -1565,9 +1567,6 @@ def register_exception(e: Exception): ) except Exception as e: register_exception(e) - else: - error = AssertionError(f"Tool did not produce an output with name '{name}' (or at index {output_index})") - register_exception(error) other_checks = { "command_line": "Command produced by the job", From 8612c35d0c2631fa6966fe57249ed4c41e3e53bc Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 26 Mar 2024 08:50:38 +0100 Subject: [PATCH 12/12] black --- lib/galaxy/tool_util/verify/interactor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 9043cf5c885d..d08804ae58b3 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -1552,7 +1552,9 @@ def register_exception(e: Exception): else: output_data = data_list[len(data_list) - len(testdef.outputs) + output_index] except IndexError: - error = AssertionError(f"Tool did not produce an output with name '{name}' (or at index {output_index})") + error = AssertionError( + f"Tool did not produce an output with name '{name}' (or at index {output_index})" + ) register_exception(error) if output_data: try: