Port the Test Suite to JSON about jsonschema HOT 40 CLOSED

I laid out the idea here yesterday on a post on the JSON Schema mailing list.

I do think this is important, so I think we should prioritize this, so it's nice if you're in for starting to work on it :).

If you'd like to jump right in, fork https://github.com/json-schema/json-schema/tree/gh-pages and we can get started merging our test suite with @fge's and convert the tests over. Send over the link so that if I get some time later I can do some myself too.

As I said in the post, I think there should be 4 properties per object: description, schema, data, valid (so no more parametrization, just separate JSON objects for each one since who cares at this point about duplication, the clarity is worth it). We'd have those objects sitting inside an array, and those arrays inside a file for each schema validator (type.json, additionalProperties.json ...) and then those inside a folder for each draft (so draft3 folder and draft4 folder then with duplication).

Just as an illustrative complete example:

[
{
    "description": "1 is an integer", 
    "schema": {"type": "integer"},
    "data": 1, 
    "valid": true, 
}
]

Also as per the post, fge has some of this done for us already in https://github.com/fge/json-schema-validator/tree/master/src/test/resources/keyword so we can work off that.

Also I really hope GitHub isn't being dumb, since yesterday I got 6 emails for the same comment from someone as they were editing it, and I just realized that I've edited this comment about 15 times.

from jsonschema.

Ahh good, looks like there is considerable interest there in this idea. I'll hold off a bit to see what the consensus is.

from jsonschema.

The consensus will precisely require a maximum of input, so there is no restraining oneself in this matter ;)

As to the example given, I believe it should be expanded so as to test several values for one schema, including "marginal" values. However, this is not easy when it comes to numeric instances. For example, I have to validate this kind of instance (example is simplified):

1.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001

against:

{
    "maximum": 1.000000000000000000000000000000000000000000000000000000000000000000000000001,
    "exclusiveMaximum": true
}

Since this was one of my core requirements, I do validate that kind of stuff correctly, but I expect most implementations out there will not. Should such an implementation be considered invalid? No. After all, the JSON RFC itself states that JSON parsers may impose limits on allowed numeric instances, I just happened to choose one which had no (visible) limits in this regard.

from jsonschema.

Well. The reason I said "no parameterization" is that I think it'd be slightly easier for validator implementors to just have one JSON object per test method, and additionally that adding extra examples (presuming they aren't testing additional behavior) is noisy, so if there actually is different things under test then they'd deserve their own description (like your additional example, which is clearly testing something other than "can validate maximum", say).

As for what to do about implementation-defined behavior, we can provide some additional tests in their own section that could be deemed optional I guess.

from jsonschema.

I think paramerterization is still a good idea, as there are going to be at least 2 tests for any given schema, test valid, and test invalid. I think for clarity, we could easily move the description down to the test level:

[
{
    "description": "integer type",
    "schema": {"type": "integer"},
    "tests": [
        {
            "data": 1,
            "valid": true,
            "description": "test 1 is an integer"
        },
        {
            "data": 1.1,
            "valid": false,
            "description": "test 1.1 is not an integer"
        }
    ]
}
]

from jsonschema.

Well, presumably there will be as many as 6 or 7 tests there for testing integer passes for integers and fails for the other types, the question though is why is having them grouped there better than having 6 or 7 individual tests with those descriptions?

from jsonschema.

I suppose avoiding duplication is the answer there, I guess I just don't see what the advantage of listing out the schema to be tested upon multiple times is.

from jsonschema.

OK. I guess that's reasonable. Mostly I find that duplication is bad when you have something that is going to be updated and maintained, but these tests would be pretty much static, and that therefore there's added benefit in duplication because writing test methods now just involves a single loop (or two loops, one over files, one over test objects). Whereas with nesting / parametrization, now you have 3, and additionally now if you want to use the descriptions to produce pretty test failure messages like xUnit can, you may now need to combine a top level description with the current parametrization's message, or otherwise put things together. I don't feel too strongly though so if you disagree I wouldn't try to convince you more than that :).

from jsonschema.

I could go either way. The added complexity of a test runner for these in python seems trivial, haven't thought about other languages myself.

EDIT: In terms of maintenance, the combined approach may be easier to update if later drafts of the schema change how the schema to validate a given instance should be written. For example {"type": "any"} being replaced by {}, or the required change.

from jsonschema.

I was looking on the jsonschema google group about common ways to test specs and I found this thread.

I'd go with an array of hashes, in my opinion it will make the test suite more readable to have a test checking one assertion without having to remember or read the common context at the start.

Here's a stub for maximum property using Julian's approach:

https://gist.github.com/3779919

and this one is the terser version:

https://gist.github.com/3779950

I think having this kind of duplication in the tests is not a bad thing.

Often in Rspec I use the second syntax when I want to share a context between tests, but in this case the JSON syntax seems more readable using the first approach.

Just my 2 cents :)

from jsonschema.

The added complexity of a test runner for these in python seems trivial, haven't thought about other languages myself.

In Java, it's really trivial if you use TestNG since it has a very nice stuff called data providers:

https://github.com/fge/json-schema-validator/blob/master/src/test/java/org/eel/kitchen/jsonschema/keyword/AbstractKeywordValidatorTest.java

All my keyword tests use that single file, they just inherit that class. And all tests are multithreaded to boot.

from jsonschema.

OK, I think the easiest thing rather than working directly inside the JSON Schema website's repo is to create another one, and then when the suite is done to some small extent either add it as a submodule with an HTML page we can write up, or to copy it in.

Right now, I've created the repository. If you'd (@fge) prefer it be on the json-schema account (or if you'd allow it to be :) that's cool let me know and we can move it.

Otherwise, it's here. I'll post it on the ML too in case anyone wants to help out.

from jsonschema.

I just did a quick dump of fge's test suite over to the new format, needs some manual editing before it's ready though. https://github.com/gazpachoking/JSON-Schema-Test-Suite/tree/messy_dump/tests/draft3

from jsonschema.

Beware that you really swallowed it all, and some tests may fail because of parser limitations (for instance, divisibleBy with very huge number). I do test these correctly because unlimited numeric instance scale/precision was one of my requirements; not so with other implementations!

This kind of tests should be avoided for general use, I think. An "anal retentive" suite could be created for those ;)

from jsonschema.

Yeah, I'm going to go through each of them manually and clean some stuff up and add descriptions. I'll keep an eye out for those sorts of tests. ;)

from jsonschema.

I'm adding the tests as I clean them up to this branch: https://github.com/gazpachoking/JSON-Schema-Test-Suite/tree/clean/tests/draft3

Also, turns out I'm horrible at coming up with descriptions for the tests. :P

from jsonschema.

Yeah. It's harder to come up with descriptions with the nesting. Oh well. We can do as best we can.

I'm done until... tomorrow night I think, so no need to worry about stepping on each other's toes until then.

from jsonschema.

Do you think it would be better if we ditch the top level description and just have one for each test?

from jsonschema.

Sounds good to me.

from jsonschema.

I made a branch with a test runner for these, let me know what you think of the method. https://github.com/gazpachoking/jsonschema/blob/json-test-suite/tests.py#L30

from jsonschema.

Hmm, seems nose wants to run my helper functions as tests, I'll have to fix that.

from jsonschema.

This looks pretty good! I don't care too much about which method as long as it meets those characteristics, if we come up with something cleaner later we can always update it.

Only things that stick out are that you can ditch those try/except/raise failureException blocks – they're just going to swallow tracebacks that are useful.

Just use validate(thing) for the case where it should be valid and with assertRaises(ValidationError): validate(thing) for the case it isn't.

Also, we should probably just do cls(schema).validate(instance), since validate does meta-validation which shouldn't be needed, we can just do the validation itself.

Oh – also, the function you wrote can be used as a class decorator, which I kind of like – if you do, perhaps have it be
something like

@load_json_tests(the_dir)
class TestDraft3Validation(TestCase):
    validator = Drat3Validator

from jsonschema.

I was using just validate() method and assertRaises, I changed it in order to print out the test description in the error. I can remove that if you think it's better.

from jsonschema.

Ah I see. Yeah we do want that in there I guess.

Are the tests just being numbered? Perhaps we should use the test description as the test method name instead?

from jsonschema.

I had the file name as the test name with a number after. e.g. test_additionalProperties_0 Description could be used instead though.

from jsonschema.

Okay, made some changes. Nose doesn't run helpers anymore, works in python 3, implemented your class decorator idea, moved description down to the method name instead of having it in asserts. Test method names are now like uniqueItems: objects are deep compared

from jsonschema.

Also, found some test failures on python 3 now that those work, seems the unique item checking is having problems there.

from jsonschema.

Bugs or problems with the JSON test suite? I expect a bunch of the latter.

from jsonschema.

Oh, also – think the test methods should be valid identifiers, and ones that are discoverable on their own, so, I'd prefer test_uniqueItems_objects_are_deep_compared.

from jsonschema.

At the moment I think bugs. Everything passes fine on python 2.6 and 2.7. We get a non-unique elements SchemaError for this schema https://github.com/gazpachoking/jsonschema/blob/json-test-suite/tests/draft3/enum.json#L20 on 3.2. Didn't look into the actual cause yet.

from jsonschema.

I may have just figured it out. I bet it's because 1 == True

EDIT: Don't know why it only shows up on python 3 yet though.

from jsonschema.

Yeah, I was actually a bit surprised it even allowed me to make methods that weren't valid identifier names. I guess it worked and they got discovered because they all have the __name__ attribute test_case.

from jsonschema.

Okay, updated again. Test method names are now valid identifiers, and I'm setting their __name__ again.

from jsonschema.

As for the test failures, I think _uniq is wrong, for multiple reasons. For one, 1 == True, but those two elements should be unique. Secondly, the second implementation with islice should be using the sorted list sort rather than the raw container.

EDIT: Moved this over to a new issue.

from jsonschema.

I think the test runner is solid now, any other issues?

from jsonschema.

Don't think so, looks good. I may move the tests into a package later when
I get a chance to take a closer look, but I added you as a collaborator,
feel free to merge this and the test runner.

On Sunday, October 28, 2012, Chase Sterling wrote:

I think the test runner is solid now, any other issues?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-9855365.

from jsonschema.

The only problem with the test runner is that the tests won't pass right now due to the bool/int issue.

from jsonschema.

Special case that one by checking the description (ech, but temporary) and
mark it as unittest.expectedFailure.

On Sunday, October 28, 2012, Chase Sterling wrote:

The only problem with the test runner is that the tests won't pass right
now due to the bool/int issue.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-9855429.

from jsonschema.

Done.

from jsonschema.

Awesome. Thanks :)
On Oct 28, 2012 11:42 PM, "Chase Sterling" [email protected] wrote:

Done.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-9855685.

from jsonschema.