Skip to content

fix(cobertura): deduplicate method names within a class - #11

Merged
AriPerkkio merged 2 commits into
vitest-dev:mainfrom
travzhang:fix/cobertura-deduplicate-method-names
Aug 31, 2026
Merged

fix(cobertura): deduplicate method names within a class#11
AriPerkkio merged 2 commits into
vitest-dev:mainfrom
travzhang:fix/cobertura-deduplicate-method-names

Conversation

@travzhang

@travzhang travzhang commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Deduplicate <method name="..."> values in the Cobertura reporter when a file's fnMap contains colliding function names (e.g. two (anonymous_0) or two constructor entries).

Related: vitest-dev/vitest#11069

Scope

This PR intentionally fixes the issue only at the Cobertura report layer.

Duplicate function names in a merged fnMap are valid in JavaScript and can be expected when coverage from multiple chunks is combined. Rather than renaming functions during merge() in istanbul-lib-coverage, this change ensures the Cobertura XML output is safe for downstream consumers (notably the Jenkins Coverage plugin), which require method names to be unique within a class.

Rationale:

  • The failure happens at Cobertura report consumption time, not during coverage collection or merging.
  • Other report formats do not share this constraint.
  • Keeping fnMap names unchanged preserves existing coverage data semantics; only the Cobertura XML representation is adjusted.

Problem

When coverage from multiple chunks is merged, fnMap entries for the same source file can end up with identical name values but different source locations. The Cobertura reporter previously emitted both with the same name attribute, causing Jenkins to fail parsing the report.

Example:

  • (anonymous_0) at line 12
  • (anonymous_0) at line 16

Solution

When emitting <method> tags for a class, deduplicate names within that class:

  • First occurrence keeps the original name
  • Later collisions become name_2, name_3, …

State is scoped per class via a createUniqueNamer() helper.

Test plan

  • Added unit test for duplicate constructor and (anonymous_0) names in the same file
  • Existing Cobertura fixture tests still pass
  • Verified output names: constructor, constructor_2, (anonymous_0), (anonymous_1)

@AriPerkkio AriPerkkio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a note that in addition to fixing merge() (which will happen in #10), this change is required on its own. It's possible to end up with duplicate fnMap entries with plain single-file Javascript too: AriPerkkio/ast-v8-to-istanbul#169 (comment). This PR will make sure Jenkins doesn't crash in those cases either.

Comment thread packages/istanbul-lib-report/test/reports/cobertura/index.test.ts Outdated

@AriPerkkio AriPerkkio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@AriPerkkio
AriPerkkio merged commit 97eb92d into vitest-dev:main Aug 31, 2026
4 of 10 checks 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