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

Test new "when" directive #23

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Test new "when" directive #23

wants to merge 4 commits into from

Conversation

svonworl
Copy link
Collaborator

@svonworl svonworl commented Mar 22, 2022

This PR tests mr-c's "when" implementation, https://github.com/galaxyproject/gxformat2/pull/74/commits, which I merged with master of https://github.com/galaxyproject/gxformat2. I imported the resulting source into this repo (which incorporates denis-yuen's recent binding updates) via updateparser.sh , added a dependency to the pom to get it building, and modified the appropriate IT to point at a forked test repo that incorporates a "when" clause in a galaxy format 2 file. The test repo is currently in my github account, we can move it if necessary.

This code builds, the tests pass, and I was able to load the resulting Galaxy plugin into a develop build of Dockstore and successfully import a Galaxy repo containing two versions: one with a well-formed "when" directive, which Dockstore marks valid, and one with a not-well-formed "when" directive, which Dockstore marks not valid. Dockstore displays a validation error message when I view the latter sourcefile.

The ITs don't currently contain a test for a bad "when" directive (for example, a "when" in the right position, but with malformed value), but I did try to load one while creating the test repo (when: true rather than when: "true"), and the IT failed correctly.

So, tentatively, I think it all works!

@denis-yuen
Copy link
Collaborator

Yeah, I think this makes sense and is what I expected.

Copy link
Collaborator

@denis-yuen denis-yuen left a comment

Choose a reason for hiding this comment

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

Approving, but not in the sense of approving for merge(?)
This is a bit weird, we should tag the CWL/Galaxy folks to let them know we looked into this albeit with a big lag.

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