Reviewing merge request #2: fixed unit tests and other minor changes

We did a fresh clone and ran the tests (mvn test) and there were failures. The failures were due to the fact that the JSONSchemaTest is pulling down a schema from http://json-schema.org/card and validating against data that does not pass. I'm assuming that these passed at one point and the schema at the URL changed and now the test fails (kind of a brittle test IMHO). The fix was to update the data.

There was another issue with schema validations pulled in from a $ref. That was working correctly. It was just populating a NoOpValidator in the list of validators. I fixed that as well.

Also, we plan on using this library in a multi-classloader environment (OSGi) and so I replaced the Class.forName() class loading with a getClass().getClassLoader().loadClass(). Which, in a single classloading environment will be a non-functional change.

Hope you merge in these changes. We really appreciate the work you've done on this project.

Commits that would be merged:

Version 1
  • Version 1
  • 12ab6ff
  • d3f44de

    • tests didn't pass - fixed the parsing of the ref property and added correct data to card.json to make the tests pass

  • 6eee76d

    • made the loading of the actual validator classes be a bit more friendly for multi-classloader environments

  • 958ee83

    • Merge branch 'unit_test_fixes'

Showing 12ab6ff-d3f44de

Comments

Pushed new version 1

avatar mattasync | 2011-07-12 15:31:42 UTC

I'm getting test failures even with these changes – it appears that the card schema at http://json-schema.org/card has some extra commas which confuse the parser. Also, after shifting that to a local schema, it’s no longer producing an error that longitude is required – is this a change in the schema?

avatar mjewell | 2012-01-05 12:13:16 UTC

@mjewell this project hasn’t been committed to since June 02 2010. Our merge request has gone unanswered for the past 5 months. It’s a dead project. I'd recommend forking and taking it in your own direction. Good luck.

avatar mattasync | 2012-01-05 13:52:39 UTC

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository