Skip to content

Schema regularization/validation fixes for tests around workflow outputs#1286

Closed
jmchilton wants to merge 3 commits into
galaxyproject:mainfrom
jmchilton:collection_descriminator
Closed

Schema regularization/validation fixes for tests around workflow outputs#1286
jmchilton wants to merge 3 commits into
galaxyproject:mainfrom
jmchilton:collection_descriminator

Conversation

@jmchilton

Copy link
Copy Markdown
Member

I think a bunch of this will be red based on other structural PRs uncovering pre-existing red workflows - but I'll extract the green stuff and try to work on isolating and green-ifying the red workflows.

FOR CONTRIBUTOR:

FOR REVIEWERS:

  • .dockstore.yml: file is present and aligned with creator metadata in workflow. ORCID identifiers are strongly encouraged in creator metadata. The .dockstore.yml file is required to run tests
  • Workflow is sufficiently generic to be used with lab data and does not hardcode sample names, reference data and can be run without reading an accompanying tutorial.
  • In workflow: annotation field contains short description of what the workflow does. Should start with This workflow does/runs/performs … xyz … to generate/analyze/etc …
  • In workflow: workflow inputs and outputs have human readable names (spaces are fine, no underscore, dash only where spelling dictates it), no abbreviation unless it is generally understood. Altering input or output labels requires adjusting these labels in the the workflow-tests.yml file as well
  • In workflow: name field should be human readable (spaces are fine, no underscore, dash only where spelling dictates it), no abbreviation unless generally understood
  • Workflow folder: prefer dash (-) over underscore (_), prefer all lowercase. Folder becomes repository in iwc-workflows organization and is included in TRS id
  • Readme explains what workflow does, what are valid inputs and what outputs users can expect. If a tutorial or other resources exist they can be linked. If a similar workflow exists in IWC readme should explain differences with existing workflow and when one might prefer one workflow over another
  • Changelog contains appropriate entries
  • Large files (> 100 KB) are uploaded to zenodo and location urls are used in test file

jmchilton and others added 3 commits July 13, 2026 11:38
Galaxy's Tests (GalaxyWorkflowTests) pydantic model and gxwf
validate-tests discriminate dataset-vs-collection outputs solely on
class: Collection. An output asserted with element_tests but no
class: Collection routes to the dataset branch (extra=forbid) and is
rejected, though planemo runs it fine. Add class: Collection to the
204 collection output/element nodes (recursive) across 50 files;
runtime ignores the key. gxwf validate-tests green count 39 -> 68,
no regressions.

24 files using the old duplicate-key asserts syntax are skipped here
(separate normalization).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…b inputs

Nested collection elements in test `job:` inputs used the legacy `type:`
key (e.g. `class: Collection` / `type: paired`) while the top-level
collection used `collection_type:`. The input Collection pydantic model
(galaxy.tool_util_models.test_job.Collection, extra=forbid) accepts only
`collection_type`; a stray nested `type:` fails the Collection branch and
makes the smart-union fall back to the File branch, producing confusing
"required property 'location'" / "additionalProperty: collection_type"
errors on the whole input. gxwf validate-tests job-input errors 686 -> 0.

Runtime-safe: planemo delegates staging to Galaxy's galactic_job_json
(lib/galaxy/tool_util/cwl/util.py). Its to_elements() derives every
nested sub-collection type by splitting the top-level collection_type
string ("list:paired" -> "paired" -> ...) and reads only `identifier`
and `elements` off each nested element -- never the element's own
`type`/`collection_type` key. The nested `type:` was dead metadata at
run time; renaming it is inert to planemo/Galaxy and only aligns the
fixtures with the strict Tests schema. No test-run behavior changes.

46 renames across 24 files. 24 files using the old duplicate-key asserts
syntax are unparseable by the round-trip loader and skipped (front C).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Applies the same two mechanical transforms to files that became
eligible on current main: several duplicate-key-asserts files that main
has since fixed (now round-trip parseable), a new test file, and
MAGs-generation (whose rebase conflict resolution re-ran only the input
rename). Re-running both idempotent transforms on the full tree closes
the coverage gaps the rebase left.

class:Collection added to 31 collection output/element nodes across 8
files; nested job-input type -> collection_type on 3. gxwf
validate-tests: 0 element_tests and 0 job-input discriminator errors
remain in parseable files (16 duplicate-key files still skipped, front C).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@mvdbeek mvdbeek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, i wonder if we'll hit the 6h timeout but in principle this is fine of course

@github-actions

Copy link
Copy Markdown

Test Results (powered by Planemo)

Test Summary

Test State Count
Total 32
Passed 26
Error 3
Failure 3
Skipped 0
Errored Tests
  • ❌ Pathogen-Detection-PathoGFAIR-Samples-Aggregation-and-Visualisation.ga_0
  • ❌ ont-artic-variation.ga_0
  • ❌ rnaseq-sr.ga_0
Failed Tests
  • ❌ Generic-variation-analysis-on-WGS-PE-data.ga_0
  • ❌ baredSC-1d-logNorm.ga_0
  • ❌ pe-wgs-variation.ga_0
Passed Tests
  • ✅ Generic-variation-analysis-reporting.ga_0
  • ✅ MAGs-binning-evaluation.ga_0
  • ✅ MAGs-generation.ga_0
  • ✅ MAGs-generation.ga_1
  • ✅ MAGs-taxonomy-annotation.ga_0
  • ✅ Nanopore-Pre-Processing.ga_0
  • ✅ Taxonomy-Profiling-and-Visualization-with-Krona.ga_0
  • ✅ Velocyto-on10X-filtered-barcodes.ga_0
  • ✅ Velocyto-on10X-from-bundled.ga_0
  • ✅ WGS-PE-variant-calling-in-haploid-system.ga_0
  • ✅ baredSC-2d-logNorm.ga_0
  • ✅ genotype-variant-calling-wgs-pe.ga_0
  • ✅ host-or-contamination-removal-on-long-reads.ga_0
  • ✅ host-or-contamination-removal-on-short-reads.ga_0
  • ✅ host-or-contamination-removal-on-short-reads.ga_1
  • ✅ metagenomic-raw-reads-amr-analysis.ga_0
  • ✅ metaprosip.ga_0
  • ✅ mfassignr.ga_0
  • ✅ pe-artic-variation.ga_0
  • ✅ pe-wgs-ivar-analysis.ga_0
  • ✅ pox-virus-half-genome.ga_0
  • ✅ rnaseq-pe.ga_0
  • ✅ scrna-seq-fastq-to-matrix-10x-cellplex.ga_0
  • ✅ scrna-seq-fastq-to-matrix-10x-v3.ga_0
  • ✅ short-read-quality-control-and-trimming.ga_0
  • ✅ tissue-micro-array-analysis.ga_0

@jmchilton

Copy link
Copy Markdown
Member Author

Note

Posted by Claude (AI assistant) on behalf of @jmchilton — not authored by them personally.

Closing this in favor of smaller PRs. This one bundled 55 workflow directories, which CI structurally cannot complete: the PR test job caps at max-chunks: 4 (.github/workflows/workflow_test.yml), so ~14 full Galaxy test runs landed in each chunk. Chunk 0 was cancelled at GitHub's 6h job limit part way through hyphy, and chunk 1's job died so hard GitHub dropped its log entirely, part way through cutandrun. Neither uploaded a results artifact, so 6 workflows never got a verdict at all.

What CI did tell us

From the combined artifact plus the raw chunk logs of run 29268785765: 38 workflow directories passed, 11 failed, 6 got no result.

Every failure has a pre-existing cause unrelated to the schema change in this PR. The diff here is two mechanical patterns only — class: Collection added to outputs that already had element_tests:/elements:, and type:collection_type: on nested collection elements in inputs. No .ga files or expected data touched. The 18-minute lint job produced exactly one ERROR across the whole repo, and it is a missing Toolshed revision, not anything in this diff.

Replacement PRs (expected green)

All workflows in these passed on this branch:

Held back — pre-existing breakage, needs work first

Root cause Workflows
compleasm/0.2.6+galaxy3 crashes parsing busco-data.ezlab.org's file_versions.tsv (ValueError: too many values to unpack (expected 3) in download_placement) — upstream tool bug Assembly-Hifi-HiC-phasing-VGP4, Purge-duplicate-contigs-VGP6, Purge-duplicates-one-haplotype-VGP6b, Scaffolding-HiC-VGP8, hi-c-contact-map
Tool revision not installable: iuc/fasttree/2.1.10+galaxy1 is not in the Toolshed (this is also the sole lint error); iuc/featurecounts/2.1.1+galaxy1 fails to install PathoGFAIR samples aggregation, rnaseq-sr
Stale expected VCFs — lofreq/SnpEff output has drifted from the checked-in files generic-variant-calling-wgs-pe, sars-cov-2-pe-illumina-wgs-variant-calling
Flaky/too-tight asserts — SRA upload of SRR12447380 errored; baredSC PNG size assert 108407 ± 10000 got 119622 sars-cov-2-ont-artic, baredSC-1d-logNorm

Never ran

hyphy, cutandrun, hic-hicup-cooler, functional-annotation-of-sequences, histological-staining-area-quantification, multiplex-tissue-microarray-analysis. These need their own small PR to get an actual verdict; cutandrun is the prime suspect for killing chunk 1 and is worth isolating.

Suggested follow-up

Raising max-chunks above 4 for PR CI (the weekly ci.yaml already uses 40) would stop repo-wide mechanical changes from hitting this wall again.

@jmchilton jmchilton closed this Jul 14, 2026
mvdbeek pushed a commit that referenced this pull request Jul 15, 2026
… reached in #1286)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mvdbeek added a commit that referenced this pull request Jul 15, 2026
…-rebased

tests: declare class:Collection / collection_type (workflows CI never reached in #1286)
mvdbeek pushed a commit that referenced this pull request Jul 16, 2026
…ularization

fasttree 2.1.10+galaxy1 is no longer installable from the toolshed (only
2.1.10 and 2.1.10+galaxy3 are), so the workflow could not be invoked and
lint errored. Supersedes #1162, which bumped content_id/tool_id/changeset
but left tool_version at +galaxy1 in all three fasttree steps.

Also carries the class:Collection / collection_type test annotations for
this workflow, split out of #1286.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants