[BUILD-1030] New look at tooling for assessing quality of PRs Created: 10/Mar/23  Updated: 28/Mar/23  Resolved: 28/Mar/23

Status: Closed
Project: Build
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Neutral
Reporter: Maxime Michel Assignee: Maxime Michel
Resolution: Done Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screenshot 2023-03-20 at 15.11.46.png     PNG File Screenshot 2023-03-22 at 10.40.41.png     PNG File Screenshot 2023-03-22 at 10.40.51.png     PNG File Screenshot 2023-03-28 at 12.42.14.png    
Issue Links:
relation
is related to MGNLTEST-215 Setup SonarQube instance with fargate Closed
is related to MGNLTEST-213 Enable SonarQube to use (on some) "co... Closed
Template:
Acceptance criteria:
Empty
Task DoR:
Empty
Team: Foundation

 Description   

As we nowdays automatically deploy to the SaaS anything we merge to master, we need to better check the quality of the code. We can't enforce something like a JaCoCo threshold due to amount of untested code. What we're looking for instead is PR-level validation. Here are a couple of options:

bitbucket-code-coverage

The easiest to roll out solution I've seen. Using JaCoCo reports as material, it annotates the code to highlight tested/untested lines of code. This only requires a Bitbucket server plugin and running the following commands:

mvn clean verify -Penable-code-coverage jacoco:report

mvn com.atlassian.bitbucket:code-coverage-maven-plugin:publish \
    -Dbitbucket.url=https://git.magnolia-cms.com \
    -Dbitbucket.user=mmichel \
    -Dbitbucket.password=$LDAP_PSWD \
    -Dbitbucket.commit.id=8e3839923cb1434dcf320e1c9115676d206ef081 \
    -Dcoverage.file=magnolia-core/target/site/jacoco/jacoco.xml \
    -Dcoverage.format=JACOCO

Source: https://bitbucket.org/atlassian/bitbucket-code-coverage/src/master/

Problem: this isn't helpful in the sense that it doesn't validate the PR with a yes/no answer to the question of whether the quality is sufficient. As agreed to in the March 21st' architecture meeting, it's anyway another tool in the toolbox, we might as well give it a try.

Bitbucket code insights

Already built into Bitbucket is an API to which we can post checks of any nature about the code: https://confluence.atlassian.com/bitbucketserver/code-insights-966660485.html

This is (I assume) what is used by professional tools such as Sonarqube. However, because it's a REST API, nothing prevents us from pushing simpler reports/metrics into it: test coverage, Snyk report, style, etc.

The downside is that it will require inhouse development work & maintenance. It will also never match Sonarqube's own insights.

jacoco-report GitHub action

Using the Jacoco data and a bit of tinkering, as well as the Bitbucket code insights, it is possible to calculate the coverage for the PR's changed files. This coverage can then be compared to a limit, and fail the PR if deemed insufficient.

See https://github.com/Madrapps/jacoco-report as well as attached screenshot for Bitbucket Server rendering

curl -u mmichel:$LDAP_PSWD --request PUT 'https://git.magnolia-cms.com/rest/insights/latest/projects/PLATFORM/repos/main/commits/8e3839923cb1434dcf320e1c9115676d206ef081/reports/test-report-1' \
--header 'Content-Type: application/json' \
--data-raw '{
    "title": "Coverage PR quality check",
    "details": "This checks evaluates whether coverage on changed files is sufficient.",
    "result": "FAIL",
    "data": [
        { "title": "Number of analysed files", "type": "NUMBER", "value": 1 },
        { "title": "Coverage on changed files", "type": "PERCENTAGE", "value": 80 },
        { "title": "Safe to merge?", "type": "BOOLEAN", "value": false }
    ],
    "reporter": "Foundation"
}'

Sonarqube

We have already looked into rolling Sonarqube. It's likely a popular name on the market for a good reason, however, we've found it expensive per line of code. Also, it will need to be adapted to our code style.

  • would the price be lower if we ran it ourselves? As we use Bitbucket server, we have no choice but to deploy it ourselves, actually.
  • can it run exclusively on pull requests, dramatically lowering the lines of code it's scanning, vastly reducing the price? Yes, but no impact pricing.
  • that being said I have pulled our most critical repositories (as defined by: is a dependency of SaaS webapp) and the total lines of java code valid for Sonar is 230k, which is only $100 a month. We would likely start with the $12/month plan, but, even if we scaled, it would be totally acceptable (assuming developers like the added value)

Conclusion: we roll out SonarQube.



 Comments   
Comment by Maxime Michel [ 22/Mar/23 ]

Just added two screenshots to show another type of check running on some PRs that I found out by chance.

Comment by Maxime Michel [ 28/Mar/23 ]

Just added a screenshot of the type of enforcing Sonar will do on PRs. It's effectively enforcing coverage (without us having to do the implementation described above) but at the same time also evaluating security, technical debt, etc. It's great!

Comment by Maxime Michel [ 28/Mar/23 ]

Going with SonarQube makes the most sense. See linked tickets for progress.

Generated at Sun Feb 11 23:47:30 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.