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.

Sign up for GitHub

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

Jump to bottom

Adding array operations unit test WDL #25

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Adding array operations unit test WDL #25

wants to merge 4 commits into from

Conversation

tefirman
Copy link
Collaborator

@tefirman tefirman commented Dec 17, 2024 •
edited
Loading

Description

  • Adding an "Array Operations" workflow that tests array operations and scatter-gather functionality by converting an array of strings to uppercase.

Related Issues

Testing

  • Ran locally using miniwdl, succeeds.

@tefirman tefirman marked this pull request as ready for review December 17, 2024 00:55
@tefirman tefirman requested a review from seankross December 17, 2024 00:55
@sitapriyamoorthi
Copy link
Collaborator

@tefirman this looks great! Wondering if we should add tests that specifically evaluate:

  1. Empty arrays as inputs
  2. A way to ensure elements of an array are correctly indexed
  3. Verifying that the length, flatten and sort functions applied to arrays works
  4. Maybe test array concatenation (overkill?)
  5. Testing array of arrays works

Just some thoughts... feel free to comment on if all or some of these will be useful!

@seankross seankross added the unit test Adding or modifying a unit test label Dec 18, 2024
@tefirman
Copy link
Collaborator Author

tefirman commented Dec 19, 2024 •
edited
Loading

@sitapriyamoorthi -- Great call. I added a few more tasks to implement your suggestions:

  • ValidateIndex
  • ArrayFunctions
  • ArrayConcat
  • Array of arrays as an input

Not super worried about empty arrays, but I did run it with an empty array and it seems to work fine. Mind taking another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seankross seankross Awaiting requested review from seankross

@sitapriyamoorthi sitapriyamoorthi Awaiting requested review from sitapriyamoorthi

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
unit test Adding or modifying a unit test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Array of local task inputs localization
3 participants
@tefirman @sitapriyamoorthi @seankross