-
Notifications
You must be signed in to change notification settings - Fork 0
fix: improve JsonSchemaCheckIT metrics reporting for defensible compatibility statistics #32
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
…tibility statistics - Add comprehensive SuiteMetrics class with thread-safe counters - Track groups discovered, tests discovered, validations run, passed/failed - Categorize skips: unsupportedSchemaGroup, testException, lenientMismatch - Add console summary line with detailed metrics breakdown - Support JSON/CSV export via -Djson.schema.metrics=json|csv - Add per-file breakdown for detailed analysis - Preserve existing strict/lenient behavior while adding metrics - Zero additional dependencies, thread-safe implementation Fixes #31
…maCheckIT - Replace estimated 71% compatibility with actual measured 63.3% (1,153 of 1,822 tests) - Add comprehensive metrics reporting documentation - Document test coverage: 420 groups, 1,657 validations, 576 skips categorized - Add usage examples for JSON/CSV metrics export - Clarify distinction between lenient and strict mode results - Provide defensible statistics based on actual test suite measurements The documentation now reflects the accurate, measured compatibility statistics provided by the new metrics system rather than estimates.
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.
Pull Request Overview
This PR implements comprehensive metrics collection in JsonSchemaCheckIT to provide defensible, repeatable compatibility statistics instead of estimated percentages.
- Added SuiteMetrics class with thread-safe counters for groups, tests, validations, and skip reasons
- Enhanced test execution tracking to count discovered vs run tests and categorize skips (unsupported groups, exceptions, lenient mismatches)
- Implemented console summary and structured export functionality (JSON/CSV) with per-file breakdown
- Updated documentation to replace estimated 71% compatibility with actual measured 63.3%
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| JsonSchemaCheckIT.java | Added comprehensive metrics collection, SuiteMetrics class, and export functionality |
| README.md | Updated compatibility claims from estimated 71% to measured 63.3% with metrics details |
| AGENTS.md | Added metrics reporting documentation and current compatibility statistics |
| README.md (root) | Updated JSON Schema compatibility section with measured statistics and metrics usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| private static SuiteMetrics.FileCounters perFile(Path file) { | ||
| return METRICS.perFile.computeIfAbsent(file.getFileName().toString(), k -> new SuiteMetrics.FileCounters()); | ||
| } |
Copilot
AI
Sep 15, 2025
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.
[nitpick] The method name 'perFile' is unclear and doesn't indicate it's a getter/accessor. Consider renaming to 'getOrCreateFileCounters' to better express its purpose.
| ); | ||
|
|
||
| if (!METRICS_FMT.isEmpty()) { | ||
| var outDir = java.nio.file.Path.of("target"); |
Copilot
AI
Sep 15, 2025
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.
[nitpick] Hardcoded 'target' directory path reduces flexibility. Consider using a system property or making it configurable, especially since other paths in the codebase may vary across different build environments.
| var outDir = java.nio.file.Path.of("target"); | |
| var outDir = java.nio.file.Path.of(System.getProperty("test.output.dir", "target")); |
| private static String buildJsonSummary(boolean strict, String timestamp) { | ||
| var totals = new StringBuilder(); | ||
| totals.append("{\n"); |
Copilot
AI
Sep 15, 2025
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.
[nitpick] Manual JSON construction using StringBuilder is error-prone and hard to maintain. Consider using the existing ObjectMapper (MAPPER) to serialize a data structure to JSON, which would be more robust and less prone to formatting errors.
| /** | ||
| * Thread-safe metrics container for the JSON Schema Test Suite run. | ||
| */ | ||
| final class SuiteMetrics { |
Copilot
AI
Sep 15, 2025
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.
[nitpick] The SuiteMetrics class is missing comprehensive documentation. Add class-level Javadoc explaining its purpose, thread-safety guarantees, and the meaning of different counter categories to help future maintainers understand the metrics collection design.
Summary
This PR implements comprehensive metrics collection in JsonSchemaCheckIT to provide defensible, repeatable compatibility statistics instead of estimated percentages.
Changes
unsupportedSchemaGroup: Whole groups skipped at compile timetestException: Individual tests that threw exceptionslenientMismatch: Expected≠actual in lenient mode-Djson.schema.metrics=json|csvExample Output
Console Summary (Lenient Mode)
Console Summary (Strict Mode)
Actual Measured Compatibility
63.3% (1,153 of 1,822 tests pass) - replacing the previous estimated claim of 71%
Test Coverage: 420 test groups, 1,657 validation attempts, 576 total skips categorized
Bug Fix: Filename Escaping
Fixed a critical bug where filenames containing special characters (quotes, commas, newlines) would break the JSON and CSV output:
Compatibility
Usage
Documentation Updates
Impact
This provides the defensible metrics needed to support compatibility claims:
Fixes #31
EOF < /dev/null