devcon5io / mutation-analysis-plugin

Plugin for SonarQube to process mutation analysis results.
GNU Lesser General Public License v3.0
30 stars 17 forks source link

Make Issue type configurable #35

Closed yonpah closed 3 years ago

yonpah commented 5 years ago

In my opinion, mutation testing is not primarily aimed at detecting issues in the production code, but rather to gauge the quality of the unit tests. Therefore, I feel that classifying mutation test issues as Bugs sends the wrong message.

There might well be bugs lurking there, since some condition is untested, but a mutation test issue simply says that some part of the code is insufficiently tested. Without any corroborating evidence, I think it is premature to claim that there is anything wrong with the production code (i.e. a bug).

The way to fix mutation test issues is usually to modify or add unit tests, not change the production code. (Looking at mutation test issues may give insights that inspire redesign of the production code, but that is probably not the most common case, and even so, that process would probably still start with adding unit tests before refactoring).

I would prefer mutation test issues to be classified as Code Smells, but I realise that this is project dependent, so I propose that the issue type should be configurable [Bugs|Vulnerabilities|Code Smells], preferably on mutation operator level in the same way severity is, with Code Smells being the default option.

gmuecke commented 5 years ago

The reason mutations are classified as "Bug" rather than a "Code smell" ist that it might actually be good code (as in "no code smell") but a potential bug, as there is no corresponding test that specifies the functionality the particular piece of code provides. Depending on the criticality of the code, the decision was to treat "potential bugs" as "real bugs" as default, because otherwise they might drown in th sea of code smells that tends to be larger than the pool of bugs. But I agree that this might depend on the project context, especially when the code is not that critical or mutation testing is used as a tool to measure continuous improvement. Having a large set of mutations might make it difficult to see the real bugs.

As a starter, I'd suggest to make this configurable on a global/project level. Though I would not provide the option to declare mutations as Vulnerabilities on a global level, as this category is clearly related to security. Applying this setting on a rule level would be next.

As a mitigation, you might use the tags to filter out mutation operators (actually, we use the rest api to pull out metrics during release build and substract the mutation-bugs).

gmuecke commented 5 years ago

I did some experiments with the issue type.

Although a concrete issue can be manually changed to a specific type, there seems to be no standard way in the sonar api for changing the issue type of a rule (customer properties or the serverity can be modified though).

One alternative way would be, to apply the setting on initialization of the rule repository, which happens during startup of the plugin. But the drawback is, that the server has to be restarted in for these changes to take effect. Which is undesirable, especially as there would be no way to make project-specific settings.

As a workaround, all mutator-specific rules could loaded twice - as bug and code_smell - and let the administrator decide, how to setup their individual profile - either with the "Bug-Rules" or with the "Code-Smell-Rule". It does bloat up the rules provided by the plugin, but as there are only ~25 it would be acceptable.

I wouldn't provide the vulnerability variant, as mutations are not clearly a vulnerability (in terms of security) and it would bloat the plugin rules repository even further.

@yonpah Does this sound sensible to you?

yonpah commented 5 years ago

Given the api limitations I think it is a good workaround. I also agree not to add a vulnerability variant.