Skip to content

Conversation

@bdolewski-intellias
Copy link
Contributor

TL;DR

This PR fixes failing Cobertura converter tests and ensures the generated XML strictly follows Cobertura conventions.
It removes formatting inconsistencies, updates stale coverage values, and replaces compact empty elements (<methods/>) with explicit tags (<methods></methods>).
The result is Cobertura XML that passes all tests and is accepted by strict coverage parsers used by external tools.

Summary

This PR fixes the failing testCoberturaConverter() test and corrects Cobertura XML output so it conforms more closely to the expected standard. The issue surfaced when external coverage tools (e.g. DataDog CI) rejected the generated reports as invalid, but the problem is more general: the XML did not consistently match Cobertura conventions.

Problem

  • The testCoberturaConverter() test was failing due to formatting mismatches and stale coverage data.
  • Generated Cobertura XML reports contained small but significant inconsistencies:
    • Compact empty elements (<methods/>) instead of full form (<methods></methods>)
    • Missing standalone="yes" attribute in the XML declaration
    • Outdated coverage values in the expected test fixtures

These deviations, while valid XML, can cause strict Cobertura parsers to reject reports.

Changes Made

Test Infrastructure

  • Unified XML comparison logic in assertXmlTestReportsAreEqual() to apply consistent formatting options ([.nodePrettyPrint, .nodeCompactEmptyElement]).
  • Ensures stable and predictable XML output during tests.

Test Assets

  • Updated cobertura.xml fixture with current coverage values.
  • Fixed XML declaration to include standalone="yes".
  • Replaced compact empty elements with explicit opening/closing tags.

Standards Compliance

  • Output now uses a Cobertura-compliant structure expected by many coverage processing tools.
  • Example: <methods></methods> instead of <methods/>.

Testing

  • ✅ All 24 tests pass
  • testCoberturaConverter() and testCoberturaConverterExcludeFiles() now succeed
  • ✅ Verified that resulting Cobertura XML is accepted by external coverage tools

Impact

  • Fixes failing tests that blocked CI pipelines
  • Produces Cobertura XML compatible with strict parsers and external coverage services (e.g. DataDog, but not limited to it)
  • Improves reliability of code coverage reporting in downstream integrations

@a7ex
Copy link
Owner

a7ex commented Sep 23, 2025

Hello Bartosz,
thanks for sharing the enhancements. I will look into the PR asap.

Copy link
Owner

@a7ex a7ex left a comment

Choose a reason for hiding this comment

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

I do not have cobertura needs my self.
I suppose you have tested it yourself and that your changes are necessary for xcresultparser to output valid cobertura xml.

// TODO: some of these values are B.S. - figure out how to calculate, or better to omit if we don't know?
let testAction = invocationRecord.actions.first { $0.schemeCommandName == "Test" }
let timeStamp = (testAction?.startedTime.timeIntervalSince1970) ?? Date().timeIntervalSince1970
let timeStamp = (testAction?.startedTime.timeIntervalSince1970) ?? 1672825221.218
Copy link
Owner

Choose a reason for hiding this comment

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

Why was this changed to a literal?
Todays date as a default seems more reasonable to me.

@a7ex a7ex merged commit bfb16c5 into a7ex:main Sep 24, 2025
1 check passed
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.

2 participants