Report elapsed time in JUnit XML#146
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds elapsed-duration reporting to JUnit XML output so CI consumers can display non-zero suite and testcase times.
Changes:
- Adds
elapsedSecondstoCrapReportwith a helper forSystem.nanoTime()deltas. - Uses elapsed time for JUnit
testsuites,testsuite, and per-testcasetimeattributes. - Updates formatter and integration tests for the new dynamic time values.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
core/src/main/java/media/barney/crap/core/CrapReport.java |
Carries elapsed seconds on reports. |
core/src/main/java/media/barney/crap/core/Main.java |
Records elapsed time for pre-resolved module runs. |
core/src/main/java/media/barney/crap/core/ReportFormatter.java |
Formats elapsed time into JUnit XML attributes. |
core/src/test/java/media/barney/crap/core/ReportFormatterTest.java |
Tests elapsed JUnit formatting. |
core/src/test/java/media/barney/crap/core/MainTest.java |
Updates JUnit sidecar assertions for dynamic time values. |
Comments suppressed due to low confidence (5)
core/src/test/java/media/barney/crap/core/MainTest.java:258
- Like the earlier integration assertion, this only proves that a
timeattribute was emitted; it still passes for the default0.0value produced when elapsed time is not propagated throughCliApplication. Add a non-flaky check that this CLI path uses a real elapsed duration.
assertTrue(junit.contains("<testsuites tests=\"3\" failures=\"1\" errors=\"0\" skipped=\"1\" time=\""));
core/src/test/java/media/barney/crap/core/MainTest.java:289
- This weakened assertion accepts any
timevalue, including the default0.0, so it does not cover the new behavior for this CLI path. The test should verify that elapsed time is actually populated in the JUnit sidecar rather than just checking for the attribute prefix.
assertTrue(junit.contains("<testsuites tests=\"3\" failures=\"1\" errors=\"0\" skipped=\"1\" time=\""));
core/src/test/java/media/barney/crap/core/MainTest.java:323
- This assertion does not catch elapsed-time propagation failures because
time="0.0"also satisfies it. Since agent mode still emits a JUnit sidecar through the CLI path, the test should check a deterministic elapsed value or otherwise prove that the sidecar is not using the default report time.
assertTrue(junit.contains("<testsuites tests=\"3\" failures=\"1\" errors=\"0\" skipped=\"1\" time=\""));
core/src/test/java/media/barney/crap/core/MainTest.java:392
- This sidecar assertion is too broad for the new timing behavior: it passes even when the full JUnit report still contains the default
time="0.0". Add coverage that verifies elapsed time is propagated while keeping the sidecar complete under--failures-only.
assertTrue(junit.contains("<testsuites tests=\"3\" failures=\"1\" errors=\"0\" skipped=\"1\" time=\""));
core/src/test/java/media/barney/crap/core/MainTest.java:520
- This integration check also accepts the old/default
time="0.0"output, so it does not verify that the legacy pre-resolved-module overload attaches elapsed time before publishing the JUnit XML. Consider adding a deterministic timing hook or parsing the XML and asserting the expected elapsed value for this path.
assertTrue(Files.readString(junitReport).contains("<testsuites tests=\"3\" failures=\"1\" errors=\"0\" skipped=\"1\" time=\""));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t-metadata # Conflicts: # core/src/test/java/media/barney/crap/core/MainTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
errors=0unchanged because threshold breaches are modeled as JUnit failures and execution errors do not emit a reportCloses #128
Verification