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 test to validate WDLs in the scripts directory. #7826

Merged
merged 1 commit into from
May 4, 2022

Conversation

gbggrant
Copy link
Collaborator

@gbggrant gbggrant commented May 3, 2022

No description provided.

@gbggrant gbggrant requested a review from cmnbroad May 3, 2022 21:10
@gbggrant gbggrant marked this pull request as ready for review May 3, 2022 21:15
@gbggrant
Copy link
Collaborator Author

gbggrant commented May 3, 2022

@cmnbroad I saw your comment in #7822. I'm trying to add WDL validation here, using the infrastructure in gradle. I was interested here in just validating the WDLs in the scripts directory.

Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

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

@gbggrant One comment on naming, otherwise looks good to merge once thats addressed. Thanks for doing this.

#test wdl validation
wdlValidation:
runs-on: ubuntu-latest
name: Validate WDLs using womtools
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you modify the names (workflow names, name property, gradle task names, and comments) so the "scripts" and "generated" versions are more clearly distinguished from each other (i.e., validteScriptsWDL and validateGeneratedWDL or something similar). Its hard to tell from looking at just this definition that it's not doing the autogenerated validation.

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.

👍 with Chris's suggestions

@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #7826 (91af7ec) into master (c72f2a8) will decrease coverage by 0.002%.
The diff coverage is n/a.

❗ Current head 91af7ec differs from pull request most recent head db7b43f. Consider uploading reports for the commit db7b43f to get more accurate results

@@               Coverage Diff               @@
##              master     #7826       +/-   ##
===============================================
- Coverage     86.955%   86.953%   -0.002%     
+ Complexity     36903     36902        -1     
===============================================
  Files           2214      2214               
  Lines         173540    173540               
  Branches       18735     18735               
===============================================
- Hits          150902    150899        -3     
- Misses         16036     16039        +3     
  Partials        6602      6602               
Impacted Files Coverage Δ
.../hellbender/utils/python/PythonUnitTestRunner.java 75.410% <0.000%> (-3.279%) ⬇️
...nder/utils/runtime/StreamingProcessController.java 66.818% <0.000%> (-0.455%) ⬇️
...itute/hellbender/tools/LocalAssemblerUnitTest.java 92.448% <0.000%> (ø)

Renamed existing test for validating generated WDLs.
@gbggrant gbggrant merged commit e0e6669 into master May 4, 2022
@gbggrant gbggrant deleted the gg_AddWdlValidationTest branch May 4, 2022 20:13
gbggrant added a commit that referenced this pull request May 4, 2022
Renamed existing test for validating generated WDLs.
gbggrant added a commit that referenced this pull request May 5, 2022
* Added a test to validate WDLs in the scripts directory. (#7826)
Renamed existing test for validating generated WDLs.
This was referenced Mar 17, 2023
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