-
-
Notifications
You must be signed in to change notification settings - Fork 205
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
beef up bignum tests #378
Comments
What are "bignum" tests testing? Is this a requirement of JSON Schema or of JSON compatibility? |
It tests support for arbitrary-precision decimals, as opposed to binary64 numbers (i.e. a double-precision float like ECMAScript uses). Presumably, the tests should be written so that if you're using a binary64 float to store numbers, there will be at least one failing test for each schema. It technically is a requirement (JSON Schema is designed to test the document itself, not how it's stored in memory), but the vast majority of implementations and uses get by just fine with nothing more than a binary64 float, so for practical reasons, tests of arbitrary precision goes in optional/. |
Support for arbitrary-precision numbers is required by JSON, not JSON Schema. Testing this is like testing for arbitrary string length. It's an inherited requirement of working with JSON rather than something the spec states. |
It would be reasonable to put some tests of bignums into the optional/ directory, surely? At the least, it makes an implementor aware of these cases and they can decide whether to support them or not. |
@gregsdennis Yes. That supports the need for this test, doesn't it? @karenetheridge Yes, the test file is already found in optional/bignum.json |
No... I don't test the ability to add numbers with the This test suite is supposed to test JSON Schema. Anything that isn't supported because of the decisions of the way JSON is implemented is out of the Schema library's control (assuming they're separate libraries). If you want to start a JSON test suite, this is a perfect test for that. However, JSON recognizes that support for arbitrary-precision numbers is limited to the environment and framework on which it's running. JSON Schema also inherits that caveat. At best, this needs to be an optional test, but I don't think it belong because it's not explicitly testing a requirement of JSON Schema. |
I support adding more tests to optional/bignum.json. Otherwise, it could be very easy for an implementor to overlook that these values exist in the JSON data model, and since many (most?) JSON decoders would treat these values differently, the need for JSON Schema to treat these as if they were any other number necessitates special treatment in the JSON Schema implementation as well. |
@gregsdennis I'm not sure I follow your argument then... JSON Schema normatively includes JSON, and is defined to parse JSON documents (i.e. When we have a test like
What do you think this is doing? It's testing that the JSON document is valid against the schema. Same thing for numbers:
and even very large numbers:
And I believe we've established, if there's a reason that more than a few implementations might not be compliant because of coding environment, we can add tests for that. For example, we have tests for empty keys in objects (since some environments want minimum one character in a key) and regular expressions (since some implementations don't support the same dialect, or unicode). Also, while JSON describes the format of numbers, it doesn't proscribe how the value must be made available to applications. JSON Schema does. |
The examples you showed aren't testing the JSON, per se. They're testing the schema's ability to match and compare JSON values assuming the implementation handles it. Precise numeric values is something that JSON concedes is a limitation of implementation, so I'm not sure that it's JSON Schema's responsibility to test for it. You'd want a JSON Test Suite to do that. Empty keys would be the same: it's a limitation of the JSON support rather than of the JSON Schema support. It's a fine line, but it's still there. Regex support, on the other hand, is something we should definitely test, since JSON just sees a string.
I'm not sure what you mean here. I don't know. If these are optional anyway, fine. It just seems weird to me to test the underlying technology rather than the functionality we've defined. |
The "bignum" tests don't appear to be testing much. Most of them are missing boundary tests, and tests on either side of the boundary.
For example, the test for "exclusiveMinimum" should include a test above, below, and exactly equal to the argument.
Tests also be added for "enum" and "const" where applicable.
The text was updated successfully, but these errors were encountered: