Improved extends and $ref support, added json-schema-test-suite test-cases #39
Conversation
…DO: nested refs TODO: Failures: 25, Errors: 16
|
Could you rebase this against the current master? |
|
Hello. Please take a look at #41, which was merged into master, before you rebase so you'll have a good idea and understanding of all the changes that were included in that Pull Request. Thanks! |
|
rebased already. had some conflicts. seems we had done equal fixes for the same issues ;) |
|
Hello. I fixed all of the draft-03 bugs and issues with the exception of things related to $ref, $schema, and id. So #41 doesn't include any changes to address that functionality. Not all ref tests, remote ref tests, etc. work correctly. If you look at the commits in #41, you'll see what I fixed. There is some overlap with things that you have worked on. The entire test suite that's included with #41 should pass. The tests that I added were based on tests from JSON-Schema-Test-Suite, which you have been working with. I noticed that your work seemed to focus on $ref, etc. so my work focused on fixing all other draft-03 deficiencies. Note that with #41, the entire draft-03 test suite should pass, excluding ref.json, refRemote.json, and a few of the optional tests. Let me know if you have any questions about #41. It significantly improves the json-schema project's functionality with respect to supporting draft-03. Thanks! |
|
looks good. my fork seems to be obsolete. my postings on json-schema-test-suite might me misleading. I didn't work on $ref here. At the beginning I thought I could solve some businescases with $ref but later I found out 'extends' is what I was looking for. So my main focus was 'extends' - and the errors concerning the test-suite as well. edit: to be honest, its been almost two months ago I worked on the pull request here. lost the details. Did you include all test-cases from the test-suite? |
|
Hello again. Note that #41 also fixes |
|
yeah, it's a pity my pull request has been ignored for so long and you refixed the things i have already done. i used git submodule to easily staying up to date if there are some changes in the test-suite. Installing the test-suite via composer as dev-dependency with - if you like - a fixed version would be the way to go in my opinion - explicit, easy, isolatable, safe. Question: did you include the remote tests? |
|
Hello. I think the biggest reason why your pull request was never merged is because it was never fully complete. Your master never passed the build due to the large number of failures and errors in the test suite that you were trying to add. If it were me, I would never merge from a pull request that didn't have a green build. My pull request had only two commits that weren't green. Those were due to Scrutinizer errors, not test failures. I also fixed and/or added a lot more draft-03 functionality in my request than you did in yours. Plus I included a bunch of new unit tests. Look over the commits in #41 to see what I did that was beyond what you did. Using a Git submodule can keep you the up-to-date with the latest in the test suite repository. However, failures are possible, and not just because functionality is broken. I filed a bug against one of the tests in the suite, which would have caused an erroneous failure not due to broken functionality, but because of a faulty test. So I don't think using a Git submodule is the best way to go even with a dependency on a fixed version, unless that fixed version is known to be free of defects. The test suite only has one tag, 1.0.0, and that's pretty old. As I mentioned, all of the JSON Schema validation implementations that I've seen incorporate the test suite into their projects by copying the content of the tests. None of them use a Git submodule to pull in the suite. So there is isolation from the test suite project and any potential breakage. I've copied the draft-03 suite into my local repository. I have all of the .json files and I've written a small amount of code to read those files and execute the tests. The entire draft-03 test suite passes with the exception of a few test files that I've marked as skipped (e.g. ref.json, refRemote.json, and three .json files in the optional folder.) As for your pull request, since #41 has rendered it largely obsolete and unnecessary, I would simply close this request and not merge it. You will have lost some effort, but since you said that you haven't actively worked on this pull request for two months, I think closing without merging is probably the best way to go. Hope that's OK. |
|
Ok, you skipped the remote tests. |
* Improve performance - don't loop over everything if already valid * Don't coerce already-valid types (fixes #379) * Add remaining coercion cases & rewrite tests * Add all remaining coercion cases from ajv matrix * Rewrite the coercion tests to tidy things up a bit * Add CHECK_MODE_EARLY_COERCE If set, falls back to the old behavior of coercing to the first compatible type, regardless of whether another already-valid type might be available. * Add multiple-type test that requires coercion * \JSON_PRETTY_PRINT doesn't exist in PHP-5.3, so work around this * Various PR cleanup stuff * Fix whitespace * Turn $early into $extraFlags * Change "string" to "ABC" in string test * Update README.md description of CHECK_MODE_EARLY_COERCE * Move loop after complex tests definition * Move test #39 to grid #15
* Improve performance - don't loop over everything if already valid * Don't coerce already-valid types (fixes jsonrainbow#379) * Add remaining coercion cases & rewrite tests * Add all remaining coercion cases from ajv matrix * Rewrite the coercion tests to tidy things up a bit * Add CHECK_MODE_EARLY_COERCE If set, falls back to the old behavior of coercing to the first compatible type, regardless of whether another already-valid type might be available. * Add multiple-type test that requires coercion * \JSON_PRETTY_PRINT doesn't exist in PHP-5.3, so work around this * Various PR cleanup stuff * Fix whitespace * Turn $early into $extraFlags * Change "string" to "ABC" in string test * Update README.md description of CHECK_MODE_EARLY_COERCE * Move loop after complex tests definition * Move test jsonrainbow#39 to grid jsonrainbow#15
I integrated the json-schema-test-suite test cases.
Fixed about 20 tests but still some fail.
For its remote schema tests the testsuite needs a small http server running on http://localhost:1234
Please see README.md for how to install and run.
If you don't want the json-schema-test-suite tests to run, simply comment out the code in
test/SuiteTest.php.