-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5,11 +5,14 @@ | |||||
| import jdk.sandbox.java.util.json.Json; | ||||||
| import org.junit.jupiter.api.DynamicTest; | ||||||
| import org.junit.jupiter.api.TestFactory; | ||||||
| import org.junit.jupiter.api.AfterAll; | ||||||
| import org.junit.jupiter.api.Assumptions; | ||||||
|
|
||||||
| import java.io.File; | ||||||
| import java.nio.file.Files; | ||||||
| import java.nio.file.Path; | ||||||
| import java.util.concurrent.ConcurrentHashMap; | ||||||
| import java.util.concurrent.atomic.LongAdder; | ||||||
| import java.util.stream.Stream; | ||||||
| import java.util.stream.StreamSupport; | ||||||
|
|
||||||
|
|
@@ -25,6 +28,8 @@ public class JsonSchemaCheckIT { | |||||
| new File("target/json-schema-test-suite/tests/draft2020-12"); | ||||||
| private static final ObjectMapper MAPPER = new ObjectMapper(); | ||||||
| private static final boolean STRICT = Boolean.getBoolean("json.schema.strict"); | ||||||
| private static final String METRICS_FMT = System.getProperty("json.schema.metrics", "").trim(); | ||||||
| private static final SuiteMetrics METRICS = new SuiteMetrics(); | ||||||
|
|
||||||
| @SuppressWarnings("resource") | ||||||
| @TestFactory | ||||||
|
|
@@ -37,6 +42,19 @@ Stream<DynamicTest> runOfficialSuite() throws Exception { | |||||
| private Stream<DynamicTest> testsFromFile(Path file) { | ||||||
| try { | ||||||
| JsonNode root = MAPPER.readTree(file.toFile()); | ||||||
|
|
||||||
| // Count groups and tests discovered | ||||||
| int groupCount = root.size(); | ||||||
| METRICS.groupsDiscovered.add(groupCount); | ||||||
| perFile(file).groups.add(groupCount); | ||||||
|
|
||||||
| int testCount = 0; | ||||||
| for (JsonNode group : root) { | ||||||
| testCount += group.get("tests").size(); | ||||||
| } | ||||||
| METRICS.testsDiscovered.add(testCount); | ||||||
| perFile(file).tests.add(testCount); | ||||||
|
|
||||||
| return StreamSupport.stream(root.spliterator(), false) | ||||||
| .flatMap(group -> { | ||||||
| String groupDesc = group.get("description").asText(); | ||||||
|
|
@@ -55,29 +73,62 @@ private Stream<DynamicTest> testsFromFile(Path file) { | |||||
| try { | ||||||
| actual = schema.validate( | ||||||
| Json.parse(test.get("data").toString())).valid(); | ||||||
|
|
||||||
| // Count validation attempt | ||||||
| METRICS.validationsRun.increment(); | ||||||
| perFile(file).run.increment(); | ||||||
| } catch (Exception e) { | ||||||
| String reason = e.getMessage() == null ? e.getClass().getSimpleName() : e.getMessage(); | ||||||
| System.err.println("[JsonSchemaCheckIT] Skipping test due to exception: " | ||||||
| + groupDesc + " — " + reason + " (" + file.getFileName() + ")"); | ||||||
|
|
||||||
| // Count exception skip | ||||||
| METRICS.skipTestException.increment(); | ||||||
| perFile(file).skipException.increment(); | ||||||
|
|
||||||
| if (STRICT) throw e; | ||||||
| Assumptions.assumeTrue(false, "Skipped: " + reason); | ||||||
| return; // not reached when strict | ||||||
| } | ||||||
|
|
||||||
| if (STRICT) { | ||||||
| assertEquals(expected, actual); | ||||||
| try { | ||||||
| assertEquals(expected, actual); | ||||||
| // Count pass in strict mode | ||||||
| METRICS.passed.increment(); | ||||||
| perFile(file).pass.increment(); | ||||||
| } catch (AssertionError e) { | ||||||
| // Count failure in strict mode | ||||||
| METRICS.failed.increment(); | ||||||
| perFile(file).fail.increment(); | ||||||
| throw e; | ||||||
| } | ||||||
| } else if (expected != actual) { | ||||||
| System.err.println("[JsonSchemaCheckIT] Mismatch (ignored): " | ||||||
| + groupDesc + " — expected=" + expected + ", actual=" + actual | ||||||
| + " (" + file.getFileName() + ")"); | ||||||
|
|
||||||
| // Count lenient mismatch skip | ||||||
| METRICS.skipLenientMismatch.increment(); | ||||||
| perFile(file).skipMismatch.increment(); | ||||||
|
|
||||||
| Assumptions.assumeTrue(false, "Mismatch ignored"); | ||||||
| } else { | ||||||
| // Count pass in lenient mode | ||||||
| METRICS.passed.increment(); | ||||||
| perFile(file).pass.increment(); | ||||||
| } | ||||||
| })); | ||||||
| } catch (Exception ex) { | ||||||
| // Unsupported schema for this group; emit a single skipped test for visibility | ||||||
| String reason = ex.getMessage() == null ? ex.getClass().getSimpleName() : ex.getMessage(); | ||||||
| System.err.println("[JsonSchemaCheckIT] Skipping group due to unsupported schema: " | ||||||
| + groupDesc + " — " + reason + " (" + file.getFileName() + ")"); | ||||||
|
|
||||||
| // Count unsupported group skip | ||||||
| METRICS.skipUnsupportedGroup.increment(); | ||||||
| perFile(file).skipUnsupported.increment(); | ||||||
|
|
||||||
| return Stream.of(DynamicTest.dynamicTest( | ||||||
| groupDesc + " – SKIPPED: " + reason, | ||||||
| () -> { if (STRICT) throw ex; Assumptions.assumeTrue(false, "Unsupported schema: " + reason); } | ||||||
|
|
@@ -88,4 +139,146 @@ private Stream<DynamicTest> testsFromFile(Path file) { | |||||
| throw new RuntimeException("Failed to process " + file, ex); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private static SuiteMetrics.FileCounters perFile(Path file) { | ||||||
| return METRICS.perFile.computeIfAbsent(file.getFileName().toString(), k -> new SuiteMetrics.FileCounters()); | ||||||
| } | ||||||
|
|
||||||
| @AfterAll | ||||||
| static void printAndPersistMetrics() throws Exception { | ||||||
| var strict = STRICT; | ||||||
| var totalRun = METRICS.validationsRun.sum(); | ||||||
| var passed = METRICS.passed.sum(); | ||||||
| var failed = METRICS.failed.sum(); | ||||||
| var skippedU = METRICS.skipUnsupportedGroup.sum(); | ||||||
| var skippedE = METRICS.skipTestException.sum(); | ||||||
| var skippedM = METRICS.skipLenientMismatch.sum(); | ||||||
|
|
||||||
| System.out.printf( | ||||||
| "JSON-SCHEMA SUITE (%s): groups=%d testsScanned=%d run=%d passed=%d failed=%d skipped={unsupported=%d, exception=%d, lenientMismatch=%d}%n", | ||||||
| strict ? "STRICT" : "LENIENT", | ||||||
| METRICS.groupsDiscovered.sum(), | ||||||
| METRICS.testsDiscovered.sum(), | ||||||
| totalRun, passed, failed, skippedU, skippedE, skippedM | ||||||
| ); | ||||||
|
|
||||||
| if (!METRICS_FMT.isEmpty()) { | ||||||
| var outDir = java.nio.file.Path.of("target"); | ||||||
|
||||||
| var outDir = java.nio.file.Path.of("target"); | |
| var outDir = java.nio.file.Path.of(System.getProperty("test.output.dir", "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] 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.
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.
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.