Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a github action to run womtool validation on all WDLs #7822

Closed
wants to merge 6 commits into from

Conversation

gbggrant
Copy link
Collaborator

@gbggrant gbggrant commented Apr 29, 2022

Added a github action to run womtool validation on all WDLs
Also fixed two wdls that were failing validation.

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@d51a4e5). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             ah_var_store     #7822   +/-   ##
================================================
  Coverage                ?   86.280%           
  Complexity              ?     35189           
================================================
  Files                   ?      2170           
  Lines                   ?    164837           
  Branches                ?     17775           
================================================
  Hits                    ?    142221           
  Misses                  ?     16286           
  Partials                ?      6330           

Fixed two wdls that were failing to validate
@gbggrant gbggrant changed the title Trying to add womtool check Add a github action to run womtool validation on all WDLs Apr 29, 2022
@gbggrant gbggrant marked this pull request as ready for review April 29, 2022 19:20
@@ -151,7 +151,7 @@ workflow JointCallExomeCNVs {
}
Array[String] subset_arr = select_all(subset)
Array[String] subset_index_arr = select_all(subset_indexes)
Array[String] failed_samples = select_all(failed)
Array[String] failed_samples_arr = select_all(failed)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed the name of this (internal) variable as it is also defined as an output and was failing validation for that reason

@@ -0,0 +1,1070 @@
version 1.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Completely added this file to make JointGenotyping.wdl validate (it couldn't find the file, which is zipped up in a dependencies zip).

Copy link
Collaborator

@mcovarr mcovarr left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for adding this!

.github/workflows/womtool_check.yml Outdated Show resolved Hide resolved
.idea
)
declare -r CROMWELL=https://github.com/broadinstitute/cromwell
declare -r WOMTOOL=$CROMWELL/releases/download/78/womtool-78.jar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the drive-by, but I wanted to point out that there is already a gradle task and github action that we use to do something very similar (install cromwell and womtool to validate and test the autogenerated WDLs). It would be good to integrate with that, or at least use the same cromwell and womtool versions for all the WDLs in the repo (not sure how far back this branch is). BTW, I think the existing tasks use pretty old cromwell and womtool versions, but its probably time to update those. Either way I'll make a PR to update those, but just wanted to make sure you knew about the existing machinery.

@mcovarr
Copy link
Collaborator

mcovarr commented May 4, 2022

OOC does this PR stand in its current form or is #7826 going to be backported / cherry picked to ah_var_store once that merges?

@gbggrant gbggrant closed this May 4, 2022
@gbggrant gbggrant deleted the gg_AddWomToolCheck branch May 4, 2022 20:24
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.

None yet

3 participants