SonarQube is a static source code analysis tool. It can quickly flag common developer mistakes and help to enforce language specific best practices. SonarQube leverages reports generated by other build tools such as Jacoco for analyzing test coverage, and is executed during our build process via the Gradle Sonarqube Plugin. Besu has integrated these scans into our CI process to provide developers with reporting on their PRs, and encourage them to maintain the metrics we deem important.
How it fits into CI
As of this writing, SonarQube results do not affect the ability to merge any submitted PR. Although a failed check will be displayed on the PR, it is not gated from being merged. We encourage devs to make sure these scans pass, but we do not require it at this point. SonarQube scans were introduced in Q3 of 2021, and the development team is still learning how they can best be leveraged by our CI process.
SonarQube is currently being run on all PRs opened into the repository. Results are listed as comparisons
The Besu Way
SonarQube ships with a default collection of rules that reflect its opinion on what Java programming best practices are. This is referred to by Sonar as "The Sonar Way", and is a regularly updated reflection of the very broad Java software development industry. The developers maintaining Besu, have started with this ruleset and then removed any rules that they disagree with. The key differences are:
- Disabling code duplication warnings. Duplication of code is sometimes acceptable when it leads to performance improvements in Java. There are a few cases in Besu, particularly in the EVM subsystem, where repeated code is faster than it would be if it were reusable via method calls.
- Code coverage requirements set to 80%.
- Removal of some Android specific rules which cause false positives.
Code Coverage
Besu uses jacoco to calculate the coverage of our unit tests. Currently, our integration and reference test targets which also run on CI, are not contributing to the coverage statistics. Coverage numbers are not always objective, and there are perfectly good reasons not to write unit tests for code like getters and setters. The Besu team encourages devs to strive for maximal coverage where it makes sense, but not to fail a build that doesn't hit 80% coverage.
IDE Plugins
IntelliJ is an IDE commonly used by Besu developers, and it also has a SonarQube plugin available for it. With this plugin, you can run the rulesets configured from your Sonarcloud instance, right in your IDE. This is a great way to make sure you are cleaning things up as you go, and helps avoid surprise reports after pushing your branch to a pull request.
Outstanding Issues
- Coverage on non-unit tests: Unit tests are great, but often integration or system tests are more efficient. Tests against a full system are more accurate to the real world since the developer cannot mock out any dependencies. It would be extremely nice to calculate code coverage for these higher level tests.
- Only run on PRs into main: Right now, sonarqube analysis will happen on every push to every branch which has an open pull request, regardless of the branch it is targeting. This is nice since it makes developers aware of issues earlier. This is not nice since it is time consuming and costs compute resources in the CI cluster. It is possible that early analysis for developers can be accomplished using IDE tooling.
- Only run nightly: high level metrics such as maintainability and complexity should not be so volatile that they emit a useful signal more than once a day or so. It may be reasonable to only run this analysis when preparing a nightly build.
- Leak periods: the definition of "new" code is lines of code modified since the last release was run. Is that the right timeframe to examine?