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

Show sub-errors for oneOf #216

Merged
merged 4 commits into from
Jan 22, 2015
Merged

Show sub-errors for oneOf #216

merged 4 commits into from
Jan 22, 2015

Conversation

isage
Copy link
Contributor

@isage isage commented Jan 15, 2015

anyOf and allOf show sub-errors.
with this patch oneOf do the same

@iainbeeston
Copy link
Contributor

Great idea, could you add some tests for this similar to those for any of and all of?
(e.g. in test_all_of_ref_schema.rb and test_any_of_ref_schema.rb)

@iainbeeston
Copy link
Contributor

Also, this change seems to cause a test failure related to oneOf

@isage
Copy link
Contributor Author

isage commented Jan 15, 2015

Looking at the test, i'm not sure why it was working before.
insert_defaults will insert foo => "view" into data, so data now matches both variants of oneOf, which should fail.

But still, there's seems to be an error in my patch, because even {foo => view} doesn't seem to validate
I'll try to fix this and write tests

@isage
Copy link
Contributor Author

isage commented Jan 15, 2015

Oh, okay, i finally get, how test works. "default" inserts value only for one oneOf case, which will fail with strict, and the first one will pass.
I forgot to handle exceptions, that's why it was failing. Now it works.
I'll write tests for oneOf tomorrow

@isage
Copy link
Contributor Author

isage commented Jan 15, 2015

Okay, i've added tests for sub-errors

@RST-J
Copy link
Contributor

RST-J commented Jan 20, 2015

Seems fine to me 👍

@iainbeeston
Copy link
Contributor

Ok, I'm happy with this 👍

Merging

iainbeeston added a commit that referenced this pull request Jan 22, 2015
Show sub-errors for oneOf
@iainbeeston iainbeeston merged commit aded4d7 into voxpupuli:master Jan 22, 2015
@inglesp inglesp mentioned this pull request Feb 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants