Proposal: use test suite structure

[joshmoore]

Alternate proposal to #116 described under #116 (comment) to make use of the test suite structure from https://github.com/json-schema-org/JSON-Schema-Test-Suite#structure-of-a-test

sciprt used to combine the JSON

#!/usr/bin/env python

import glob
import json

template = """{{
    "description": "TBD",
    "schema": {{
        "id": "TBD"
    }},
    "tests": [
{tests}
    ]
}}
"""

test = """
  {{
    "formerly": "{filename}",
    "description": "TBD",
    "data": {data},
    "valid": "TBD"
  }}"""


tests = []
for prefix in ("valid*/**", "invalid*/**"):
    for filename in glob.glob(prefix):
        with open(filename) as o:
            try:
                data = json.load(o)
                data = json.dumps(data, indent=4)
            except:
                print(f"Error on {filename}")
                raise
        tests.append(test.format(filename=filename, data=data))

tests = ",\n".join(tests)
output = template.format(tests=tests)
print(json.dumps(
    json.loads(output),
    indent=2))

Here we are not really using the schema field correctly, and perhaps we agree to use:

"schemas": [ "schema/file_1", "schema/file_2"]

[sbesson]

In general, the proposal here closely matches what is described in #116 (comment).

Thanks for pushing a first draft of its implementation. Overall, I am more convinced that it is a valuable step forward as we try to increase the coverage of our entire specification with validation schemas.

A few inline questions which are more RFEs about using more of the upstream test-schema.json features.

Apart from fixing the CI, I assume the main remaining thing is to port the invalid tests and consuming the valid key in the test generation? And possibly apply the same changes to 0.4 in a separate PR?

[sbesson]

I can't remember why we used the 202012 implementation. Is the goal here to have use the common-denominator? In that case would the version-agnostic Validator API be an option?

[joshmoore]

My local version no longer had the dated Draft so I went with the latest. @will-moore may be able to say more.

[will-moore]

Sorry, don't remember

[sbesson]

Ah I seem to have introduced it in 9602cb4. Looking at the failing tests, I think I remember why.

The JSON schema makes use of the minContains/maxContains features which were introduced in recent version of the schema (2019-09) so we'll need to use a matching version of the validator to test the examples.

[sbesson]

Possibly points to using the id key - see https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/master/test-schema.json#L61 ?

[joshmoore]

Yes, definitely happy to move away from these for the proper suite ones. For examples, we'll still need to generate.

[joshmoore]

I assume the main remaining thing is to port the invalid tests and consuming the valid key in the test generation?

Is that not what https://github.com/ome/ngff/pull/118/files#diff-55735767b6ee79dde0d4c0f1f3d88dced97cbeafb3bffc4ed05d514087a3e416R57 is doing?

[sbesson]

Is that not what https://github.com/ome/ngff/pull/118/files#diff-55735767b6ee79dde0d4c0f1f3d88dced97cbeafb3bffc4ed05d514087a3e416R57 is doing?

Ah you're right that both valid and invalid data are tested in image_suite.json. I got confused because the tests define in latest/tests/strict_image_suite.json has "valid": "TBD". I assume this gets intepreted as true?

[joshmoore]

I assume this gets intepreted as true?

Likely unintentionally. Sorry, that as a copy-n-paste error when I split the files. Now fixed.

[sbesson]

Great, this leave one remaining failed test duplicate_axes_name. This is a tricky one as it the JSON is invalid as per the specification but this belongs to the class of data invalidities that the JSON schema is simply not suited to detect. Possible we just remove this test case to avoid the confusion?

[joshmoore]

Looks like we're down to just the one "invalid_but_dont_fail" test:

self = Suite(schema={'$schema': 'https://json-schema.org/draft/2020-12/schema', '$id': 'https://ngff.openmicroscopy.org/lates...: '0', 'coordinateTransformations': [{'scale': [0.13, 0.13], 'type': 'scale'}]}], 'version': '0.5-dev'}]}, valid=False)
validator = Draft202012Validator(schema={'$defs': {'axes': {'contains': {'properties': {'name': {'type': 'string'}, 'type': {'enum...ma', '$schema': 'https://json...020-12/schema', 'description': 'JSON from OME-NGFF .zattrs', ...}, format_checker=None)

    def validate(self, validator) -> None:
        if not self.valid:
            with pytest.raises(ValidationError):
>               validator.validate(self.data)
E               Failed: DID NOT RAISE <class 'jsonschema.exceptions.ValidationError'>

@will-moore, what would you expect to do here? set valid = true?

[will-moore]

Agree with @seb. Probably best to simply remove the duplicate_axes_name example. It's not an example of valid JSON, and it also can't be used to test the schema, so I think it's confusing to keep it anywhere.

[joshmoore]

Thanks, both. That gets us back to green. Other then potentially refactoring some of the code to the top-level to be re-used between versions, I think I'm happy with this as a first round of refactoring.

[sbesson]

All good from my side. Happy to handle the conversion of the 0.4 tests as a follow-up