Skip to content

fix: restore quickfixj-core JaCoCo coverage and exclude generated message modules#1286

Merged
chrjohn merged 2 commits into
masterfrom
copilot/coverage-issues-fix
Jul 25, 2026
Merged

fix: restore quickfixj-core JaCoCo coverage and exclude generated message modules#1286
chrjohn merged 2 commits into
masterfrom
copilot/coverage-issues-fix

Conversation

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Two coverage bugs introduced in #1285: quickfixj-core tests produced no coverage data, and generated message classes (mapped to src/test/resources/golden/ source files by Codecov) appeared in the report despite the intent to exclude them.

quickfixj-core/pom.xml — JaCoCo agent not reaching the test JVM

Both surefire-java8 and surefire profiles override <argLine> with concrete JVM flags but omit @{argLine}. A non-empty child override replaces the parent's value entirely, discarding the JaCoCo agent. quickfixj-base happened to work because its profiles use an empty <argLine>, which Maven leaves unset and falls back to the parent's @{argLine}.

<!-- before -->
<argLine>-Xmx2g -Djava.net.preferIPv4Stack=true</argLine>

<!-- after -->
<argLine>@{argLine} -Xmx2g -Djava.net.preferIPv4Stack=true</argLine>

quickfixj-messages/pom.xml — message modules generating unwanted reports

JaCoCo reported coverage for generated quickfix.fixlatest.* / quickfix.field.* classes. Codecov then resolved their SourceFile attributes to the identically-named .java files under src/test/resources/golden/, making those golden files appear as covered source. The existing <excludes> in the root pom was using filesystem paths (quickfixj-messages/quickfixj-messages-*/target/classes/**) — JaCoCo report excludes require class-name patterns — so it never filtered anything.

Simplest correct fix: skip JaCoCo entirely for all message sub-modules (generated code, no meaningful coverage value) via their shared parent pom, and remove the broken <excludes> from the root pom.

<!-- quickfixj-messages/pom.xml -->
<plugin>
    <groupId>org.jacoco</groupId>
    <artifactId>jacoco-maven-plugin</artifactId>
    <configuration>
        <skip>true</skip>
    </configuration>
</plugin>

…odules

- quickfixj-core/pom.xml: prepend @{argLine} in both surefire profile
  argLine overrides so the JaCoCo Java agent is actually passed to the
  test JVM (previously the profile values replaced the parent's @{argLine}
  and no coverage data was collected for core)
- quickfixj-messages/pom.xml: add <skip>true</skip> for jacoco-maven-plugin
  so that generated message-class modules are excluded from coverage
  entirely, preventing golden-file source-mapping artifacts in Codecov
- pom.xml: remove the broken jacoco report <excludes> pattern that used
  filesystem paths instead of class-name patterns and never matched anything
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.22%. Comparing base (9b58948) to head (33530dc).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #1286       +/-   ##
=============================================
+ Coverage     42.85%   70.22%   +27.37%     
- Complexity      684     2242     +1558     
=============================================
  Files           125      163       +38     
  Lines          3794     9076     +5282     
  Branches        359     1192      +833     
=============================================
+ Hits           1626     6374     +4748     
- Misses         2021     2238      +217     
- Partials        147      464      +317     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@the-thing

Copy link
Copy Markdown
Collaborator

Also maybe.

if: matrix.os == 'ubuntu-latest' && matrix.java == 21
=>
if: matrix.os == 'ubuntu-latest' && matrix.java == 21 && github.repository == 'quickfix-j/quickfixj'

@chrjohn

chrjohn commented Jul 25, 2026

Copy link
Copy Markdown
Member

Also maybe.

if: matrix.os == 'ubuntu-latest' && matrix.java == 21 => if: matrix.os == 'ubuntu-latest' && matrix.java == 21 && github.repository == 'quickfix-j/quickfixj'

@the-thing good point, but isn't upload prevented anyway since other repo does not have the secret?

@the-thing

Copy link
Copy Markdown
Collaborator

Yes, but I don't remember if build fails silently or the whole build fails.

@chrjohn
chrjohn marked this pull request as ready for review July 25, 2026 17:42
@chrjohn

chrjohn commented Jul 25, 2026

Copy link
Copy Markdown
Member

@the-thing added, thanks.

@chrjohn
chrjohn merged commit 1c48bd4 into master Jul 25, 2026
22 of 23 checks passed
@chrjohn
chrjohn deleted the copilot/coverage-issues-fix branch July 25, 2026 17:43
@chrjohn chrjohn added this to the QFJ 3.0.2 milestone Jul 26, 2026
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.

3 participants