Don't wipe collected outputs when a test names an undefined output

[jmchilton]

Updated: Why is this PR message soooo long 😿 - it just fixes #1625 - that is the PR description 😆 . -John

Fixes #1625 — planemo test crashed with a raw bioblend.ConnectionError and wrote no
test report when a workflow test YAML named an output the workflow doesn't define.

Root cause

collect_outputs() has two modes. Full mode builds self._outputs_dict; single-output
mode (output_id=..., used by get_output) returns just that one value. But the final
self._outputs_dict = outputs_dict at planemo/galaxy/activity.py:710 runs in both
modes. In single-output mode the loop continues past every non-matching output, so when
output_id names an output the runnable doesn't define, nothing is ever added and that
assignment replaces the whole cache with {}.

That cache is load-bearing. Outputs are collected eagerly while Galaxy is still running —
the output_collectors added in 5ff19c8 ("Ignore missing invocation outputs when
fetching outputs") call structured_test_data from inside GalaxyEngine._run's
ensure_runnables_served block. BaseEngine.test() then rebuilds the same structured data
after _run has returned and Galaxy has shut down, expecting every lookup to hit the cache.

So a single typo'd output label does this:

1. pass 1, Galaxy up: real outputs collected and cached
2. pass 1: the typo'd label is looked up, matches nothing, cache wiped to {}
3. pass 2, Galaxy down: the real outputs are missing from the cache, get re-fetched from
   the API, and bioblend raises — out of structured_data, uncaught, no report

The "Expected output [x] not found in results." message the reporter wanted already exists
at planemo/runnable.py:606; the crash just happened first.

Introduced by de8b2a9 ("Download outputs only if needed in test assertions"), which added
single-output mode without guarding the trailing assignment.

Fix

Return None from single-output mode instead of falling through to the assignment.
get_output already caches that None under the requested id itself.

The same path also covers an output the workflow does define but didn't create (optional
outputs, output_src falsy) — that previously wiped the cache too.

Test

tests/test_cmd_test.py::CmdTestTestCase::test_workflow_test_undefined_output — a real
integration test, runs Galaxy end to end. New fixtures tests/data/wf20_undefined_output.yml
(one cat step, one output) and its -test.yml, which expects wf_output_1 and then
wf_output_typo.

Ordering matters: the undefined label has to come after a real one, so the real output is
looked up again in pass 2 against the wiped cache. With the undefined label first, the cache
is empty anyway and nothing goes wrong.

The test asserts on the JSON report rather than the exit code alone, because an uncaught
exception also exits 1 — a crash is distinguished by writing no report at all.

Red-to-green (managed Galaxy, local):

	result
before the fix	FileNotFoundError: .../tool_test_output.json — planemo died with bioblend.ConnectionError at activity.py:735 → collect_outputs → _get_metadata → show_dataset
after the fix	passes: status failure, output_problems == ["Expected output [wf_output_typo] not found in results."]

Reproduced the reporter's traceback exactly before fixing — same call chain, show_dataset
where theirs hit show_dataset_collection. Log showed 2 collections before shutdown, 1
after, which is the wipe.

Neighbouring workflow tests re-run green: test_workflow_test_simple_yaml,
test_workflow_test_output_sanitization, test_workflow_with_identical_output_names,
test_workflow_with_optional_input_output_not_provided (this one exercises the
not-created-output path through the same code). 5 passed, 1 skipped (dockerized).

black / isort / flake8 clean; mypy unchanged (pre-existing errors only).

Worth a second opinion

structured_data is computed twice per test case — once eagerly while Galaxy is up, once
again after it has shut down — and correctness depends entirely on the cache surviving in
between. This restores that invariant rather than removing the fragility. Collecting once,
or failing loudly when a post-shutdown lookup misses the cache, would be sturdier.

🤖 Generated with Claude Code