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

Don't error when there is empty OneOf #55

Merged
merged 4 commits into from
Apr 30, 2015
Merged

Don't error when there is empty OneOf #55

merged 4 commits into from
Apr 30, 2015

Conversation

pksunkara
Copy link
Contributor

Boutique gives exceptions when it encounters empty OneOf groups. For example:

+ Attributes (object)
    + id: pavan
    + One of

Also release 0.1.7 version.

]
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

you can provide the same test with using the current testsuite:

  • put AST into test/formats/samples-ast as a new sample under name of your selection
  • put rendered JSON into test/formats/samples-json under the same name
  • add the test case to formats-test.coffee

Also please test all the situations you fixed - oneOf for JSON Schema, oneOf for JSON, etc. How current testsuite basically works (this could be in the README, shame on me :-/ ):

  • you write .mson samples for your test cases
  • you run scripts/generate-samples-ast.coffee from command line - it should generate AST samples for you
  • you check the AST samples
  • you write down expected JSON and JSON Schema samples
  • you add these new test cases to the formats-test.coffee file
  • you run tests

The problem is that the script is still for protagonist-experimental instead of drafter, but that can be easily fixed I guess. Feel free to ask in case you're in troubles :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing the script from protagonist-experimental to drafter is failing a few tests. I didn't want to deal with it for now.

@honzajavorek
Copy link
Contributor

Please, make the test part of the existing test suite. Otherwise looks good!

@pksunkara
Copy link
Contributor Author

@honzajavorek Updated.

honzajavorek added a commit that referenced this pull request Apr 30, 2015
Don't error when there is empty OneOf
@honzajavorek honzajavorek merged commit d881db4 into master Apr 30, 2015
@honzajavorek honzajavorek deleted the pksunkara/fix branch April 30, 2015 08:23
@honzajavorek
Copy link
Contributor

🚀

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.

None yet

2 participants