Static code analysis

[simondel]

SonarCloud has an (supposedly) easy to use integration with Azure Pipelines. This could help us identify code smells and potential bugs.

[ptoonen]

@simondel are you still actively working on this, otherwise I can pick it up from here?

[simondel]

@ptoonen go ahead! I ran into issues SonarCloud. It only parsed code from one project as being C# code.

There is a task based (not YAML) pipeline on Azure Pipelines with SonarCloud. I think it's called stryker-net OLD or something.

[simondel]

The current project can be found here: https://sonarcloud.io/dashboard?id=stryker-net.stryker-net

As you can see, SonarCloud only detected 115 lines of C# code. None from the Core package.

The code is there though: https://sonarcloud.io/component_measures?id=stryker-net.stryker-net&metric=lines&selected=stryker-net.stryker-net%3Asrc%2FStryker.Core%2FStryker.Core%2FTesting%2FProcessExtensions.cs

[ptoonen]

@simondel can you authorize me on the sonar project?

[ptoonen]

Okay, so I can successfully analyze the solutions 1-by-1. The problem is that SonarQube doesn't like code that's referenced by multiple projects (it does correctly mark the files as 'shared' but won't analyze them for some reason). I'm guessing the real problem is that Core and Core.UnitTest are referenced by two solutions in the repository.

Now the solution here is a bit tougher but we've got a few options:

1. Split the repo into multiple repo's - one for core, one for CLI (kind of like the javascript version). This wouldn't be a strange option considering the fact that they're actually two separate things and you may want to version treat them as such.
2. Combine all projects into one solution. Also a valid option because of the tight coupling between them at the moment. This is seems to be the approach sonarsource uses with the msbuild scanner.
3. Build the csproj's rather than the solutions. This is also what happens in the newer azure-pipelines.yml file. This would however mean that we'd need to add a to all the csproj files we would want to analyze otherwise SonarQube doesn't support it.

@richardwerkman @Mobrockers @simondel what are your thoughts on this? Do you see any other viable options?

[ptoonen]

ps. I prefer option 2

[simondel]

@ptoonen I'm not sure if solution 2 would work. There is already a solution that contains all projects. It's the CLI solution.

PS I've granted you and @Mobrockers permissions on SonarCloud (github login). I couldn't rant @richardwerkman any permissions since he never signed in.

[ptoonen]

@simondel it works, look at the current results. Do mind that it doesn't include everything because as of right now there are 3 solutions in the repo: Stryker.CLI.sln, Stryker.Core.sln and Stryker.CLI.IntegrationTests.sln

This sounds a bit redundant to me. If we add the integration tests to the CLI solution, we should be good to go.

[rouke-broersma]

@ptoonen The integration test solution includes only some known-bad projects which we run stryker on as an integration test. I don't think they should be included in the sonar analysis anyway

[richardwerkman]

@ptoonen I'm also in for option 2. However I split the integrationtest project in a new solution intentionally to make live unit testing possible in the CLI solution. Otherwise each line that changes would trigger a 45 second testrun.

[richardwerkman]

@simondel I've logged into SonarCloud so I guess you can grant me permissions now 👍

[ptoonen]

However I split the integrationtest project in a new solution intentionally to make live unit testing possible in the CLI solution.

We could keep them separate, or exclude them from live unit testing: https://docs.microsoft.com/en-us/visualstudio/test/live-unit-testing?view=vs-2017#include-and-exclude-test-projects-and-test-methods

[rouke-broersma]

There is no actual used code in the integration test projects so I suggest we just keep them as it is and not include in sonar analysis

[ptoonen]

Okay, so I've got most of it done. Biggest issue we have now is that Sonar will only accept one .trx file. That means we either have to merge (which is tough seeing as trxmerger is not perfect), or pick which one we want to use.

For the work in progress, look here: https://github.com/ptoonen/stryker-net/blob/164-static-code-analysis/azure-pipelines.yml

@simondel @richardwerkman what do you guys think? I'm guessing coverage on the Core project is the most useful if we'd have to pick one. The reason we have multiple .trx files is because they're multiple testruns because dotnet test doesn't support testing a full solution.