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

feat(validation-tests): validation tests path resolution #845

Merged
merged 13 commits into from
Dec 20, 2024

Conversation

meganwolf0
Copy link
Collaborator

@meganwolf0 meganwolf0 commented Dec 11, 2024

Description

Updates the validation testing and fixes a bug with the way indicies were identified (see #840 ) for specific bug.
This PR includes

  • Re-factor of the existing filter/path functionality. Intent was to increase test coverage and generally make this package easier to understand/use
  • Additional functionality to delete an item from a list (this was not previously supported). Solving the bug above yielded some code that made this functionality pretty easy to implement.
  • Updated the path parsing to make it more semantic for users - previously all items needed to be "." delimited, now made it so key[foo=bar] is equivalent to key.[foo=bar]
  • DOCS! Added some new docs for testing a validation, as I felt that was missing context. Also updated the testing.md reference doc.

Related Issue

Fixes #840

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@meganwolf0 meganwolf0 changed the title 840 validation tests path resolution (feat): validation tests path resolution Dec 11, 2024
@meganwolf0 meganwolf0 changed the title (feat): validation tests path resolution feat(validation-tests): validation tests path resolution Dec 12, 2024
@meganwolf0 meganwolf0 marked this pull request as ready for review December 12, 2024 17:13
@meganwolf0 meganwolf0 requested a review from a team as a code owner December 12, 2024 17:13
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

One typo to fix, the other suggestions you are free to take or ignore as you see fit! This looks good, and I like your documentation.

docs/getting-started/develop-a-validation.md Outdated Show resolved Hide resolved
docs/getting-started/test-a-validation.md Show resolved Hide resolved
docs/getting-started/test-a-validation.md Outdated Show resolved Hide resolved
docs/reference/testing.md Outdated Show resolved Hide resolved
mildwonkey
mildwonkey previously approved these changes Dec 16, 2024
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

(I'll re-approve if you want to change the integer map key wording but my actual requested changes are resolved)

Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

I like the changes! 😀 ❤️ :shipit:

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

I really like the test-a-validation.md doc.

@brandtkeller brandtkeller merged commit c54d5ba into main Dec 20, 2024
10 checks passed
@brandtkeller brandtkeller deleted the 840-validation-tests-path-resolution branch December 20, 2024 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Validation tests path not resolving as expected
3 participants