PR decoration for GitHub no longer working

github
pull-request

(Daniel Ingrey) #1

We’re running a trial of SonarCloud and everything was going great until our testing branch was merged into master. Now I can’t get it to decorate / report back on the pull requests on GitHub.
I’ve tried

  • adding a new token
  • reinstalling the SonarCloud github app
  • using the SonarCloud app without a token
  • using a token without the SonarCloud app
  • recreating the project in SonarCloud

(Janos Gyerik) #2

We have recently changed the way we decorate pull requests on GitHub. In the past we used Reviews, today we’re using Checks. The user interface is a bit different, so first I would like to make sure that you’re not simply overlooking it. Please verify the following, even if they may seem trivial, as sanity checks:

  • Does the pull request have any open issues?
  • On the page of the pull request on SonarCloud, is there a warning in the top-right corner? It may have text like “Last analysis had 1 warnings” with yellow background. The warning may contain a clue.
  • On the PR’s page on GitHub, there should be a new tab named Checks (after Conversation and Commits, and before Files changed). On that tab, do you see a check labeled SonarCloud?
  • On the Conversation tab, at the bottom of the page there should be a line with the SonarCloud status of the pull request. Is there such line, and what does the text say?

If the project is open-source, it will help a lot to share a link to it, and to a pull request where you think decoration should happen but you don’t see it.


(Daniel Ingrey) #3
  • There are no open issues. Does it need to find issues to decorate?
  • There is a warning in SonarCloud “SCM provider autodetection failed. Please use “sonar.scm.provider” to define SCM of your project, or disable the SCM Sensor in the project settings.”. This makes sense as the scanner is run from a docker container. We copy over the source, run tests and produce a coverage report. We then run the scanner using this report. Could this be an issue?
  • There is a check labeled SonarCloud, it says " SonarCloud Queued 3 hours ago". Clicking re-run has no effect
  • The SonarCloud check isn’t displayed on the Conversation tab, just our CI check

The project isn’t open source unfortunately, we’re trialing SonarCloud on our most updated repo which is internal.


(Robert Damiano) #4

Hi @janos

We are experiencing the same issue. We have disconnected our GitHub SonarCloud App and re-added it and this is the resulting error…

Please let us know if there is anything we can do to fix this.

Thanks


(Aurélie Boiteux-Cabourdin) #5

Hi Daniel,

PR decoration will happen whether or not there are open issues.
How do you launch the PR analysis ?

Also you may have a look at this doc, this may help.


(Daniel Ingrey) #6

Hey. We use sonar-scanner from within a docker container during a jenkins build. It was working which is why I’m very confused as to what might have changed. The scanner context from a recent PR run is below.

Settings for module: trussle_bat
  - sonar.exclusions=**/*.spec.js, **/*.spec.ts, **/*.spec.jsx, **/*.spec.tsx, **/*.provider.ts, **mock**
  - sonar.host.url=https://sonarcloud.io
  - sonar.javascript.lcov.reportPaths=coverage/lcov.info
  - sonar.login=******
  - sonar.organization=trussle
  - sonar.projectBaseDir=/build
  - sonar.projectKey=trussle_bat
  - sonar.projectName=BAT
  - sonar.projectVersion=1.0
  - sonar.pullrequest.base=master
  - sonar.pullrequest.branch=PR-457
  - sonar.pullrequest.github.endpoint=https://api.github.com/
  - sonar.pullrequest.github.repository=trussle/bat
  - sonar.pullrequest.key=457
  - sonar.pullrequest.provider=github
  - sonar.scanner.app=ScannerCli
  - sonar.scanner.appVersion=3.2.0.1227
  - sonar.sourceEncoding=US-ASCII
  - sonar.sources=assets,lib,routes
  - sonar.typescript.lcov.reportPaths=coverage/lcov.info
  - sonar.working.directory=/build/.scannerwork

This is what I get in the conversation tab in github:


(Janos Gyerik) #7

Yes, this prevents the decoration. The decoration requires the sha1 of the HEAD of the pull request. The Git plugin picks this up during the scan, and the scanner writes it to the report sent to the server. If the SCM provider was not detected as Git, this won’t happen, and so no decoration will happen. This also explains what you seen on GitHub about the pending check.

I have not yet seen a use case of copying sources to run a scan. Would it be possible to copy the .git folder too, or clone from the sources instead of copying, or some other way so that you have sources + SCM data too?

The reason why it was working before is related to the change of GitHub API to decorate. The previous method (using Reviews API) could work with PR numbers without precise commit sha1, the new method (Checks API) requires commit sha1.

One possible remedy I can think of is to add a new scanner parameter to override explicitly the commit sha1 to use. I just don’t know if there is significant demand for this use case.


(Daniel Ingrey) #8

ah ok. Thanks! I’lll try copying over the git files too and if that doesn’t work checkout from within the container.

If you’re interested in our use case (which I’m not sure is that common but anyway…). We use docker multi stage builds and run all tests from within a builder container we create as part of the multi stage builds. Reasoning being we use macs to develop on but linux to host our deployments. Running everything inside the containers keeps the environments consistent.


(Janos Gyerik) #9

The status line “SonarCloud Code Analysis” belongs to the new way of decoration. It looks like it’s working.

The status line “SonarCloud” with the message “Expected — …” is a mystery to me. The “SonarCloud” context was indeed used in our previous way of decoration, but I don’t see how such state is possible. And a non-green status should have become green when the new decoration came into effect. If you push a new commit and re-analyze, that should make that line go away. Let me know if it doesn’t, and if you have a way to reproduce it.


(Robert Damiano) #10

Yes, the new way is working. Uninstalling SonarCloud and reinstalling it did not remove the “Expected” requirement. We had to go into our GitHub repo settings > branches > edit to remove the required check. We expected the uninstall to automatically do that. No biggie, we now have it working with the new app.

Thanks for your help.


(Daniel Ingrey) #11

I’m now running the scan from within the checked out directory and I’m still not getting any PR decoration or checks on the conversation tab in github. I’m not sure if it could be a problem, but how our jenkins build works is it merges master into the branch and creates a new branch named PR- eg PR-123. I think this is standard jenkins behaviour though, and we don’t build the actual branch during pull request as we don’t care about it. But I guess github wont have the same sha1? I’ve tried changing sonar.pullrequest.branch to both the jenkins branch name and the branch name of the PR but no dice. I’m not sure if this is the cause.

We’re over halfway through our trial now and I’ve been spending time trying to fix this rather than actually getting to try the product which isn’t great.


SonarCloud doesn't update GitHub status
(Janos Gyerik) #12

It will not have the same sha1, and that is a problem. SonarCloud will decorate the sha1 of the working tree that is being analyzed. If that’s not the same as the sha1 of the HEAD of the PR on GitHub, then you won’t see it on the PR on GitHub, because GitHub only shows Checks and Statuses for the sha1 that match the HEAD…


(Janos Gyerik) #13

Mystery solved, and the next post in this very thread hinted at that too: the status is “Expected” when the admin of the repo configured to require the check. (And SonarCloud cannot clear that setting, only the admin of the repo can.)


(Michaël Krens) #14

If you only need the HEAD sha1 I’d definitely argue having an option to add this as a parameter would have been great for us as well as now we have to jump through hoops to include the .git folder which is never included in our docker builds normally.


(Janos Gyerik) #15

@michi88 Good point. We agree the current behavior is confusing and error-prone, and we have started planning an improvement, to be realized soon.


(Mikhail Filipchuk) #18

Awesome! Yeah, we have the same issue and use case @daningrey mentioned. Would love to use the fix and see good old check on GitHub PRs as well =)


(Mikhail Filipchuk) #19

Any update on the issue?