Skip to content

Commit

Permalink
Merge pull request #17874 from bernt-matthias/assert-empty-output_24.0
Browse files Browse the repository at this point in the history 
[24.0] Missing outputs should be recorded as test errors
  • Loading branch information
@mvdbeek
mvdbeek authored Apr 2, 2024
2 parents e10ed2e + 8612c35 commit e331ccb
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 70 deletions.
44 changes: 24 additions & 20 deletions lib/galaxy/tool_util/verify/interactor.py
View file
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
83 changes: 33 additions & 50 deletions test/functional/tools/output_filter.xml
View file
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 All @@ -26,7 +26,6 @@ echo 'test' > p2.reverse
<!-- Must pass all filters... -->
<filter>filter_text_1 == "foo"</filter>
</data>
<data name="out_3" format="txt" from_work_dir="3"/>
<collection name="list_output" type="list" label="List">
<discover_datasets pattern="(?P&lt;identifier_0&gt;[45])" ext="txt" visible="true" />
<filter>produce_collection is True</filter>
Expand All @@ -37,134 +36,118 @@ echo 'test' > p2.reverse
</collection>
</outputs>
<tests>
<test expect_num_outputs="3">
<test expect_num_outputs="2">
<param name="produce_out_1" value="true" />
<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" />
</assert_contents>
</output>
<output name="out_3">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
</test>
<test expect_num_outputs="2">
<test expect_num_outputs="1">
<param name="produce_out_1" value="true" />
<param name="filter_text_1" value="bar" /> <!-- fails second filter in out2 -->
<output name="out_1">
<assert_contents>
<has_line line="test" />
</assert_contents>
</output>
<output name="out_3">
<assert_contents>
<has_line line="test" />
<has_line line="1" />
</assert_contents>
</output>
</test>
<test expect_num_outputs="1">
<!-- tool runs with no outputs should fail -->
<test expect_num_outputs="0" expect_test_failure="true">
<param name="produce_out_1" value="false" />
<param name="filter_text_1" value="not_foo_or_bar" />
<output name="out_3">
<assert_contents>
<has_line line="test" />
</assert_contents>
</output>
<assert_stdout>
<has_n_lines n="0"/>
</assert_stdout>
</test>
<test expect_num_outputs="4">
<test expect_num_outputs="3">
<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" />
</assert_contents>
</output>
<output name="out_3">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
<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>
</test>
<test expect_num_outputs="5">
<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" />
<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" />
</assert_contents>
</output>
<output name="out_3">
<assert_contents>
<has_line line="test" />
<has_line line="2" />
</assert_contents>
</output>
<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

0 comments on commit e331ccb

Please sign in to comment.