I’ve then created a branch with some deliberately poor code as to fail the quality gates and to observe my PR build fail and for sonar to create comments within. However, within the PR, no comments are presented and the build reports a successful completion even though the quality gate has failed when you inspect the build itself.
To extend this out and demonstrate…
I have a build with the SonarCloud steps wrapped around it. The only difference I can observe from the demo is that we do not use VSTS Tasks to construct my build - instead we run a build.cake script.
As such my build definition looks like this:
[Image removed due to restriction] Image shows SonarCloud Prepare, Run and Publish steps wrapped a round a Powershell step and a Publish Test Results to VSTS step
When I queue this build and it completes, I get my feedback from SonarCloud as shown:
[Imaged Removed] Image shows that the Sonar Quality Gate fails
I’ve got this same build definition set as a build validation requirement in the branch policy of my master branch. When I create a PR into master, the build auto-triggers but then I get no PR comments and it reports back as successful:
Can you give us all the parameters you pass in your build script (sonar.pullrequest.key, …)? For decoration on VSTS, quite a few extra properties are required:
sonar.pullrequest.provider=vsts
sonar.pullrequest.key
sonar.pullrequest.branch
sonar.pullrequest.target
sonar.pullrequest.vsts.instanceUrl
sonar.pullrequest.vsts.project
sonar.pullrequest.vsts.repository
All those parameters are automatically passed if you use the tasks from the SonarCloud extension.
This is where screenshots would have helped as I wonder if I’ve given you the impression that I perform the Sonar steps within the build script too? I had to backtrack from that approach since I could find no way of publishing the results of the analysis back into VSTS. As such, I use the SonarCloud VSTS Steps either side of running my build script unfortunately.
However, here is a screenshot showing you what I’m passing & how:
I did briefly attempt to use the “Use Standalone Scanner” option from that screenshot but I was greeted with some errors so hence turning to Sonar for help. Also the how-to guide didn’t present that approach either so I guessed I was barking up the wrong tree.
Does anything stand out as being incorrect to you with this setup?
And as a final point, I’ve also tried to elicit the PR feedback by disabling the Cake/Powershell Step and replacing it with a basic .NET Core Build step. The Sonar Analysis was identical as was the lack of PR feedback.
OK, then I guess your configuration is correct on VSTS side (if not, you would not see the PR in SonarCloud).
Then the only thing to check is the token that will be used to decorate the PR on VSTS. We have looked at logs on SonarCloud side, and the error is:
Failed to access VSTS, the repository or the Pull Request: API resource location 225f7195-f9c7-4d14-ab28-a83f7ff77e1f is not registered on https://xxxxx.visualstudio.com/. HTTP 401 Unauthorized
I have reproduced the same behavior by using an expired token. Can you double check?
The token I was using has an expiry of June 2019 so it definitely hasn’t expired. I double checked permissions and to confirm it does have ‘Code (read and write)’ assigned.
However, this morning I’ve created a new token with the same access levels, and then used this token in one of our projects and indeed and to my surprise has worked (well, mostly as expected).
I’ll switch my projects to the new key. Its curious that the old key wouldn’t work when it has the same roles and not expired but anyhow, that’s not your problem. You’ve lead me to the fix - thank you!
Mr pull request comments have now been generated which is very cool. I have a final question / clarification to make before retiring this issue.
As you can see, I have 5 Open issues and PR comments (which is awesome); but, the build (labelled Quality Gate) has succeeded and as such with an approval this PR can be merged despite those 5 issues.
This misaligned with my expectations that the build should be flagged as failed as to prevent this PR from being merged until the high severity issues are fixed; else there is simply no auto-enforcement to protect master from the poor code about to me merged into it.
Are your expectations of his this should work different or is there something else I need to do to acquire this behaviour?
I’m glad you managed to fix the authentication issue.
Regarding your question, we don’t consider that breaking the build to prevent merging the PR is the good approach. A build is a technical step, that is passing or failing, no discussion.
Static code analysis is a much more complex topic. Analyzers may report some issues. Developers will analyze them and decide either:
to fix them and update the PR (and a new build will be triggered)
mark the issues as “false positive”/“won’t fix” in SonarCloud (and this status will automatically be propagated to the master branch analysis when the PR will be merged)
mark the issue as “confirm” in SonarCloud, which basically mean: ok, I acknowledge the issue, but I will not fix it as part of this PR.
The two second points are actions that are asynchronous to the build, so “breaking the build” is not right. Instead SonarCloud will dynamically update the PR in VSTS (changing comment status, and also updating the PR status).
If you want to prevent the merge until all issues are “cleared” (using one of the 3 options I mentioned above), it is possible. Just add a status policy as described in the last section of the lab (Block pull requests if the Code Quality check failed).
If you follow my reasoning, naming your build “Quality Gate” is misleading, I would change it.
