Skip to content

Conversation

@simbo1905
Copy link
Owner

@simbo1905 simbo1905 commented Sep 28, 2025

Bug fixes and jQwik test

INFO: Edge case generation exceeds number of tries. Stopped after 1000 generated cases.
timestamp = 2025-09-28T17:14:36.545155, JtdPropertyTest:exhaustiveJtdValidation =
|-----------------------jqwik-----------------------
tries = 1000 | # of calls to property
checks = 1000 | # of not rejected calls
generation = RANDOMIZED | parameters are randomly generated
after-failure = SAMPLE_FIRST | try previously failed sample, then previous seed
when-fixed-seed = ALLOW | fixing the random seed is allowed
edge-cases#mode = MIXIN | edge cases are mixed in
edge-cases#total = 1000 | # of all combined edge cases
edge-cases#tried = 157 | # of edge cases tried in current run
seed = -1152848530146296972 | random seed to reproduce generated values

Process finished with exit code 0

closes #88

- Added BigDecimal fractional part checking in validateInteger method
- Added test case testInt32RejectsDecimal() to verify the fix
- Ensures all integer types (int8, uint8, int16, uint16, int32, uint32) reject decimal values
- Maintains RFC 8927 compliance for integer type validation
- Fixed JTD validator to correctly default additionalProperties to false when no properties are defined
- Added test case testAdditionalPropertiesDefaultsToFalse() to verify the fix
- Updated CI test count from 463 to 464 to account for new test
- This ensures RFC 8927 compliance where empty properties schemas reject additional properties by default

The bug was in Jtd.java line 446 where additionalProperties was set to true instead of false
when both properties and optionalProperties were empty. This caused empty schemas to incorrectly
allow additional properties instead of rejecting them by default.

Closes #91
- Removed JtdExhaustiveTest.java from this PR to separate property test development from the bug fix
- Backed up the property test as JtdExhaustiveTest.java.backup for future development
- Updated CI test count from 464 back to 463 to reflect removal of property test
- This allows the additionalProperties bug fix (Issue #91) to be merged independently
- The property test can be restored and continued separately after merge
- Added special-case handling in pushChildFrames to skip pushing variant schema
  when discriminator object contains only the discriminator key
- This fixes validation failures when discriminator maps to simple types like boolean
- Preserves RFC 8927 semantics while handling property-test conventions
- Test case testDiscriminatorInElementsSchema now passes

The fix addresses the bug where discriminator objects like {alpha:type1}
were incorrectly validated against simple type schemas like {type:boolean},
causing "expected boolean, got JsonObjectImpl" errors.
- Updated CI test count from 464 to 465 to reflect new discriminator test
- Added instructions for creating PRs with GitHub CLI to AGENTS.md
- Includes guidance on avoiding special characters in titles and using body-file flag
- Added testNestedElementsPropertiesRejectsAdditionalProperties() to TestRfc8927
- Tests nested elements containing properties schemas with empty properties
- Verifies that additional properties are correctly rejected by default
- Reproduces the exact failing case found by JtdExhaustiveTest property testing
- Schema: {elements:{elements:{properties:{}}}}
- Document: [[{},{},[{},{extraProperty:extra-value}]]
@simbo1905 simbo1905 requested a review from Copilot September 28, 2025 15:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements exhaustive property-based testing for the JTD validator, focusing on fixing RFC 8927 compliance issues with empty schema semantics. The changes address discriminator validation bugs and ensure the empty schema {} correctly accepts all JSON instances per RFC 8927 specification.

  • Fixes discriminator field validation issues where the discriminator property was being double-validated
  • Implements proper RFC 8927 empty schema semantics throughout the codebase
  • Adds comprehensive test coverage for edge cases discovered through property-based testing

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TestRfc8927Compliance.java Updates test comment and data to reflect RFC 8927 empty schema semantics
TestRfc8927.java Extensive new test cases for discriminator validation, empty schema behavior, and RFC 8927 compliance
JtdSpecIT.java Minor comment cleanup removing redundant description
JtdExhaustiveTest.java Major fixes to discriminator value generation and property validation logic
DocumentationAJvTests.java.backup Complete removal of backup test file
DocumentationAJvTests.java Refactored empty schema tests to demonstrate RFC 8927 compliance
Jtd.java Core validator fixes for discriminator field handling and empty schema compilation
ARCHITECTURE.md Added documentation for empty schema behavior per RFC 8927
README.md Added user-facing documentation explaining RFC 8927 empty schema semantics
AGENTS.md Added development guidelines for RFC 8927 compliance and MDD process
ci.yml Updated expected test count from 466 to 474

@openhands-ai
Copy link

openhands-ai bot commented Sep 28, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • CI
    • CI

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #100 at branch `feature/jqwik-property-testing-88`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

@simbo1905 simbo1905 changed the title Exhaustive property-based testing for JTD validator JtdExhaustiveTest Exhaustive property-based testing for JTD validator JtdPropertyTest Sep 28, 2025
@simbo1905 simbo1905 changed the title Exhaustive property-based testing for JTD validator JtdPropertyTest Random property-based testing for JTD validator JtdPropertyTest Sep 28, 2025
@simbo1905 simbo1905 changed the title Random property-based testing for JTD validator JtdPropertyTest jqwik property-based testing for JTD validation JtdPropertyTest Sep 28, 2025
@simbo1905 simbo1905 merged commit 2478f20 into main Sep 28, 2025
4 checks passed
@simbo1905 simbo1905 deleted the feature/jqwik-property-testing-88 branch September 28, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add jqwik property-based testing for JTD validation

2 participants