PR analysis shows "for merge into master", but base is Staging

SonarQube
1

We have pull requests getting blocked by SonarQube. The analysis includes issues that are not distinct from the base branch.

The comments to GitHub are correct (a single issue introduced by the PR), but SonarQube posts a failed commit check with 33 open issues. When I visit SonarQube the header says for merge into master from PR-1234 and the issues include 32 issues introduced by Staging relative to master, not just the single issue the PR introduced.

Jenkins is running sonar-scanner with the following arguments:

-Dsonar.projectKey=example
-Dsonar.projectName=Example/example
-Dsonar.pullrequest.branch=PR-1234
-Dsonar.pullrequest.key=1234
-Dsonar.pullrequest.base=Staging
-Dsonar.pullrequest.provider=github
-Dsonar.pullrequest.github.repository=Example/example
-Dsonar.pullrequest.github.endpoint=https://github.example.com/api/v3/

How do I get the PR analysis to correctly analyze these PRs relative to their base branch and not master?

2

Hi,

You don’t make it clear whether your base branch is a short-lived or long-lived branch. From the name, and based on the default regex that determines these things, I’ll guess short-lived, and say that that’s the root of the problem, it’s on our radar, and we hope to address it in 7.6 (current ETA 2019Q1)

  • MMF-1375: Support PR targeting short living branches

 
Ann

1 Like
3

Thank you. That’s what I expected. I updated sonar.branch.longLivedBranches.regex to include Staging, deleted the Staging branch in SonarQube, and started a new “first” analysis of Staging. I’ll see what happens.

4

Marking Staging a long-lived branch, deleting the Staging branch in Sonar, re-scanning Staging, and then re-scanning PR-1234 didn’t fully work.

PR-1234 is showing as for merge into master from PR-1234 which is good, but is still showing issues originating from master. I’m going to try deleting the short-lived PR branch next.

5

To clarify. Staging was a short-lived branch.

Deleting the PR-1234 branch and re-scanning did not fix the issue. It is still showing the issues between Staging and master.

Next thing I am trying is disabling shallow clone in Jenkins and re-scanning PR-1234.

6

I’m having no luck with this. Even with a full clone it shows issues not introduced by the PR.

What happens if sonar.pullrequest.base (Staging) has commits that sonar.pullrequest.branch (PR-1234) doesn’t have?

Staging:
A => B => D

PR-1234:
A => B => C

Will PR-1234’s scan show issues from the diff between C and D? (that would undesirable to me)

7

Hi,

I believe at least part of your problem will be addressed by 7.4:

MMF-1265 - “New code” on branches should start from the branch point

 
Ann

1 Like
8

Confirmed. I created a pull request based on an older commit in Staging and the analysis of my PR showed results for the intervening commits just as you have described in the JIRA issue.

I’m really looking forward to 7.4. We aren’t fully in production yet. Is there a way I can beta test 7.4? Would that be useful to you folks?

9

Hi,

Thanks for the testing offer, but we don’t put out betas anymore (we’re moving toward continuous deployment on SonarCloud, so our features get put through their paces pretty heavily there). On the plus side though, we anticipate releasing/announcing 7.4 on Monday.

So… now you have a reason to look forward to a Monday?

 
:smile:
Ann

1 Like
10

LOL. Indeed. I’m pretty excited about that release actually, so definitely an exciting way to kick off the week :slight_smile:

In 7.3, rebasing the PR on the latest scanned version of Staging cleans up the issues and shows only those relevant to the PR.

Thank you very much for your detailed guidance and information on this.

11

I did another experiment. This approach is less invasive as it uses merge and push fast forward, rather than rebase + force push so people may be more comfortable with it.

In 7.3, merging in the latest scanned commit of Staging into the PR branchcleans up the issues and shows only those relevant to the PR.

1 Like
12

@ganncamp did MMF-1265 ship with the 7.4? I’m still getting scan behavior that suggests that it is using the latest scan and not the scan nearest the commit from which the PR descends.

13

Hi,

It did, but I should point out that (part of) what shipped in SQ was the API to allow the SCM plugin to provide the necessary data to make this work. The updated Git and SVN plugins were included in the 7.4 bundles, but if you didn’t take them for some reason or if you’re using a different SCM then you won’t see the behavior.

If none of those caveats are relevant, would you open a new thread with the details, please?

 
Thx,
Ann

1 Like