bcgov / jag-file-submission

Generic File Submission API
https://bcgov.github.io/jag-file-submission/#/
Apache License 2.0
6 stars 13 forks source link

Code Climate - Issue Tracking for Demo #205

Closed akroon3r closed 3 years ago

akroon3r commented 4 years ago

Shelly from BCDevExchange would like us to track (pros/cons) that we come across during the implementation of Code Climate.

Feel free to add comments as you see fit.

akroon3r commented 4 years ago

CC_TEST_REPORTER_ID: Secret or Not Secret

When this project was started, developers involved started following the examples for creating a git action to work with Jacoco test reporter found on this repository. Here, the repository contributors list the CC_TEST_REPORTER_ID as a secret specific to the repository that is being tested.

We interpreted that this value should be secret for a reason. After extensive research, we came back to Code Climate's own documentation that tells users that

Your repo's test coverage ID is only used to identify your repo when submitting test coverage payloads. It can't be used to access any of your data.

and therefore, has no potential risk to using as an environment variable in your Git Action.

akroon3r commented 4 years ago

Translating Jacoco XML test cover reports

This Java project implements interfaces such as the fee service.

Fee Service Interface:

package ca.bc.gov.open.jag.efilingapi.fee;

import ca.bc.gov.open.jag.efilingapi.fee.models.Fee;
import ca.bc.gov.open.jag.efilingapi.fee.models.FeeRequest;

/**
 * A FeeService interface that provides services
 */
public interface FeeService {

    Fee getFee(FeeRequest feeRequest);

}

When mvn -B verify -P all is run from the src/backend directory, Jacoco test coverage reports are generated for each service inside the backend. This is a sample of the XML generated:

   <package name="ca/bc/gov/open/jag/efilingapi/fee">
        <class name="ca/bc/gov/open/jag/efilingapi/fee/FeeService" sourcefilename="FeeService.java" />
        <class name="ca/bc/gov/open/jag/efilingapi/fee/MockFeeService" sourcefilename="MockFeeService.java">
            <method name="&lt;init&gt;" desc="()V" line="8">
                <counter type="INSTRUCTION" missed="0" covered="3" />
                <counter type="LINE" missed="0" covered="1" />
                <counter type="COMPLEXITY" missed="0" covered="1" />
                <counter type="METHOD" missed="0" covered="1" />
            </method>
            <method name="getFee" desc="(Lca/bc/gov/open/jag/efilingapi/fee/models/FeeRequest;)Lca/bc/gov/open/jag/efilingapi/fee/models/Fee;" line="12">
                <counter type="INSTRUCTION" missed="0" covered="8" />
                <counter type="LINE" missed="0" covered="1" />
                <counter type="COMPLEXITY" missed="0" covered="1" />
                <counter type="METHOD" missed="0" covered="1" />
            </method>
            <counter type="INSTRUCTION" missed="0" covered="11" />
            <counter type="LINE" missed="0" covered="2" />
            <counter type="COMPLEXITY" missed="0" covered="2" />
            <counter type="METHOD" missed="0" covered="2" />
            <counter type="CLASS" missed="0" covered="1" />
        </class>
        <sourcefile name="MockFeeService.java">
            <line nr="8" mi="0" ci="3" mb="0" cb="0" />
            <line nr="12" mi="0" ci="8" mb="0" cb="0" />
            <counter type="INSTRUCTION" missed="0" covered="11" />
            <counter type="LINE" missed="0" covered="2" />
            <counter type="COMPLEXITY" missed="0" covered="2" />
            <counter type="METHOD" missed="0" covered="2" />
            <counter type="CLASS" missed="0" covered="1" />
        </sourcefile>
        <sourcefile name="FeeService.java" />
        <counter type="INSTRUCTION" missed="0" covered="11" />
        <counter type="LINE" missed="0" covered="2" />
        <counter type="COMPLEXITY" missed="0" covered="2" />
        <counter type="METHOD" missed="0" covered="2" />
        <counter type="CLASS" missed="0" covered="1" />
    </package>

You can see that Jacoco generates <class name="ca/bc/gov/open/jag/efilingapi/fee/FeeService" sourcefilename="FeeService.java" /> for the Fee Service Interface, whereas the files inside this package generate lines and counter type nested inside. This would lead someone to believe that Jacoco is not considering an interface to be a testable object.

The code climate repository uses the following golang file as a test for jacoco report coverage. Nowhere in this file is there an example of an empty class like the Fee Service interface seen above.

This project uses the following step in its Git Action to format ALL Jacoco test coverage reports after they've been generated into a JSON file that is required for code climate to consume the test coverage reports:

# Formatting the BACKEND coverage reports generated (dynamically)
      - name: Format BACKEND coverage reports
        run: |
          projectRelRegex="^\.\/src\/backend\/(.*)\/target\/site\/jacoco\/jacoco\.xml$"
          for file in $(find . -name "jacoco.xml")
          do
              echo $file
              echo $projectRelRegex
              if [[ $file =~ $projectRelRegex ]]
              then
                  projectRel="${BASH_REMATCH[1]}"
                  echo "analyzing project: " $projectRel
                  projectName="${projectRel//\//-}"
                  JACOCO_SOURCE_PATH=${{ github.workspace }}/src/backend/$projectRel/src/main/java ./cc-test-reporter format-coverage ${{github.workspace}}/src/backend/$projectRel/target/site/jacoco/jacoco.xml --input-type jacoco --output coverage/$projectName-codeclimate.json;
                  echo "coverage generated: coverage/$projectName-codeclimate.json;"
              else
                  echo $file does not match
              fi
          done

This JSON file contains the following object for the Fee Service interface:

"coverage": "[]",
      "covered_percent": 0,
      "covered_strength": 0,
      "line_counts": {
        "missed": 0,
        "covered": 0,
        "total": 0
      },

This tells code climate that there are line counts with a defined value (0) when they should be empty or null.

The developer team has opening the following issue with the code climate repository to track this bug.

akroon3r commented 4 years ago

Repo Coverage Badge vs. Codebase Summary Coverage

badge1 badge2

We think that this is a rounding bug ie// floor vs ceiling?

Haven't reported this yet.

akroon3r commented 4 years ago

Multi Module Java Project Reporting

Initially developers were using the aggregated Jacoco XML report found in src/backend/coverage-report/target/site/jacoco-aggregate/jacoco.xml. Developers later found that Code Climate has an issue with Jacoco reports that contain group tags ie// multi module java projects.

The git action uses a for loop to loop through all services in the backend and format the service test coverage report into a coverage directory. Afterwards the code climate test reporter binary is used to sum all formatted coverage reports into one "master" report before being sent to Code Climate for consumption. Debugging this issue took a bit of time before finding that note specifically for jacoco reports.

sdevalapurkar-bcgov commented 4 years ago

potential con: coverage issue where it seems like the coverage is not normalized. If n lines of code are removed, then the test coverage can go down as a result even though this should not be the case.