Skip to content

feat: first step towards the test command#468

Merged
a-frantz merged 48 commits intomainfrom
feat/test
Dec 8, 2025
Merged

feat: first step towards the test command#468
a-frantz merged 48 commits intomainfrom
feat/test

Conversation

@a-frantz
Copy link
Copy Markdown
Member

@a-frantz a-frantz commented Nov 19, 2025

Describe the problem or feature in addition to a link to the issues.

As the wdl-engine interface is currently undergoing a refactor, this PR does not interact with wdl-engine at all. This PR simply defines the YAML which will be used for defining unit tests and does some basic parsing and logging as proof of concept.

stjude-rust-labs/rfcs#1

Before submitting this PR, please make sure:

For external contributors:

  • You have read the contributing guide in its entirety.
  • You have not used AI on any parts of this pull request.

For all contributors:

  • You have added a few sentences describing the PR here.
  • You have added yourself or the appropriate individual as the assignee.
  • You have added at least one relevant code reviewer to the PR.
  • Your code builds clean without any errors or warnings.
  • You have added tests (when appropriate).
  • You have added an entry in the CHANGELOG (when appropriate).
  • You have updated the README or other documentation to account for these changes (when appropriate).
  • You have either (a) made a PR to the next branch in the sprocket.bio repository [if you are a Sprocket team member] or (b) have suggested any changes you think need to be made to sprocket.bio in the description of your PR [if you are an external contributor].

For PRs containing lint rule changes:

  • You have updated any and all effected entries within RULES.md.
  • You have added a test case in crates/wdl-lint/tests/lints that covers every
    possible diagnostic emitted for the rule within the file where the rule
    is implemented.

@github-actions github-actions Bot added the S-awaiting-pass-CI PR is awaiting CI to pass. label Nov 19, 2025
@github-actions github-actions Bot removed the S-awaiting-pass-CI PR is awaiting CI to pass. label Nov 30, 2025
@github-actions github-actions Bot added the S-awaiting-pass-CI PR is awaiting CI to pass. label Nov 30, 2025
moves some logging out of `INFO` (stderr from tracing crate) and into plain STDOUT
Copy link
Copy Markdown
Contributor

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

This looks like a great first step towards the test command.

I have a few feedback comments and at least one "discussion" comment regarding the UX of the feature that we can easily punt to later if desired.

Comment thread src/commands/test.rs Outdated
Comment thread src/test.rs Outdated
Comment thread src/test.rs Outdated
Comment thread src/commands/test.rs Outdated
Comment thread src/commands/test.rs
Comment thread src/test.rs Outdated
Comment thread src/test.rs Outdated
Comment thread src/commands/test.rs Outdated
@a-frantz a-frantz requested review from peterhuene and removed request for acfoltzer December 7, 2025 15:10
@a-frantz
Copy link
Copy Markdown
Member Author

a-frantz commented Dec 7, 2025

@peterhuene I believe that I've "rustified" this sufficiently. We are yielding iterators, avoiding redundant String allocs, and validating+collecting errors as they are encountered then reporting all of the problems with found tests.

I've also gone ahead and added some simple tagging and filtering of tests (--include-tag and --filter-tag CL args).

Lastly, I've made another change to where tests are looked for. In addition to test YAML being looked for at "sibling" locations (e.g. tools/foo.wdl and tools/foo.yaml) tests can also be nested under a sibling test/ directory (e.g. tools/foo.wdl and tools/test/foo.yml) The cli:matrix_combinations test shows an example:

tests/cli/test/matrix_combinations/inputs/
├── example.wdl
├── example.yaml
├── flag_filter.wdl
└── test
    └── flag_filter.yml

Test files still have to have the same basename as the WDL they are testing.

I believe the above addresses all the received feedback 🎉 thanks for the review! LMK if there are any other changes we want to get in this PR

Copy link
Copy Markdown
Contributor

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

Looks great, just two nit comments!

Comment thread src/commands/test.rs
Comment thread src/commands/test.rs Outdated
@a-frantz a-frantz requested a review from peterhuene December 8, 2025 17:19
@a-frantz a-frantz merged commit 4db7c51 into main Dec 8, 2025
20 checks passed
@a-frantz a-frantz deleted the feat/test branch December 8, 2025 18:06
raman976 pushed a commit to raman976/sprocket that referenced this pull request Dec 11, 2025
@a-frantz a-frantz mentioned this pull request Dec 15, 2025
12 tasks
Mahathi-s154 pushed a commit to Mahathi-s154/sprocket that referenced this pull request Mar 26, 2026
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.

3 participants