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 conformance test for MultipleInputFeatureRequirement on Workflow outputs #169

Merged
merged 2 commits into from
Jun 23, 2022

Conversation

kinow
Copy link
Member

@kinow kinow commented May 10, 2022

For common-workflow-language/common-workflow-language#637

Tried to write a test that calls echo-tool.cwl twice, storing the output, and then uses both values in the workflow outputs. Confirmed it fails without the MultipleInputFeatureRequirement.

@kinow kinow requested a review from mr-c May 10, 2022 10:10
@@ -3532,3 +3532,15 @@
doc: |
Use of $(runtime.outdir) for outputBinding glob.
tags: [ required, command_line_tool ]

- label: multiple-input-feature-requirement
Copy link
Member Author

Choose a reason for hiding this comment

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

p.s.: was reading the doc for conformance tests, and noticed this line:

label: a short list of underscore (_) separated words that succinctly identifies and explains the test.

I had already used dashes, as I did in the previous test, where I followed the pattern of the previous test… Is there a reason for underscores? Should we fix the existing labels? Or maybe update https://github.com/common-workflow-language/cwl-v1.2/blob/1.2.1_proposed/CONFORMANCE_TESTS.md?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm.. I don't have a strong opinion right now, except that we should be consistent. Can you make a separate issue to discuss this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree! Done: #191

Thanks

@kinow
Copy link
Member Author

kinow commented Jun 21, 2022

Rebased and fixed conflicts.

@mr-c mr-c enabled auto-merge (squash) June 23, 2022 15:42
@mr-c mr-c disabled auto-merge June 23, 2022 15:42
@mr-c mr-c merged commit d3c77ef into 1.2.1_proposed Jun 23, 2022
@mr-c mr-c deleted the multiple_input-output branch June 23, 2022 15:42
kinow added a commit to kinow/cwl-v1.2 that referenced this pull request Oct 20, 2022
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