Fix test runs for optional dependencies

[iainbeeston]

This has become a bit of a mammoth fix to what I thought was a minor problem!

Here's a rundown of what I've learned:

1. On travis, the builds that test json-schema with optional dependencies have not been running since we introduced minitest, because the minitest dependency was only defined in the main Gemfile (and so the optional test runs would still be using test unit, and wouldn't find any valid test classes in our /tests directory). If you look in travis, the optional builds have been running 0 tests.
2. When I fixed this (so that the optional test runs were executing correctly) a single test in the common test suite was failing but only with yajl and multi_json, and not for the main test suite, because with those parses json schema was interpreting "127.0" as a float, whereas by default (with the json gem) it was a string (and with a string, the tests pass).

yajl and multi_json are right - "127.0" should be a float.

What's happening in the json-schema parsing code right now, is that if a json text is passed in and it doesn't parse, then we leave it as a string (then validate that string). However, yajl/multi_json parse "127.0" without errors (and return a number for validation), whereas json-gem throws an error because it (incorrectly) thinks that is invalid json (and then json-schema just uses the original string).

So, my conclusion is that the way we're using json-gem is incorrect right now. But if we switch on quirks mode (which is what mutli_json and rails do) it will parse numbers correctly. So that's what I've done (as well as fixing the optional builds).

This doesn't fix the test failure, but at least all the builds are behaving in the same way. The real problem is in the common test suite (they should be using a string for that particular test, not a float). I'm going to fix the common test suite now, but the code in this PR should be correct.

[iainbeeston]

Incidentally, if we want to measure test coverage, we need the build to work with multi_json, because simplecov is dependent on multi_json (right now if we include simplecov in the gemspec or Gemfile then our tests fail, because of the bug described above)

[iainbeeston]

Actually, this is a mistake - we don't want to do this. The underlying problem is actually in the data the common test suite is using...

[iainbeeston]

Here's the pull request in the common test suite that should fix the build:

json-schema-org/JSON-Schema-Test-Suite#70

[pd]

Eek, I can't believe I didn't notice that half of the build matrix wasn't actually running tests any more. =\

The issue here is the same ambiguity we've run into elsewhere; if you're calling validate(schema, '127.0') and we attempt to parse that string, with quirks mode, it correctly gives us a float. The current is_a?(String) guard on IPFormat.validate means we don't even bother attempting to validate it as an IP address. We'd only get the original string back (which is what we want) if we called validate(schema, '"127.0"'), which is obviously pretty hacky.

TBH I don't know how we can possibly fix this while maintaining the (mis?)feature of allowing validate to accepts arbitrary strings which may or may not contain JSON values.

[iainbeeston]

Ok, bear with me - I have a fix (half finished)

[iainbeeston]

This is ready to merge now (pending review, of course).

I've added a :parse_data option to the validator, which (when set to false) can be used to skip any attempt at parsing the input or trying to work out if it's a json text or uri or whatever. This is actually really useful. For example, if you've already parsed your data, or it's a hash (not a string) you don't want to waste time trying to parse it. But the specific reason why it's useful here is that with the common test suite, we parse the whole test file, and then use small bits of it for validation. We don't want to re-parse those when they're loaded into json-schema, because they've already been parsed and parsing again (without the context) can change the meaning (e.g. "127.0" by itself is a floating point value, but if it's actually the value from "data": "127.0" then we know it's really a string).

I'll probably start using that when I use json-schema in rails, because often I want to validate the data in API requests. Rails parses those automatically on the way into the app (based on the mime type) so there's no need to re-parse them.

[iainbeeston]

I'm having a bit of trouble with the build for this, looks like older versions of json-gem segfault if you try to parse something that's not a string. Will try again later

[iainbeeston]

Travis was having DNS problems last night so I didn't get a chance to resolve the failures on all platforms. Will try again today some time

[pd]

I think we should generally start leaving these off in the code examples, it's just noisy. It's safe to assume that people will know to require the library they're using.

[iainbeeston]

Yeah that's a good point.

Let me raise an issue so we don't forget

[pd]

Two minor comments that totally aren't blockers, but huge 👍 in general. Literally every project I use json-schema on will swap over to using this option once this lands. =)

edit: the latest travis run had a segfault in rbx that I can't reproduce locally (even with the same seed); I've saved the log here, and triggered a re-run to see if it's a reliable error. It's worth noting that the test run passed just fine, the segfault was after the tests succeeded.

[iainbeeston]

@pd Thanks for the feedback. Sounds like I've addressed your two comments in one way or another. The discussion on shortening the readme has moved to #204

I think this pull request is good to merge, if anyone else wants to take a look?

[RST-J]

Looks good to me and I also like the new option 👍

[iainbeeston]

Thanks guys