bacterial-genome-assembly: convert asserts to list form and fix resurrected assertion#1276
Draft
jmchilton wants to merge 2 commits into
Draft
Conversation
YAML silently collapses duplicate mapping keys, so repeated has_text/has_line/has_text_matching entries only ran the last assertion per type. Convert to list form so each assertion runs. This test is currently RED — the previously-dropped assertion(s) do not match tool output. Split out from galaxyproject#1217 to track down individually. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Test Results (powered by Planemo)Test Summary
Failed Tests
|
The `"Assembly": "shovill_contigs_fasta"` assertion never actually ran. It was added (wf 1.1.9, 0925cf7) as a second `has_text:` key in the same `asserts:` mapping, directly above `has_text: contig00146`. YAML collapses duplicate mapping keys (last wins), so only `contig00146` was ever evaluated — the Assembly assertion was dead from the moment it was written and was never checked against real tool output. Converting the block to list form (this PR series) ran it for the first time, and it failed: tooldistillator's JSON has no `Assembly` key and no `shovill_contigs_fasta` value. The author appears to have guessed the output shape (expecting the input dataset label echoed under `Assembly`). Replace it with a substring that is actually present and preserves the intent — confirming the summary is of the shovill assembly: `"analysis_software_name": "shovill"`. `contig00146` is genuinely in the output and is kept. Caveat: `has_text` substring matching over a serialized JSON document is a poor way to validate structured output — it silently depends on key ordering and whitespace in the serializer and asserts nothing about structure. A structured JSON-property assertion would be more robust; a follow-up could migrate these to that. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jmchilton
changed the title
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Convert the
tooldistillator_summarize_assemblyasserts:block to list form and fix the assertion that this exposed.The
"Assembly": "shovill_contigs_fasta"assertion never actually ran: it was authored (wf 1.1.9,0925cf747) as a duplicatehas_text:mapping key stacked abovehas_text: contig00146. YAML collapses duplicate mapping keys (last wins), so onlycontig00146was ever evaluated — the Assembly assertion was dead from the day it was written and never validated. Converting to list form ran it for the first time and it failed: tooldistillator's JSON has noAssemblykey and noshovill_contigs_fastavalue.Fix: replace it with a substring that is actually present and preserves the intent (confirm the summary is of the shovill assembly):
"analysis_software_name": "shovill".contig00146is genuinely in the output and is kept.Caveat / possible follow-up
has_textsubstring matching over a serialized JSON document is a fragile way to validate structured output — it silently depends on key ordering and whitespace and asserts nothing structural. A follow-up could migrate these to a proper JSON-property assertion.Split from the passing-subset PR; belongs on its own because it changes an assertion's meaning, not just its form.