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

Validation errors don't provide any context #140

Closed
parsonsmatt opened this issue Mar 6, 2018 · 2 comments · Fixed by #195
Closed

Validation errors don't provide any context #140

parsonsmatt opened this issue Mar 6, 2018 · 2 comments · Fixed by #195

Comments

@parsonsmatt
Copy link
Contributor

validateToJSON provides somewhat unhelpful error messages. Here's an example from IOHK codebase:

>>> encode newWallet
"{\"backupPhrase\":[\"shell\",\"also\",\"throw\",\"ramp\",\"grape\",\"chest\",\"setup\",\"mandate
\",\"spare\",\"verb\",\"lemon\",\"test\"],\"spendingPassword\":null,\"assuranceLevel\":\"strict\"
,\"name\":\"My Wallet\",\"operation\":\"create\"}"
>>> validateToJSON newWallet
["expected JSON value of type SwaggerString"]

It's not mentioned what field was expected to be a JSON value of type SwaggerString, just that one of them was. It would be nice to have more information on what failed the test.

@fizruk
Copy link
Member

fizruk commented Mar 7, 2018

I totally agree!
In fact I think something like default FromJSON instances do should be doable.
Also we might want to add what that value was (instead of just saying what it is not). In some cases I imagine this might be a huge value, so we could at least mention type of an actual value.

Perhaps something like this could work:

>>> validateToJSON newWallet
["$.spendingPassword: expected JSON value of type SwaggerString but got null"]

Since we're not expected to validate with incredible performance, I think this is a straightforward task.
Would like to implement a patch?

@parsonsmatt
Copy link
Contributor Author

I have other priorities at the moment :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants