-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
Chore/increase client api test coverage #8950
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
@FredrikOseberg, core features have been modified in this pull request. Please review carefully! |
I think the snapshot helps us to identify if we are:
I think this is a good improvement, but I'd like to know if this would have helped us detect that there was something off with the PR #8856 |
Confirmed that they will catch these cases in the future as long as we don't use a special variable to ignore the property in tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Added more tests around specific plans. Also added snapshot as per our conversation @gastonfournier, but I'm unsure how much value it will give because it seems that the tests should already catch this using respondWithValidation and the OpenAPI schema. The problem here is that empty array is a valid state, so there were no reason for the schema to break the tests.