Skip to content

Bump json-schema/JSON-Schema-Test-Suite to 1.2.0 + some tests improvements #265

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

Conversation

mirfilip
Copy link
Contributor

@mirfilip mirfilip commented May 9, 2016

Things done:

  • Simplify tests directory structure. Namespace is still unchanged.
  • Change drafts tests that we read from json schema test suite to be an associative array where a key is a file.json / suite description / test case description

Failing tests output before the change:

1) JsonSchema\Tests\Drafts\Draft4Test::testValidCases with data set #75 ('{"bar":1,"baz":2}', '{"properties":{"foo":{"not":{}}}}')

Failing tests output after the change:

1) JsonSchema\Tests\Drafts\Draft4Test::testValidCases with data set "not.json / forbidden property / property absent" ('{"bar":1,"baz":2}', '{"properties":{"foo":{"not":{}}}}')

It uses description fields from suite and test case. This may seem an overkill, but it really simplifies hunting for the test to fix when you have a readable path.

  • Update json-schema/JSON-Schema-Test-Suite to 1.2.0. Pay attention here, important:
    If we really want to state which version of the draft the validator follows, we need to compare against the newest available version. I presume everyone agrees ;) So, 1.2.0 bring three failure specs:
    • format.json / Draft 3 / optional
    • format.json / Draft 4 / optional
    • not.json / Draft 4 / required

Sad thing, there is no mechanism to skip one test case from not.json. For the time being, I've skipped the whole file as it's now my priority to fix that case as it's required by spec. Still, I think it doesn't hurt us as I doubt people are going to dig tests to see the compliance with spec (I'm working on documenting that).

/cc @bighappyface

@bighappyface
Copy link
Collaborator

+1

I looked into this in the past and stuffed it into my ever-growing to-do list... Thanks @mirfilip for bringing us up to the latest in terms of both the test site and a more sane PSR-4 directory structure.

@mirfilip
Copy link
Contributor Author

mirfilip commented May 9, 2016

@bighappyface Thx for the check. This will inevitably cause some PRs to need of rebase but I figured there are not too many PRs so it's a good time for such change. I'm thinking of doing the same for src, what do you think?

Also, do you want to have every PR squashed to a single commit?

@bighappyface
Copy link
Collaborator

@mirfilip I think squashed PRs are for the best. We need to add a CONTRIBUTING file to help clarify our expectations.

Yes, I say do the same for src while you are at it, and thanks ;)

description for failing tests. Flatten directory structure

Use "test.json / suite description / test case description" notation in data provider to allow a readable test output

Skip Draft3Test / Draft4Test tests which are not passing

Add some comment to skipped tests
@mirfilip mirfilip force-pushed the feature/upgrade-json-schema-test-sute-to-1.2.0 branch from 2e136d8 to 346b321 Compare May 9, 2016 21:43
@mirfilip
Copy link
Contributor Author

@bighappyface squashed. This is the final version. I will tackle src structure separately.

@jojo1981
Copy link

+1

@bighappyface
Copy link
Collaborator

+1

I'm going to let this one sit in master and bump at a later time.

@bighappyface bighappyface merged commit 6dad7bf into jsonrainbow:master May 11, 2016
@mirfilip mirfilip deleted the feature/upgrade-json-schema-test-sute-to-1.2.0 branch May 12, 2016 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants