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

Add yajl, multi_json and uuidtools to travis #157

Merged
merged 7 commits into from Oct 28, 2014

Conversation

iainbeeston
Copy link
Contributor

I discovered today that there are a few extra code paths that will only be activated if we use one of the following gems:

  • yajl
  • multi_json
  • uuid tools

It's possible for json-schema to break when using some of those gems, but for the test suite to still pass. This pull request should fix that problem.

@iainbeeston iainbeeston changed the title Add yawl, multi_json and uuidtools to travis Add yajl, multi_json and uuidtools to travis Oct 27, 2014
We have a few extra code paths that will only be activated if we use
one of these gems. Adding them to the build matrix should cover those
cases with all the tests in the regular test suite.
Previously travis was failing because it was looking for a gemspec in
the gemfiles directory
By default it returns nil, but unfortunately json-schema expects failure
to result in an exception. If nil is returned it's treated as a valid
result and is used. In the case of initialize_data this is a problem
because it overwrites the data passed in with nil
@iainbeeston
Copy link
Contributor Author

It turns out that yajl support is broken - it will parse strings that contain a json hash just fine, but if you pass it a string that does not parse, json-schema throws it away and tries to parse nil instead. Two of the common test suite tests picked this up (but I'm surprised it wasn't more than that)

@iainbeeston iainbeeston force-pushed the test_optional_gems branch 2 times, most recently from 0af4076 to 35a828d Compare October 27, 2014 22:32
latest version of ruby

Should make the build faster
@pd
Copy link
Contributor

pd commented Oct 27, 2014

👍, it's cool you got this down to fewer jobs than what it was earlier today =)

@hoxworth
Copy link
Contributor

👍 What a mess, thanks for taking these on @pd and @iainbeeston

hoxworth added a commit that referenced this pull request Oct 28, 2014
Add yajl, multi_json and uuidtools to travis
@hoxworth hoxworth merged commit 3d2632d into voxpupuli:master Oct 28, 2014
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

3 participants