Pull request analysis displays "0 new lines" for all PRs and shows no coverage information

Looking at the git-clone step of the workflow, I see the following commands executed:

$ git "init"
Initialized empty Git repository in /bitrise/src/.git/
$ git "remote" "add" "origin" "git@bitbucket.org:shiftkeyllc/shiftkey-[REDACTED].git"
$ git "fetch" "--jobs=10" "--no-tags" "origin" "refs/heads/master"
Warning: Permanently added '[bitbucket.org](http://bitbucket.org),104.192.141.1' (RSA) to the list of known hosts.
From bitbucket.org:shiftkeyllc/shiftkey-[REDACTED]
* branch master -> FETCH_HEAD
* [new branch] master -> origin/master
$ git "checkout" "master"
Already on 'master'
Branch 'master' set up to track remote branch 'master' from 'origin'.
$ git "apply" "--index" "/tmp/FileProviderprovider2778758927/diff.txt"
**$ git "checkout" "--detach"**

M bitrise.yml
M build.gradle
M gradle-scripts/jacoco_module.gradle
M provider/src/[REDACTED]Test/java/com/shiftkey/provider/ui/explore/ExploreShiftsScreenTest.kt
M provider/src/dev/java/com/shiftkey/provider/dev/DevToolsViewModel.kt
M provider/src/main/java/com/shiftkey/provider/data/model/debug/DebugScenario.kt
M provider/src/main/java/com/shiftkey/provider/data/model/notifications/AlertData.kt
M provider/src/main/java/com/shiftkey/provider/data/model/shift/Shift.kt
M provider/src/main/java/com/shiftkey/provider/ui/alerts/compose/AlertsComposables.kt
M provider/src/main/java/com/shiftkey/provider/ui/earnings/EarningsShiftsViewModel.kt
M provider/src/main/java/com/shiftkey/provider/ui/earnings/compose/EarningsExpandableShiftCard.kt
M provider/src/main/java/com/shiftkey/provider/ui/earnings/compose/EarningsShiftsComposables.kt
M provider/src/main/java/com/shiftkey/provider/ui/earnings/fragments/EarningsShiftsFragment.kt
M provider/src/main/java/com/shiftkey/provider/ui/earnings/items/EarningsShiftItem.kt
M provider/src/main/java/com/shiftkey/provider/ui/explore/ExploreViewModel.kt
M provider/src/main/java/com/shiftkey/provider/ui/explore/compose/ExploreScreen.kt
M provider/src/main/java/com/shiftkey/provider/ui/explore/compose/ExploreShiftsList.kt
M provider/src/main/java/com/shiftkey/provider/ui/shift/detail/ShiftDetailViewModel.kt
M provider/src/main/java/com/shiftkey/provider/ui/shift/highlights/ShiftHighlightsFragment.kt
M provider/src/test/java/com/shiftkey/provider/networking/local/DebugApisLocalTest.kt
M provider/src/test/java/com/shiftkey/provider/networking/local/ShiftApisLocalTest.kt
M provider/src/test/java/com/shiftkey/provider/ui/shift/highlights/ShiftHighlightsViewModelTest.kt
HEAD is now at cbafaca6 Merged in PRO-1433 (pull request #692)
$ git "submodule" "update" "--init" "--recursive" "--jobs=10"

Are you suggesting that the “–detach” option to not be used on the git checkout step?

I am trying to understand how the diff list is calculated by the scanner step and how the “–detach” option is the possible issue.

Hi,

I’m suggesting there’s a whole lotta stuff there that goes beyond a vanilla checkout. Can you just try it with a vanilla checkout of the PR’s branch?

 
Ann

Ok, I just tried that and please find the logs attached. The git-clone step, now does just these commands:

$ git "init"
Initialized empty Git repository in /bitrise/src/.git/
$ git "remote" "add" "origin" "git@bitbucket.org:shiftkeyllc/shiftkey-[REDACTED].git"
$ git "fetch" "--jobs=10" "--no-tags" "origin" "refs/heads/PRO-1408-Sonar-Test"
Warning: Permanently added 'bitbucket.org,104.192.141.1' (RSA) to the list of known hosts.
From bitbucket.org:shiftkeyllc/shiftkey-[REDACTED]
 * branch              PRO-1408-Sonar-Test -> FETCH_HEAD
 * [new branch]        PRO-1408-Sonar-Test -> origin/PRO-1408-Sonar-Test
$ git "checkout" "bb6c10cfeac1"
Note: switching to 'bb6c10cfeac1'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
  git switch -c <new-branch-name>
Or undo this operation with:
  git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at bb6c10cf ADHOC - Few more tweaks
$ git "submodule" "update" "--init" "--recursive" "--jobs=10"

apps_b100f922f79ced7c_82842da1-0fff-4802-8782-ee584b5399b4-full.txt (7.1 MB)

Hi,

That doesn’t look like a vanilla branch checkout to me… Rather than checking out the commit(?), can you check out the branch?

 
Ann

Good morning Ann,

Was able to crack this finally. Couple of things I did on my Bitrise workflows:

  1. “git-clone” step was by default merging the src branch head commit into master and I could parameterize it and skip that step.
  2. When I disabled the merge on the “git-clone” step, the step did not even pull down the master. So I added an additional script step to pull down the “master” branch after the “git-clone” step.

After these two changes, I am able to see “new lines” on SonarCloud and also the pull request comment on bitbucket shows the same information.

https://sonarcloud.io/summary/new_code?id=shiftkeyllc_shiftkey-android&pullRequest=706

Can I ask you a question about coverage and quality gate. Is there a way to set up a dynamic incremental coverage target as opposed to a static percentage. For e.g. I would like to set a rule that every new pull request/merge into the master branch maintains the previous coverage percentage or improves it. This way, we do not have to go and keep updating our targets.

Thanks a lot for all the help so far.

1 Like

Hi,

I’m glad you worked through this!

Excellent question! I call this “ratcheting QG conditions”, and… no we don’t have that. I’ve been badering our Product Managers for this for several years now and I’ll use this as additional ammunition.

BTW, you’re not going to be able to enforce a QG condition for overall coverage on a PR. Only conditions on New Code are applied in PRs. :woman_shrugging:

 
Ann

Oh, thats too bad. The absence of the “Ratcheting QG conditions” is tolerable. But unable to enforce QG for overall coverage on a PR, I would think, is something everyone would want?

Without that, its quite possible that the overall code coverage can drop and that should be part of the quality gate, is it not?

Hi,

Well… Assuming you’re enforcing coverage on new code, and assuming you’re keeping up with updating your conditions (in the absence of an automatic ratchet mechanism) then theoretically not…? Or only in very pointy corner cases…?

Let’s assume:

  • current coverage overall 87%
  • current QG condition: overall: 87% (not enforced on PRs)
  • current QG condition: New Code: 87%

Any PR would have to have >= 87% coverage to pass the quality gate & so overall coverage should naturally increase over time.

But let’s go a step further:

  • current coverage overall 87%
  • current QG condition: overall: 87% (not enforced on PRs)
  • current QG condition: New Code: 92%

Now you’re guaranteed that all new commits will have higher coverage than the current overall coverage. This is why we put so much emphasis on Clean as You Code. It doesn’t enforce unreasonable demands to clean up legacy code. And at the same time, it gives you an enforcement mechanism to make sure all new changes are of the quality you want.

 
HTH,
Ann

Makes sense. I think we can live with that. Please keep pushing for the “Ratcheting” option. It would be super useful for most, if not all teams.

1 Like

Ann,

I merged my changes into the master branch and pushed up coverage data and am seeing something a little confusing on SonarCloud.

https://sonarcloud.io/summary/new_code?id=shiftkeyllc_shiftkey-android

The new code coverage on the master branch says “26.0%”, but the overall code is “18%”, which is that different on the master branch?

Hi,

I don’t have access to that URL. Screenshot?

 
Ann

Please find attached.

Hi,

Thanks for the screenshots!

What you’re seeing is the difference between the overall coverage across your entire project - 18.1% - and the slightly higher coverage in New Code, code that was added or updated in your New Code period.

 
HTH,
Ann

Thanks Ann. Looking at the documentation, since we are not using maven/gradle and also not set the sonar.projectVersion value in the parameters to the scanner, based on what data is the new code period calculated?

Also, for long lived branches like the “main” or “master” branch, what is the recommended way to set the projectVersion value? We use the git commit count as the “versionCode” on the APK generated. Would tht be a good value to use?

At the end of the day, we would want every push/merge into the main(or any long lived) branch to be considered as a new version.

Hi,

You should check your project settings ([cog] → New Code) and see what has been chosen for the New Code period. It wouldn’t be calculating “on New Code” values if nothing had been chosen.

In my experience, it’s not a great way to do it because if I add a new issue and that fails the Quality Gate, all I have to do is trigger a new analysis to make it go away. Similarly, the floating “Number of days” New Code period window means that if you’re getting “bad” results, all you have to do is wait patiently for it to go away.

Do you deploy with each push/merge? If so, I suppose I’ll contradict myself and say that the versionCode becomes a decent option. Because that’s really the intent: to enforce that each production deployment is at least as good as the one before.

However, if that’s not the case - and assuming you use semantic versioning - then I’d look at stripping the build number off the versionCode and passing that as your sonar.projectVersion. And at that point, you can successfully use the Project version setting.

 
HTH,
Ann

Our “new code” setting is currently set to “previous version”. Looks like 1 out of the 4 available options MUST be selected.

We do not deploy the binary to the google play store with every build. For all the builds that run on CI, including the long standing branches, we generate the apk and its available to download from Bitrise build’s artifacts page.

We do use semantic versioning for the generated apks, which is major.minor.patch format. Those are again incremented before we create a release branch for a patch, minor or major app update.

Hence the “versionCode” is basically set to the commit count on the branch in question. This way, each build generated from a given branch will have an incremental version code.

With all that being said, when would our “overall coverage” be updated if I do not do anything about the “projectVersion” property? Or will it never get updated?

Hi,

Overall values are updated with every single analysis. New Code values are actually also updated with every single analysis (assuming there are changes). And you control when/how the New Code Period is reset.

 
HTH,
Ann

“New Code Period” impacts when the “overall” values are updated, correct?

Because, clearly as you see in my screenshots, the overall is 18.1 and new is 26. So the new analysis values are not updated on the overall values yet.

Is there a way to define “new code period” explicitly to something like the “previous version of the main branch”?

Hi,

It does not. Overall calculations include new code. Otherwise, they are largely unrelated. As I said, overall values are updated with each analysis, and every single line of code - old or new - is part of “overall”.

Ehm… Let’s say I have 100 LOC overall. Of those 10 are new.
Now let’s say that all 10 of the new LOC are covered → 100% coverage on new code
Let’s further say that the remaining 90 have no coverage → 10% coverage overall

This discussion has gotten really far afield from ‘Pull request analysis displays “0 new lines” …’. If you want to keep digging into this, please create a new thread.

 
Ann

Thanks @ganncamp Will create a new thread for the “new code period” clarifications.