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

Document writing a new conformance test #124

Merged
merged 8 commits into from
Oct 6, 2021
Merged

Document writing a new conformance test #124

merged 8 commits into from
Oct 6, 2021

Conversation

mr-c
Copy link
Member

@mr-c mr-c commented Oct 4, 2021

@mr-c mr-c requested a review from tetron October 4, 2021 15:31
and better explain the format of that file
and better how to invoke ./run-tests.sh
@mr-c mr-c marked this pull request as ready for review October 5, 2021 09:35
@mr-c mr-c requested a review from rupertnash October 5, 2021 09:35
@rupertnash
Copy link
Contributor

OK - I'm going to try to write the conformance test needed to close #86 following these instructions like the literal minded pedant that I am.

First thing I wonder is, do I need to install cwltest or does the run_test.sh script do that for me? Do I need to do any setup? If so, a requirements.txt would be nice and a reminder to the less experienced that they should create a venv for this.

No conda instructions for now, that package is out of date: bioconda/bioconda-recipes#30871
@mr-c
Copy link
Member Author

mr-c commented Oct 5, 2021

First thing I wonder is, do I need to install cwltest

Yes, thanks for the reminder. I added basic instructions for installing using a virtualenv. I'll mention the conda package once bioconda/bioconda-recipes#30871 is merged and released

@rupertnash
Copy link
Contributor

rupertnash commented Oct 5, 2021

We had a brief discussion about terminology: we converged on a summary that:

A CWL document contains one or more CWL descriptions

CONFORMANCE_TESTS.md Outdated Show resolved Hide resolved
@rupertnash
Copy link
Contributor

What do we do about path for files and directories? I can see two cases where it's specified as path: Any and no cases where it's set to anything else.

@rupertnash
Copy link
Contributor

What are the rules for adding an id to a conf. test? In the main conformance_tests.yaml, they are integers but in the imported ones they are strings $prefix-$local_integer

Is an id even required?

@mr-c
Copy link
Member Author

mr-c commented Oct 5, 2021

What are the rules for adding an id to a conf. test? In the main conformance_tests.yaml, they are integers but in the imported ones they are strings $prefix-$local_integer

We are migrating off of id to only use the labels: common-workflow-language/cwltest#110

Is an id even required?

No, id is optional: https://github.com/common-workflow-language/cwltest/blob/097088ecfd599c7a096e9d3fc05d458456d2a641/cwltest/cwltest-schema.yml#L8

rupertnash
rupertnash previously approved these changes Oct 5, 2021
Copy link
Contributor

@rupertnash rupertnash left a comment

Choose a reason for hiding this comment

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

This now covers the questions I had in writing a test :)

@mr-c
Copy link
Member Author

mr-c commented Oct 5, 2021

Thanks @rupertnash ! I need you to review my latest clarifications again (this repo is sensitive about changes to the main branch)

@mr-c mr-c requested a review from rupertnash October 5, 2021 16:38
Copy link
Contributor

@rupertnash rupertnash left a comment

Choose a reason for hiding this comment

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

The last commit further clarifies - I approve

@mr-c mr-c merged commit 4bd3310 into main Oct 6, 2021
@mr-c mr-c deleted the howto_write_test branch October 6, 2021 08:47
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.

About conformance test requires location: Any what does it mean ?
2 participants