Upgrade miniwdl to 1.15.0 and refresh WDL unit test skip lists (#5377) - #5567
Upgrade miniwdl to 1.15.0 and refresh WDL unit test skip lists (#5377)#5567annagiroti wants to merge 2 commits into
Conversation
miniwdl 1.15.0 adds parsing support for WDL 1.2 `hints` sections, unblocking several previously-skipped conformance unit tests and letting most nested-input tests run. Removes 30 entries across the WDL 1.1 and 1.2 unsupported-test lists that now pass, and updates the remaining entries' comments to reflect their current (rather than pre-upgrade) failure reasons.
adamnovak
left a comment
There was a problem hiding this comment.
This looks OK, but I think for the return codes ones we might be removing information that's still maybe accurate.
I think the CI failed due to the code not type checking against the new MiniWDL, so you might need to fix that.
| "single_return_code_task", # Workflow did not return the correct return code! Got: 1. Expected: 0. | ||
| "all_return_codes_task", # Workflow did not return the correct return code! Got: 42. Expected: 0. |
There was a problem hiding this comment.
Has MiniWDL 1.15 learned about return_codes? I think the Got: codes are probably right for these and it's the expected codes that are wrong.
…code test comments
adamnovak
left a comment
There was a problem hiding this comment.
This looks OK, but we probably want to redesign our own caching stuff a bit if the MiniWDL caching now needs the inputs and not just the key.
| cache_key: str, | ||
| output_bindings: WDLBindings, | ||
| file_store: AbstractFileStore, | ||
| wdl_options: WDLContext, | ||
| input_bindings: WDLBindings, |
There was a problem hiding this comment.
If we now need to pass the whole input bindings to the cache, we probably should move responsibility for computing the key inside this function.
|
I looked a little at the failing tests. I think @annagiroti I invited you to that repo so you should be able to fix it there. You can run that test in that repo with: For the spec tests, the I'm not sure why, but you can run it with: (This grabs a patched version of the spec tests that has openwdl/wdl#799 so it actually contains only parse-able JSON. You could also set up the pinned version of the spec tests that Toil is using.) |
miniwdl 1.15.0 adds parsing support for WDL 1.2
hintssections, unblocking several previously-skipped conformance unit tests and letting most nested-input tests run. Removes 30 entries across the WDL 1.1 and 1.2 unsupported-test lists that now pass, and updates the remaining entries' comments to reflect their current (rather than pre-upgrade) failure reasons.miniwdl 1.15.0 also changed CallCache.put()'s signature, adding two new required keyword arguments,
inputsandadd_paths. Threads the actual input bindings through to all three call sites (task cache, workflow cache, and the write_* file cache) so cache entries record real inputs;add_pathsis passed empty since Toil doesn't track that concept yet.Addresses #5377
Changelog Entry
To be copied to the draft changelog by merger:
Reviewer Checklist
issues/XXXX-fix-the-thingin the Toil repo, or from an external repo.camelCasethat want to be insnake_case.docs/running/{cliOptions,cwl,wdl}.rstMerger Checklist