We are trying to analyze our C# code (GitHub repo) with the SonarQube Developer Edition 8.9, sonar-scanner-msbuild-5.2.1.31210-net46, SonarQube Scanner Jenkins plugin v. 2.13 using Multibranch Pipeline Jenkins job.
We are interested in the branch and PR analysis features.
We have an issue with PR analysis though. For some PRs, the analysis reports issues that are not introduced with that PR (sometimes those changes are not even merged to develop yet, they are introduced in another PR that is still open). I am not sure if it is related to the New Code definition (we use the Previous Version). I assume the previous version means the previous version of that particular PR?
I followed the documentation to set it up.
I also searched in the community forum and found that others had/have a similar issue, i.e. that the combination of branch analysis and leak period is not producing the correct output. This example represents quite close our issue: https://community.sonarsource.com/t/new-code-has-commits-changes-from-master/20215, but there is no answer there.
Also, could you please explain how the PR analysis supposed to work. Does it consider as a new code for a particular PR only the code added in that PR as compared to the develop branch at the moment that PR’s feature branch was checked out? In other words, only the changes that we see in GitHub for that PR?
Our sense of what is new code on a PR branch is: what code is involved if a git diff is performed between the PR branch (what is currently checked out) and declared target branch. This means that
Your workspace state should be a clean checkout of the PR branch at its head revision. Not a merge preview or anything like that
The ref of the target branch should also be in the git workspace
Are you using the GitHub Branch Source plugin for Jenkins as we recommend? It should make this easy. When you specify how to discover Pull Requests, make sure the setting “The current PR revision” is chosen. Additionally you’ll see an option to specify additional behaviors at checkout and can add the behavior to include all refs for remote branches.
Hi Jeff, thanks!
Yes, we are using the GitHub Branch Source plugin for Jenkins. One thing we did not do is to add the behavior to include all refs for remote branches. I will try that.
Just to make it clear for me, if there was another PR merged into the develop after I checked out my branch, and so, those changes are not in my branch, will that affect what would be considered as a new code for my PR in that case?
If the changes landed in develop already and may be there in the cloned ref at the time this PR is analyzed, it could lead to some unpredictable results. What I’d expect is that the change be merged up into the PR branch prior to it being pushed out/analyzed. That’s always safest prior to putting a PR out for merge/review anyway so you can best attest as a developer to the impact this branch will have on the true state of the code post-merge.
Yes, we could always merge develop into the PR branch prior to pushing it to remote. It’s something we do if we expect some merge conflicts. But, 95% of the time, we don’t need to.
But even if we do that, since many developers work on the repository, it happens often that a feature is merged into develop in between, i.e. just after you merge develop into your branch but before you push it to the remote.
Is there a way to define the new code for a PR as a diff from its parent on develop (i.e. the commit from where the branch was checked out)? Like in GitHub, where we see the diff only with the parent, not with the current state of the develop branch.
I believe we’d do a diff with the current state of develop, unless you took care to make sure the local ref to develop was anchored to that specific earlier commit hash. The more I think about it, though, I don’t think this would show issues related to code modified in develop but not present in the PR.
I want to revisit your original problem description, though:
This really shouldn’t happen if you take care to have the PR branch cleanly checked out at its head revision at analysis time. In fact I can’t imagine why this would happen unless a developer had deliberately cross-merged one PR branch into another…in which case I’d argue SonarQube showing you all the combined changes is technically correct.
Since you already noted the need to make sure the ref to the target branch was included in the clone prior to analysis, I suggest we reset the discussion: please try to ensure that’s the case and let us know if you find a reproducible problem once you’ve made that change.
Hi Jeff,
we specified ref specs for the PRs, and so far there were no issues.
Now, we have another issue with the develop branch. Somehow the new code is not detected there. The SonarQube UI always shows 0 New Lines. Interesting that there is no such problem for the release branches (so far). Is it better to create a new post for this issue?