Add exhaustive jqwik schema-document integration test #47
Conversation
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
1 similar comment
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive property-based integration test for JSON schema validation using jqwik's exhaustive generation mode. The test systematically validates schema compilation, compliant document generation, and negative test case creation to ensure robust schema validation behavior.
Key changes:
- Implements exhaustive property-based testing with sealed schema types and document builders
- Creates negative test cases by systematically breaking compliant documents
- Adds custom jqwik ArbitraryProvider for comprehensive schema generation coverage
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| schemaMap.put("minItems", JsonNumber.of((long) items.size())); | ||
| schemaMap.put("maxItems", JsonNumber.of((long) items.size())); |
There was a problem hiding this comment.
The cast to long is repeated for both minItems and maxItems. Consider extracting (long) items.size() into a local variable to reduce duplication and improve readability.
| schemaMap.put("minItems", JsonNumber.of((long) items.size())); | |
| schemaMap.put("maxItems", JsonNumber.of((long) items.size())); | |
| long itemsSize = (long) items.size(); | |
| schemaMap.put("minItems", JsonNumber.of(itemsSize)); | |
| schemaMap.put("maxItems", JsonNumber.of(itemsSize)); |
| ).as((names, first, second) -> new ObjectSchema(List.of( | ||
| new Property(names.getFirst(), first), | ||
| new Property(names.getLast(), second) | ||
| ))); |
There was a problem hiding this comment.
Using getFirst() and getLast() assumes the PROPERTY_PAIRS list always contains exactly 2 elements. This creates a fragile dependency on the list structure. Consider using indexed access names.get(0) and names.get(1) or add validation to ensure the list size is exactly 2.
| ).as((names, first, second) -> new ObjectSchema(List.of( | |
| new Property(names.getFirst(), first), | |
| new Property(names.getLast(), second) | |
| ))); | |
| ).as((names, first, second) -> { | |
| if (names.size() != 2) { | |
| throw new IllegalArgumentException("Expected names list of size 2, got " + names.size()); | |
| } | |
| return new ObjectSchema(List.of( | |
| new Property(names.get(0), first), | |
| new Property(names.get(1), second) | |
| )); | |
| }); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| sealed interface JsonSchema permits ObjectSchema, ArraySchema, StringSchema, NumberSchema, BooleanSchema, NullSchema { | ||
| record ObjectSchema(List<Property> properties) implements JsonSchema { | ||
| ObjectSchema { | ||
| properties = List.copyOf(properties); | ||
| } | ||
| } | ||
|
|
||
| record Property(String name, JsonSchema schema) { | ||
| Property { | ||
| name = Objects.requireNonNull(name); | ||
| schema = Objects.requireNonNull(schema); | ||
| } | ||
| } | ||
|
|
||
| record ArraySchema(List<JsonSchema> items) implements JsonSchema { | ||
| ArraySchema { | ||
| items = List.copyOf(items); | ||
| } | ||
| } | ||
|
|
||
| record StringSchema() implements JsonSchema {} | ||
|
|
||
| record NumberSchema() implements JsonSchema {} | ||
|
|
||
| record BooleanSchema() implements JsonSchema {} | ||
|
|
||
| record NullSchema() implements JsonSchema {} | ||
| } |
There was a problem hiding this comment.
[P0] Nested schema records declared inside interface do not compile
The helper schema types are declared as records inside the nested JsonSchema interface, but the rest of this test refers to them as if they were top‑level members (new ObjectSchema(...), pattern matching, permits ObjectSchema). Because nested interface members must be referenced with their enclosing type, ObjectSchema and friends need to be named JsonSchema.ObjectSchema (and the permits clause must use the same qualified names). As written the file does not compile, so the test suite cannot run.
Useful? React with 👍 / 👎.
Summary
Testing
./mvnw -pl json-java21-schema verify -Djava.util.logging.ConsoleHandler.level=INFO(fails: network unreachable while downloading maven-source-plugin)https://chatgpt.com/codex/tasks/task_b_68d07992607083279c1bf53528bcf4a1