cicirello / jacoco-badge-generator

Coverage badges, and pull request coverage checks, from JaCoCo reports in GitHub Actions
https://actions.cicirello.org/jacoco-badge-generator/
MIT License
101 stars 42 forks source link

BUG: file not found in Multi-module #18

Closed cprudhom closed 3 years ago

cprudhom commented 3 years ago

I have a two-module project and I use TestNG as testing framework, which allows (like JUnit I believe) to declare groups. I declared 8 groups, (a) some are tagged in both modules, (b) some in only one of them. I configured this GitHub action in order to list jacoco-csv files but in (b) case only one exists.

When I run the action, it throws FileNotFoundError: [Errno 2] No such file or directory: 'solver/target/site/jacoco/jacoco.csv' which is expected. But I wonder whether it is possible to add something like:

if os.path. exists(filename):

just before JacocoBadgeGenerator.py#L90.

cicirello commented 3 years ago

I can do that. I considered that to begin with, but decided not to check, with the rationale that a missing jacoco.csv may be due to a typo in a workflow file, and that letting the workflow fail would draw attention to such a bug, while simply skipping it would report a coverage percentage that may be inaccurate.

I guess I can skip missing reports, but log them.

cprudhom commented 3 years ago

I found a way around it, uploading and downloading artifacts. It makes the workflow a bit complex and I'm afraid it will raise another problem. It also requires to call merge action from maven-jacoco-plugin in coverage profile of pom.xml.

Here is a bunch of code (sorry, it is a bit long):

name: Java CI with Maven

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]

jobs:
  test-suite:
    runs-on: ubuntu-latest
    # Tests matrix
    strategy:
      #fail-fast: false
      matrix:
        group: [ "gr1" , "gr2", "gr3" ]

    # The different steps
    steps:
      - uses: actions/checkout@v2
      - name: Set up JDK 11
        uses: actions/setup-java@v2
        with:
          java-version: '11'
          distribution: 'adopt'

      # Regression tests
      - name: Test ${{ matrix.group }}
        run: mvn -B --file pom.xml package -Pcoverage -DtestFailureIgnore=true -Dgroups=${{ matrix.group }}

      # upload jacoco-csv files
      - name: Prepare upload
        run: mkdir -p jacoco/
      - run: cp modul1/target/site/jacoco/jacoco.csv jacoco/mod1.csv
      - run: cp module2/target/site/jacoco/jacoco.csv jacoco/mod2.csv
        if: ${{ always() }}
      - name: Upload jacoco-csv files
        if: ${{ always() }}
        uses: actions/upload-artifact@master
        with:
          name: ${{ matrix.group }}
          path: |
            jacoco/mod1.csv
            jacoco/mod2.csv
          if-no-files-found: ignore

  coverage:
    needs: [ test-suite ]
    runs-on: ubuntu-latest
    steps:
      # Download jacoco-csv files
      - uses: actions/download-artifact@v2
        name: Download jacoco-csv files
        with:
          path: jacoco/

      - name: Generate JaCoCo Badge
        id: jacoco
        uses: cicirello/jacoco-badge-generator@v2.1.0
        with:
          generate-branches-badge: true
          jacoco-csv-file: >
            jacoco/gr1/mod1.csv
            jacoco/gr1/mod2.csv
            jacoco/gr2/mod1.csv
#          jacoco/gr2/mod2.csv does not exist
#          jacoco/gr3/mod1.csv does not exist 
            jacoco/gr3/mod2.csv 

      - name: Log coverage percentage
        run: |
          echo "coverage = ${{ steps.jacoco.outputs.coverage }}"
          echo "branch coverage = ${{ steps.jacoco.outputs.branches }}"

That worked but problem is that I'm sure additions JacocoBadgeGenerator.py#L94 reflect the union expected.

cicirello commented 3 years ago

I'm going to implement a fix either way.

But what causes a jacoco report to generate sometimes but not other times? The only reason I can think of is if one or more tests fail. But wouldn't test case failures cause the build itself to fail? And if not, and if the missing reports are for the modules with test case failures, then computing coverage skipping missing reports may give you inaccurate results.

Here is what I'm thinking of doing:

  1. If a report is missing, log a warning to standard out, so you can see which were missing in the workflow output.
  2. Compute coverage from whatever reports are there.
  3. Add an input to give the user of the action control over whether a badge is generated if one or more expected reports are missing.
  4. Also give control to user of action whether or not to fail the workflow if expected report is missing. Perhaps a boolean input ´fail-on-warning´.

Numbers 3 and 4 above would essentially enable user to decide how to handle missing reports: fail workflow, or don't fail workflow but also don't generate badges, or generate badges anyway. Either way the names of the missing reports would be found in the workflow output.

Perhaps one input might cover both. Maybe something like ´on-warning´ with possible values of ´fail´ to fail the run, ´quiet´ to not fail the run but also not generate badges, and ´badges´ to generate badges despite missing reports.

cprudhom commented 3 years ago

Sorry I wasn't clear at all.

Short version I doubt that the simple sum of the indicators read from the csv files represents the true code coverage in my case.

Long version As you can see, I have a multi-module maven project and my configuration is based on a matrix. Each element of the matrix represents a group of tests (via TestNG). The execution of a group covers part of the code between the 2 modules and produces (at most) 2 jacoco-csv file, one for each module. Then all files are collected and passed to the action which sums percentages. In my case, it happens that two test suites cover the same class from a module. But since the rows covered are not specified in each csv file, the sum of the indicators does not represent the union of coverage. Indeed, it can be equal to 150%. Or there is something I missed.

For instance, here are the indicators for the class PropMinBC from 2 files, each relating to a test suite.

// First test suite
choco-solver,org.chocosolver.solver.constraints.ternary,PropMinBC,31,295,7,21,5,27,8,11,1,4
// second test suite
choco-solver,org.chocosolver.solver.constraints.ternary,PropMinBC,115,211,17,11,12,20,11,8,2,3

The first one covers 295/326 instructions, the second 211/326 instructions. Sum the values is not correct (506 >> 326), neither is averaging (253) nor take the maximum (295). The appropriate approach would be to merge result (based on the binaries exec I suppose).

A workaround would be to avoid using the matrix strategy, but tests require too much time (> 1h).

cicirello commented 3 years ago

Oh I see now. Computing coverage by combining the data in the 2 csv files definitely won't be correct in your case due to the overlap. I believe it would give you the equivalent of an average or at least it would if both are based on the exact same lines of code, since it assumes the reports are independent. The 326 lines of code in each is just assumed different, interpreting it as 652 lines of code. But an average doesn't tell you anything particularly meaningful.

There is also not enough info in either the csv or the xml reports to properly merge the results. Probably the best approach is to use jacoco:merge and let jacoco take care of merging into one report.

cprudhom commented 3 years ago

I created a GitHub action jacoco-merge which should be combined with a JaCoCo report. I let you fix/close this issue but I don't think you should make too much of it either

cicirello commented 3 years ago

@cprudhom You might also take a look at jacoco:report-aggregate. I'm not sure if it applies to your case or not.