-
Notifications
You must be signed in to change notification settings - Fork 153
Tests and setup for 1.1 development #283
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
* Included new JSON-LD tests in json-ld.org directory (perhaps rename this to something more W3C specific, as these are the latest tests from W3C) * Starting from `manifest.jsonld` rather than all jsonld files in the test directory; then iterating of the the list of manifest files specified there.
- Set up tests for json-ld-api, json-ld-framing, and json-ld-1.0 - Run only json-ld-1.0 suite in Maven build (by naming convention) - Run all suites for development of 1.1 features (AllSuites.java)
- Skip only failing tests to make sure we run 1.1 code paths in CI - Use resolvable URLs in skip files, they can act like TODO lists - Delete `*-skip` files to re-generate them during next test run Now `mvn test` runs all tests that are not listed in the files.
Run tests with `-Dreport.format=jsonld`, convert to ttl, then: https://github.com/w3c/json-ld-api/blob/master/reports/README.md
For integration in https://w3c.github.io/json-ld-api/reports/ convert api and framing jsonld reports to single ttl file and then: https://github.com/w3c/json-ld-api/blob/master/reports/README.md
- be more verbose in some error messages - add some more comments pointing to the specs - add a blank node condition according to spec 16.4
Focusing on expansion and context/term processing
| @@ -0,0 +1,66 @@ | |||
| package com.github.jsonldjava.specs; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is almost identical to the JsonLd1Tests. Can the code be reused ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at this, but I think avoiding the redundancy here would lead to too much complexity. First, the actual code is already extracted in static methods of SuiteUtils. The redundant part is only the JUnit Parameters/BeforeClass/AfterClass setup. These are static methods, so simply extracting them to a superclass won't work. An option would be a custom JUnit Rule (see https://www.baeldung.com/junit-4-rules#defining-a-custom-junit-rule), but I'm not sure if/how that would work with Parameterized, and I think it might actually make the code less readable.
| @@ -0,0 +1,66 @@ | |||
| package com.github.jsonldjava.specs; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| @@ -0,0 +1,106 @@ | |||
| # Introduction | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, this is a copy of https://github.com/w3c/json-ld-api/blob/master/tests/README.md.
Will add a remark in a commit - I think it is helpful to know the source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is indeed helpful to know the source. However, the workflow for updating the tests (as documented in the README, 'Update local spec tests') involves copying json-ld-api/tests/* to json-ld-api-tests/, which replaces the README. Maybe we could add a separate README to src/test/resources, describing the origins of the spec tests?
|
Rather than pulling in partially implemented code to |
* Don't minimize guava shading, causes NoClassDefFoundError
Fix jarcache guava loading
Current state of 1.1 development
Complements the last commit.
The ignored test would test an expected behaviour of java.net.URL.HttpURLConnection, i.e. not to automatically follow redirects when this involves a protocol switching (e.g. from HTTP to HTTPS). See #289.
- remove unused imports - sorted alphabetically
As proposed in #292 (comment)
This avoids a possible endless loop. See #292 (comment).
It should be checked if an Entity has a contentType, not if an Entity has content. See #292 (comment) and #292 (comment).
- remove superfluous check (was always false) - remove superfluous IOException - remove logger as it is no more needed See #292 (comment).
Signed-off-by: Peter Ansell <p_ansell@yahoo.com>
Signed-off-by: Peter Ansell <p_ansell@yahoo.com>
Signed-off-by: Peter Ansell <p_ansell@yahoo.com>
Signed-off-by: Peter Ansell <p_ansell@yahoo.com>
|
I've updated this with the current master, making the tests pass. There is some confusion here however, as this now contains what is described in #284 (current state of 1.1 implementation) plus what is described here (just the test setup). I shouldn't have opened #284 with a base branch that I didn't intend to be merged into, sorry. But this might be obsolete anyways (see #284 (comment)). |
As announced in #277 (comment), this PR contains only the changes in the test setup for 1.1:
See README for updating, running, and report generation:
https://github.com/jsonld-java/jsonld-java/blob/1.1-tests/README.md#update-local-spec-tests
This is the report summary for 1.1 compliance w/o any 1.1 specific code (see 2318376):
I will open a second pull request with our current 1.1 implementation changes from #277.