Skip to content

feat(sprocket test): output assertions#565

Merged
claymcleod merged 54 commits intomainfrom
feat/test/outputs
Feb 11, 2026
Merged

feat(sprocket test): output assertions#565
claymcleod merged 54 commits intomainfrom
feat/test/outputs

Conversation

@a-frantz
Copy link
Copy Markdown
Member

@a-frantz a-frantz commented Jan 27, 2026

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

New user guide for these features - stjude-rust-labs/sprocket.bio#29

There is a strange serde bug which causes the YAML syntax used in this PR to deviate from the syntax used in the serde_yaml_ng docs. See this commit for details 9fbc964 that link to acatton/serde-yaml-ng#14 which links to more information

This is a bit of a "happy accident" bug IMO, as I prefer the alternate syntax used here as opposed to the YAML tag syntax the documentation uses.

I've started adding these to stjudecloud/workflows#263

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 made a PR to the next branch in the sprocket.bio repository (when appropriate).

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 Jan 27, 2026
@adthrasher
Copy link
Copy Markdown
Member

adthrasher commented Jan 28, 2026

Just off the top of my head, during our meeting.

Arrays: length(), first(), and last().
Pair: .left() and .right().

@adthrasher
Copy link
Copy Markdown
Member

A few more, though I'm not tied to the naming:

Map: has_key(), count()

Anything more complex is probably going to need to be a custom assertion. The same is true for testing struct types. I'm not sure if we need anything for an enum, I suspect they won't be common outputs and I don't know what you'd want to test other than that the variant is as expected.

@github-actions github-actions Bot removed the S-awaiting-pass-CI PR is awaiting CI to pass. label Jan 29, 2026
@a-frantz
Copy link
Copy Markdown
Member Author

A few more, though I'm not tied to the naming:

Map: has_key(), count()

count() has been lumped together with Length, which will work on String, Array, and Map.

has_key() is actually more complicated than it might seem, as any WDL primitive type can be the key. So it would actually have to be implemented per-type, which isn't hard, but does complicate the user interface for a set of assertions I don't think are super useful.

Anything more complex is probably going to need to be a custom assertion. The same is true for testing struct types. I'm not sure if we need anything for an enum, I suspect they won't be common outputs and I don't know what you'd want to test other than that the variant is as expected.

I want to think this over some more, but I think you're right. IMO the currently implemented set of assertions is about good for the "essential" assertions for this PR, but it should be easy to extend the set in future PRs.

At least, I'm going to stop implementing more unless someone asks for more specifically. Now time to write tests for all of this, which I'm not looking forward to 😭

@github-actions github-actions Bot added the S-awaiting-pass-CI PR is awaiting CI to pass. label Jan 29, 2026
@a-frantz a-frantz changed the title WIP[feat: output assertions] feat(sprocket test): output assertions Feb 2, 2026
@a-frantz a-frantz marked this pull request as ready for review February 2, 2026 22:42
@a-frantz a-frantz requested a review from a team as a code owner February 2, 2026 22:42
@a-frantz a-frantz requested a review from adthrasher February 2, 2026 22:42
Copy link
Copy Markdown
Member

@claymcleod claymcleod left a comment

Choose a reason for hiding this comment

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

Left a few comments after reviewing.

Comment thread src/test/assertions.rs Outdated
Copy link
Copy Markdown
Member

@claymcleod claymcleod left a comment

Choose a reason for hiding this comment

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

Whoops, forgot to include these in the last review!

Comment thread src/test/assertions.rs
Comment thread src/commands/test.rs
Comment thread src/commands/test.rs Outdated
Comment thread src/commands/test.rs Outdated
Comment thread src/commands/test.rs
Comment thread .gitignore Outdated
Comment thread src/test/assertions.rs
Comment thread src/test/assertions.rs
a-frantz and others added 2 commits February 11, 2026 15:59
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
@claymcleod claymcleod merged commit 897960a into main Feb 11, 2026
23 checks passed
@claymcleod claymcleod deleted the feat/test/outputs branch February 11, 2026 21:59
@github-actions github-actions Bot removed the S-awaiting-pass-CI PR is awaiting CI to pass. label Feb 11, 2026
bfrazho pushed a commit to bfrazho/sprocket that referenced this pull request Mar 22, 2026
Co-authored-by: Adam C. Foltzer <acfoltzer@acfoltzer.net>
Mahathi-s154 pushed a commit to Mahathi-s154/sprocket that referenced this pull request Mar 26, 2026
Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com>
Co-authored-by: Andrew Thrasher <adthrasher@gmail.com>
Co-authored-by: Peter Huene <peter@huene.dev>
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.

4 participants