feat(vest): add getData resolver to suite result#1250
Open
ealush wants to merge 1 commit intopr/vest-schema-filteringfrom
Open
feat(vest): add getData resolver to suite result#1250ealush wants to merge 1 commit intopr/vest-schema-filteringfrom
ealush wants to merge 1 commit intopr/vest-schema-filteringfrom
Conversation
| Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoAdd getData resolver to suite result for accessing validated data
WalkthroughsDescription• Add getData() method to suite results for accessing validated data • Returns parsed schema output on success, falls back to raw input on errors • Expose getData directly on suite instance for convenience access • Comprehensive unit tests and documentation for new functionality Diagramflowchart LR A["Suite Result"] -->|getData call| B["Has Errors?"] B -->|Yes or No Schema| C["Return Raw Input"] B -->|No Errors & Schema| D["Return Parsed Output"] E["Suite Instance"] -->|getData method| A
File Changes1. packages/vest/src/suite/SuiteTypes.ts |
Code Review by Qodo
1. getData ignores suite validity |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Vest validation library by adding a Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a getData() method to the suite result, which is a great feature for conveniently accessing the validated and parsed data. The implementation is solid, and the new tests and documentation are comprehensive. I've found one potential issue where the condition for returning the parsed data is a bit too lenient compared to the documentation, which could lead to unexpected behavior with asynchronous validations. My suggestion aims to make this behavior safer and more consistent with the documentation.
Comment on lines +45 to +46
| getData: () => | ||
| !resultBody.hasErrors() && schema ? outputData : inputData, |
Contributor
There was a problem hiding this comment.
There's a potential issue here with the condition for returning parsed data. The implementation uses !resultBody.hasErrors(), but the documentation added in this PR (in website/docs/writing_your_suite/accessing_the_result.md) states that parsed data should be returned only when the suite is 'fully valid' (i.e., isValid() is true).
The isValid() check is stricter than !hasErrors(), as it also accounts for pending tests and whether all required tests have been run. Returning parsed data while some async tests are still pending could lead to using partially validated or stale data.
To align with the documentation and ensure safer behavior, it would be better to use resultBody.isValid().
getData: () => (resultBody.isValid() && schema ? outputData : inputData),![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines +45 to +46
| getData: () => | ||
| !resultBody.hasErrors() && schema ? outputData : inputData, |
There was a problem hiding this comment.
1. Getdata ignores suite validity 🐞 Bug ✓ Correctness
SuiteResult.getData() returns parsed output when hasErrors() is false, but a suite can be invalid (isValid() false / valid=false) without errors (e.g., pending/missing tests or an empty test array). This contradicts the new documentation and can surface coerced/parsed data before validation is actually complete or successful.
Agent Prompt
### Issue description `SuiteResult.getData()` currently returns parsed/coerced schema output whenever `hasErrors()` is false, but `hasErrors()` only reflects error count and does not mean the suite is valid/completed. This can return parsed output while `isValid()` is false (e.g., empty suite / pending / missing tests), contradicting the docs and potentially causing consumers to rely on coerced data before validation success. ### Issue Context - Docs state parsed output should be returned only when the suite is fully valid (`res.isValid()` is true). - Vest validity accounts for pending/missing tests and empty test arrays, which are not reflected by `hasErrors()`. ### Fix Focus Areas - packages/vest/src/suiteResult/suiteResult.ts[43-49] - packages/vest/src/suiteResult/__tests__/getData.test.ts[1-129] ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
🚀 Benchmark Results
Raw OutputPREVIOUS_RESULTS🚀 Benchmark Results
Raw Output🚀 Benchmark Results
Raw Output🚀 Benchmark Results
Raw Output🚀 Benchmark Results
Raw Output🚀 Benchmark Results
Raw Output |
ealush force-pushed the pr/vest-schema-filtering branch from e5026fc to 27544be Compare 
ealush force-pushed the pr/vest-schema-filtering branch from 27544be to 378fda0 Compare ealush force-pushed the pr/vest-schema-filtering branch from 378fda0 to d66a01a Compare ealush force-pushed the pr/vest-schema-filtering branch from d66a01a to 1cf023c Compare ealush force-pushed the pr/vest-schema-filtering branch from 1cf023c to a4718ab Compare ealush force-pushed the pr/vest-schema-filtering branch from a4718ab to 854b5d6 Compare ealush force-pushed the pr/vest-schema-filtering branch from 854b5d6 to 08bf484 Compare ealush force-pushed the pr/vest-schema-filtering branch from fc1b209 to b84a308 Compare 
Add getData() method to suite results that returns the validated schema parse output when validations pass, or falls back to raw input data. Exposes getData on the suite instance for direct access. Includes unit tests, type definitions, and updated snapshots.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getData()method to suite results: returns validated schema parse output when no errors exist, otherwise falls back to raw input datagetDatadirectly on the suite instance for conveniencegetDataonSuiteResultandSuiteTypesgetDatafunction on resultsStack
Depends on #1249
Test plan
getData(returns parsed output on valid, raw input on errors, undefined without schema)