Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.0] Backport #16094 Missing outputs should be recorded as test errors #17873

Closed
wants to merge 11 commits into from
Closed

[24.0] Backport #16094 Missing outputs should be recorded as test errors #17873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f87c0a2
tool verification: move no output assertion
bernt-matthias May 15, 2023
eea13e9
make test a bit more specific
bernt-matthias Jul 20, 2023
dc4ce43
register failure to produce output
bernt-matthias Jul 20, 2023
be8be79
register later
bernt-matthias Jul 20, 2023
5a1efea
move assertion instead of removing it
bernt-matthias Mar 25, 2024
6167ce2
Just use the suggestion
bernt-matthias Mar 25, 2024
58a4e5a
check only presence of outputs mentioned in the tests
bernt-matthias Mar 25, 2024
73b5804
Add back test output
bernt-matthias Mar 25, 2024
b3a95b1
Update test/functional/tools/output_filter.xml
bernt-matthias Mar 25, 2024
6d5ccd1
move registering the exception
bernt-matthias Mar 26, 2024
31ec91b
black
bernt-matthias Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions lib/galaxy/tool_util/verify/interactor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1489,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):
Expand Down Expand Up @@ -1542,29 +1539,36 @@ 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):
# 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:
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(
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)

other_checks = {
"command_line": "Command produced by the job",
Expand Down
51 changes: 27 additions & 24 deletions test/functional/tools/output_filter.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<tool id="output_filter" name="output_filter" version="1.0.0">
<description>test for output filtering and expect_num_outputs</description>
<command><![CDATA[
echo 'test' > 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
]]></command>
<inputs>
<param name="produce_out_1" type="boolean" truevalue="true" falsevalue="false" checked="False" label="Do Filter 1" />
Expand Down Expand Up @@ -42,12 +42,12 @@ echo 'test' > p2.reverse
<param name="filter_text_1" value="foo" />
<output name="out_1">
<assert_contents>
<has_line line="test" />
<has_line line="1" />
</assert_contents>
</output>
<output name="out_2">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
<output name="out_3">
Expand All @@ -61,7 +61,7 @@ echo 'test' > p2.reverse
<param name="filter_text_1" value="bar" /> <!-- fails second filter in out2 -->
<output name="out_1">
<assert_contents>
<has_line line="test" />
<has_line line="1" />
</assert_contents>
</output>
<output name="out_3">
Expand All @@ -78,19 +78,22 @@ echo 'test' > p2.reverse
<has_line line="test" />
</assert_contents>
</output>
<assert_stdout>
<has_n_lines n="0"/>
</assert_stdout>
</test>
<test expect_num_outputs="4">
<param name="produce_out_1" value="true" />
<param name="filter_text_1" value="foo" />
<param name="produce_collection" value="true" />
<output name="out_1">
<assert_contents>
<has_line line="test" />
<has_line line="1" />
</assert_contents>
</output>
<output name="out_2">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
<output name="out_3">
Expand All @@ -101,12 +104,12 @@ echo 'test' > p2.reverse
<output_collection name="list_output" type="list" count="2">
<element name="4">
<assert_contents>
<has_line line="test" />
<has_line line="4" />
</assert_contents>
</element>
<element name="5">
<assert_contents>
<has_line line="test" />
<has_line line="5" />
</assert_contents>
</element>
</output_collection>
Expand All @@ -118,12 +121,12 @@ echo 'test' > p2.reverse
<param name="produce_paired_collection" value="true" />
<output name="out_1">
<assert_contents>
<has_line line="test" />
<has_line line="1" />
</assert_contents>
</output>
<output name="out_2">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
<output name="out_3">
Expand All @@ -134,37 +137,37 @@ echo 'test' > p2.reverse
<output_collection name="list_output" type="list" count="2">
<element name="4">
<assert_contents>
<has_line line="test" />
<has_line line="4" />
</assert_contents>
</element>
<element name="5">
<assert_contents>
<has_line line="test" />
<has_line line="5" />
</assert_contents>
</element>
</output_collection>
<output_collection name="paired_list_output" type="list:paired" count="2">
<element name="p1">
<element name="forward">
<assert_contents>
<has_line line="test" />
<has_line line="p1.forward" />
</assert_contents>
</element>
<element name="reverse">
<assert_contents>
<has_line line="test" />
<has_line line="p1.reverse" />
</assert_contents>
</element>
</element>
<element name="p2">
<element name="forward">
<assert_contents>
<has_line line="test" />
<has_line line="p2.forward" />
</assert_contents>
</element>
<element name="reverse">
<assert_contents>
<has_line line="test" />
<has_line line="p2.reverse" />
</assert_contents>
</element>
</element>
Expand Down
Loading