Conversation
The file name was interpolated raw into the <failure message="..."> attribute while the failure message text was encoded. Encode the file name via encode_xml_text() before use to prevent malformed XML if the path contains characters like ", <, or &.
The element was always written empty — stderr was never captured. Per the spec, <system-err> is optional; omitting it is cleaner than emitting a misleading empty tag.
The suite-level <properties> virtual was never implemented or called with any real data. Removing the dead stub and its call site.
Output printed during a test (between current_test_started and current_test_ended) is now emitted as <system-out> inside the <testcase> element. Output printed outside any test continues to appear in the suite-level <system-out>.
-pjunit now defaults the package name to the basename of argv[0], disambiguating output files in parallel ctest runs without any user configuration. An explicit override is available via -pjunit=<name> for users who need a different prefix.
|
coverage: 98.629% (-0.01%) from 98.642% — junity into main |
Instead of one file per test group, all groups are now accumulated into a single file written at the end of the run. This eliminates file-name collisions when ctest runs test executables in parallel, and produces a conformant JUnit XML report with a top-level <testsuites> element whose totals (tests, failures, errors, skipped, time) span the whole run.
Two related fixes: 1. create_file_name(): when a package is set, use it directly as the file stem rather than prepending "mutiny_". This eliminates the double-prefix (mutiny_mutiny_unit.xml) that appeared when the plugin defaulted the package name to the executable basename. 2. Discovery script: instead of appending -pjunit to every test's shared ARGS (which caused all groups of the same executable to clobber the same file), pass JUNIT_REPORT:BOOL separately and emit a unique -pjunit=<target>.<group> per ctest test. Each group now gets its own collision-free output file (mutiny_unit.JUnitOutput.xml, etc.).
- junit-output.rst: rewrite for current behavior — single file per run named after the executable/package, <testsuites> wrapper in the XML example, -pjunit=<name> override syntax, ctest per-group file naming, and corrected CI glob patterns (build/**/*.xml instead of cpputest_*.xml) - Mutiny.cmake: correct the MUTINY_JUNIT_REPORT doc comment to describe the per-test -pjunit=<target>.<group> behavior
Rename two test cases to describe what they actually test (between-test output routing to suite-level system-out) and remove the UTPRINT mention from the docs, replacing it with an accurate description of the output routing rules.
stdout was never captured
XML declaration is now emitted inline in print_tests_ended.
The method now just clears current_group_xml; the parameter was unused since the single-file refactor.
References said --junit; the actual flag is -pjunit[=<name>].
Verifies that the top-level wrapper sums tests, failures, and skipped counts from multiple groups.
Skip writing <testsuite> elements for groups with zero tests — groups that are registered but filtered out produced empty name="" / tests="0" noise in the output. For SKIPPED_TEST tests, populate the <skipped> message attribute with the macro name (e.g. "SKIPPED_TEST") so CI tools have something useful to display. Also move Shell::get_macro_name() from protected to public since it is pure read-only metadata.
No subclass overrides these methods — they were vestigial hooks from the original CppUTest design, superseded by the Output-level injectable function pointers (fopen_, fputs_, fclose_). Move all to private and drop virtual. Rewrite the one test that called create_file_name() directly to go through the public file-system interface instead.
14-year-old SKIPPED_TEST with no assertions, introduced in 2011 as a profiling scaffold and never activated. Carried forward through every refactor unchanged.
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
2 participants
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.
Description
Reviews the JUnit XML spec at https://github.com/testmoapp/junitxml/ and fixes five conformance issues in the
JUnitOutputimplementation, then cleans up the resulting dead code.Issue 1 — File path not XML-encoded in
<failure>attributeThe
messageattribute of<failure>included a raw file path, which could contain<,>,&,\"characters that are illegal unescaped in XML attributes. The file name is now passed throughencode_xml_text().Issue 2 — Always-empty
<system-err>elementEvery test suite emitted a
<system-err></system-err>block even though Mu::tiny has no mechanism to capture stderr. The element was removed rather than left misleadingly empty.Issue 3 — No-op
write_properties()stubA
write_properties()virtual method existed but its body was empty, silently discarding any properties. The stub was removed;TEST_PROPERTYsupport was already wired throughprint_test_property()/write_test_cases().Issue 4 — No
<testsuites>wrapperEach test group was written to a separate file with no enclosing
<testsuites>element. Groups are now accumulated into a single file written at the end of the run, wrapped in<testsuites>with aggregatetests,failures,errors,skipped, andtimetotals across all groups. Empty groups are suppressed.Separate from this, the plugin now defaults the package name to the executable basename when
-pjunitis given without=name, producing classnames likemutiny_unit.GroupNamethat identify the source executable in CI reports.Issue 5 —
<system-out>was suite-level onlyOutput printed during a test (via
print()) was collected into a single suite-level<system-out>block. It is now routed per-testcase when inside a test, appearing inside the correct<testcase>element.ctest collision fix
When
MUTINY_JUNIT_REPORTis enabled,mutiny_discover_tests()previously appended-pjunitto the sharedARGSfor every ctest invocation of a given executable. Because ctest runs each group as a separate process, all groups of the same executable wrote to the same file and clobbered each other. The fix moves JUnit flag generation into the discovery script, emitting a unique-pjunit=<target>.<group>per ctest test so each group gets its own collision-free output file.Dead code removal
After the conformance fixes, the
protected virtualmethods onJUnitOutput(open_file_for_write,write_test_group_to_file,write_to_file,close_file,write_test_suite_summary,write_test_cases,encode_xml_text,encode_file_name,write_failure,write_error,write_file_ending) had no subclass overrides — they were vestigial hooks superseded by the injectable function pointers onOutput. All are nowprivatenon-virtual.Related Issues
Fixes # (issue number)
Type of Change
Checklist
docs/for any user-facing changes.mu::tinynamespace,INCLUDED_MUTINY_guards,mutiny_C-prefix)..hand.c.cpp) is required for parity.CONTRIBUTING.mdfile to ensure compliance with architectural guidelines.