Using new code "since previous version" instead of "since BASELINE"

sonarqube

(Kevin Condon) #1

Using SonarQube 7.4 Developer and doing analysis with sonar-maven-plugin 3.5.0.1254

Summary

New code is missing from the first analysis done after an initial BASELINE analysis. Doing another analysis appears to fix this. This always happens. This makes it easy to miss quality problems introduced by code changes.

Steps to reproduce

  1. Start with an existing project with New Code Period set to BASELINE.
  2. Do initial analysis for a new long-lived branch with mvn clean install sonar:sonar -Dsonar.branch.name=MYBRANCH -Dsonar.projectVersion=BASELINE
  3. Make a code change.
  4. Do analysis again for the same branch, this time allowing sonar.projectVersion to be set by the plugin from the POM version with mvn clean install sonar:sonar -Dsonar.branch.name=MYBRANCH

At this point the branch’s Activity shows correctly the 2 analyses that have been done, but the Overview indicates that the new code is based on the previous version instead of the BASELINE version.

The new code doesn’t appear anywhere in the Measures; however, if I drill down into the Code, I do see the line that I added. So the new code was analyzed, but the analysis does not surface it in the SQ dashboard.

If I make another code change and repeat mvn clean install sonar:sonar -Dsonar.branch.name=MYBRANCH, the new code period will be “since BASELINE” and the new code will appear in the various measures, etc.


(Julien Lancelot) #2

Hi Kevin,

Thanks for reporting this bug, we’re able to reproduce and I’ve created the ticket https://jira.sonarsource.com/browse/SONAR-11492 to fix it.

Regards


(Kevin Condon) #3

Thank you!


(Kevin Condon) #4

Hi Julien,

One thing missing from the bug you filed is that the new code is missing from the second analysis. That breaks our ability to detect reduced coverage, smells, etc. I didn’t explicitly test for it, but it probably breaks the quality gates. It definitely fools the Bitbucket plugin merge checks, allowing code with reduced quality metrics to be merged. For us that’s more than a minor issue. Just doing a 3rd analysis isn’t enough to fix it; the only workaround is to make a meaningless code change and then run the 3rd analysis. That’s not something I want to try to automate.

Regards,
Kevin


(Julien Lancelot) #5

Hi Kevin,

We were not able to reproduce the fact that the new code is missing from second analysis.
Could you share some screenshots of what is happening ?

Thanks


(Kevin Condon) #6

The analysis shows no new code, even though there was.

No new lines to cover appear in the measures tab, but if I drill into the code tab I see the changed files; however, those changed lines don’t have the normal yellow highlighting.

I would post a series of pictures, but new users can post only 1 picture/post, and I don’t have time to tell the picture story across 5 or 6 posts.


(G Ann Campbell) #7

Hi,

In fact I saw this effect when I analyzed changes made locally and not checked in. As soon as I committed, Coverage and Duplications on New Code were calculated. Could that be what’s going on?

 
Ann


(Kevin Condon) #8

Hi Ann,

This auto-triggered when a PR was created in Bitbucket, so there was never a local build. Our Jenkins build job first runs the BASELINE analysis from the branch targeted by the PR, then it runs the analysis on the code in the PR branch, resulting in 2 analyses that don’t surface the new code in the dashboard.

Kevin


(G Ann Campbell) #9

Hi Kevin,

You shouldn’t need (anymore) to run that baseline analysis. We did some work in 7.4 to correctly detect what’s “new”. Also, you’re running a Long-Lived Branch analysis for each PR? You realize that’s going to be a lot of cleanup (or scripting), right?

 
Ann


(Kevin Condon) #10

Hi Ann,

It’s possible that our approach has been colored by earlier SQ versions. We use long-lived branches for PRs because the short-lived branches don’t prevent code coverage deficiencies in new code. I don’t think that changes with SQ 7.4, but I think you’re saying that we don’t need to build a separate BASELINE version in every PR long-lived branch, right?

Our leak period is set to BASELINE and every time we merge to our main branch the build creates a new BASELINE version analysis. So…

  • When we create a new PR branch with analysis version n, the initial leak period will be against the latest BASELINE version in the main branch?
  • Then if we make changes in the PR branch with analysis version n+1, the new leak period will be between BASELINE (as it was when the PR branch initial analysis was done) and n+1?

If the above is correct, it would definitely streamline our PR builds.

After the PR is approved, we use the BB plugin merge gate to block reduction in quality metrics. I don’t think that would be impacted by the above practice.

Am I understanding this correctly?

Thanks,
Kevin


(G Ann Campbell) #11

Hi Kevin,

Well, in the context of PRs, SLBs, that’s what I’m saying. in the context of using LLBs instead… I guess you probably still want to set the baseline in your LLB so you can easily tell what issues got added in the New Code Period (it got renamed in 7.4)

And you’re right about coverage not counting in the state of PRs/SLBs yet. We hope to get real QGs in in 7.6.

 
Ann


(Kevin Condon) #12

Hi Ann,

Yeah, if only SLBs had code coverage. :slight_smile:

That brings things back to my original issue then. Julien said he could reproduce that the new code period doesn’t show as since BASELINE in the dashboard. But he could not reproduce the issue I see with the new code missing from the analysis.

Does this discussion clear up anything so that you can reproduce that part of my issue?

Thanks,
Kevin


(G Ann Campbell) #13

Hi Kevin,

Actually, having re-read previous messages, I’m now thoroughly confused.

Every time you merge to master, all your PR LLBs are updated with a new ‘BASELINE’ analysis of master? Meaning you have multiple, non-consecutive analyses in your LLB history with a ‘BASELINE’ version string?

 
Ann


(Kevin Condon) #14

Hi Ann,

Oh, no, we’re not quite that crazy. :crazy_face:

  • Each merge to our master branch results in just a single analysis that creates a new BASELINE version in the master branch project.
  • Creating a new PR LLB results in 2 analyses, both in the PR branch project – first a BASELINE analysis from rebuilding the master branch code, then a new version analysis from rebuilding the PR branch code. We never do another BASELINE analysis after that first one in a PR branch.
  • Each additional commit to a PR branch results in a new version analysis in the PR branch project from building the code for that commit.

Kevin


(G Ann Campbell) #15

Hi Kevin,

I’ve managed to reproduce this. My procedure:

  • analyze project, update version string via Activity UI to BASELINE
  • set project’s New Code period to BASELINE
  • git branch test
  • git checkout test #yes, I know there’s a one-liner. Didn’t feel like looking it up
  • make a change.
  • git commit -a -m"test"
  • git checkout master
  • mvn clean install sonar:sonar -Dsonar.branch.name=release-foo -Dsonar.projectVersion=BASELINE
  • git checkout test
  • mvn clean install sonar:sonar -Dsonar.branch.name=release-foo

I can see my changed line when I drill into the file in the UI but it’s not highlighted as new, and the New Lines measure is 0.

The critical part seems to be the checkouts. When I ran through this without actually switching branches, my new code was detected.

 
Ann


(Kevin Condon) #17

Hi Ann,

Glad it’s reproducible. Will you add that to https://jira.sonarsource.com/browse/SONAR-11492, or is it a separate bug to file?

I can’t think of any reasonable way to leverage your don’t-switch-branches-between-analyses observation into a workaround for our automated SQ analysis in Jenkins.

Kevin


(G Ann Campbell) #18

Hi Kevin,

I didn’t mean it to be one. Just trying to thoroughly document reproduction conditions.

I’ll have to come back to you on that.

 
Ann


(G Ann Campbell) #19

Hi,

I haven’t forgotten this…

 
Ann


(G Ann Campbell) #20

Hi,

This is still on the list. Sorry for the delay.

 
Ann