We run code analyses on Pull Requests with following setup:
- SonarQube 8.3
- Jenkins Pipelines
- Gradle SonarScanner plugin 2.7
While code analyses on PR works fine in most of the cases, we do experience issues in specific situations. There are some cases where the Quality gate would pass for a PR analyses and fail when PR is merged into a branch. What we noticed is that on PR analyses some Issues are not registered/created though looking at specific code, it is visible that an issue is highlighted.
For example we experience following when new Duplicated Code is introduced:
- No Issue is being registered on PR analyses, when according to the Quality Profile there should be one with Major severity.
- Anyway Duplicated Blocks are registered and code blocks are marked with grey.
- Percents of Duplicated Code are increased as well, which means specific rule has been taken into account, but still in this case Quality Gate is green.
- Then when code is merged into the main branch, new Major issue is being registered and Quality Gate fails as expected.
My understanding is that Quality Profile is inherited from the parent project for both, PR and BRANCH analyses. So if Quality Gate is Green on PR, it should be green on BRANCH when code is merged.
I believe what you’re seeing is due to the way the issue is actually tied to the source code. Usually a duplication issue is associated with the original block of code, not the place where it was repeated (in case it’s repeated multiple times).
On a PR, only issues directly tied to new/changed code are reported, so you don’t see the duplication issue at this point, only when the code is merged.
I can see why this isn’t ideal in your case. But I also have to ask, why is a single major code smell issue causing your quality gate to fail on the main branch? We recommend (and include in the default quality gate) only factoring in code smells via the maintainability rating. So that code smell issue would simply be contributing a little bit of extra tech debt density to potentially nudge the maintainability rating towards failure, but wouldn’t directly cause a failure. If you’re simply including something like “Major issues > 0” as a criterion, in my opinion that’s overly strict.
Hope this helps!
Thanks Jeff for your response!
The rule with the duplicated blocks could be reproduced on a new code. I did it with introducing two new duplicated blocks on a PR and an issue was not registered, but I could see the Maintainability rate change.
And this is not the only rule that is not picked up on PR analyses, there is the “8 param on a method” rule for example.
When you add a new param on a new line, this is not picked up by the PR analyses, only when code is merged and this does not affect the Maintainability rate on the PR.
What I’m trying to understand is why the number of Issues would change after a merge. I would expect that the Issue number on a Branch will not be changed if PR analyses hasn’t registered any new Issues.
Should an Issue be created on a PR for the above mentioned examples? While the above mentioned cases are Code Smells, I think, are there any Bugs that will not be picked up on a PR?
Thanks for reporting these problems.
Deciding what issues to show in a P/R is challenging because we need to guess what issues are relevant for the code that was changed. The problem is establishing cause and effect between a line changed and an issue.
In v8.3 we only report for P/Rs issues that are strictly located in a changed line of code. We know that in many cases this strategy leads to false negatives (as you experience) and also false positives.
We have plans to improve the strategy before the next LTS, which include comparing the issues between the P/R and its target. However, to be honest it will probably never be 100% effective in keeping issues out of the target branch.
I think this should not be a big problem for now, unless the only issues that cannot be registered are Code Smells. If this happens with Bugs as well, we probably will have to use BRANCH analyses for feature branches too. For now as Jeff suggested, to avoid inconsistency in the statuses between PR and BRANCH analyses, we may make the Quality Gate conditions less strict and use something like “Maintainability Rate is worse than A” instead of “Major Issues > 0”.
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.