szpak / gradle-pitest-plugin

Gradle plugin for PIT Mutation Testing
http://gradle-pitest-plugin.solidsoft.info/
221 stars 58 forks source link

Add configurable thresholds to report aggregator #319

Closed szpak closed 2 years ago

szpak commented 2 years ago

As implemented in https://github.com/hcoles/pitest/pull/1095 . PIT 1.9.9 (most likely - currently in master).

Pfoerd commented 2 years ago

1.9.9 was just released so this can now be implemented 🚀

szpak commented 2 years ago

@Pfoerd Will you dare to provide a PR? ;-)

Pfoerd commented 2 years ago

@szpak currently there is no specific extension for the aggregator where the property to configure the aggregated thresholds could be placed. What is your preferred solution: adding a new PitestAggregatorPluginExtension or a "low impact" solution by e.g. reading it from the first PitestPluginExtension found in the multi-module project (like it is done for the charsets).

szpak commented 2 years ago

Good question. Both approaches have their pros and cons. On one hand, many parameters (PIT version, charset) already exist in the regular plugin extension (and most likely no one would like to set different version just for the aggregation), so adding (and maintaining) a dedicated mechanism could be an overhead (usually people use subprojects {} for the generic PIT configuration, so "any" found subproject should be fine). On the other head your 3 parameters are only for the aggregation...

Unless you have a good arguments for the separate extension, I would rather opt for the nested aggregation property in the regular extension to put those 3 new parameters in, e.g.

pitest {
    pitVersion = ...
    ...
    aggregation {
        aggregatedMutationThreshold = ...
        ...
    }
}

WDYT?

I just hadn't check if it is supported by Gradle 6.9. Nevertheless, having any problem with nested property, please just put it directly in the PitestPluginExtension and I will try to "nest" it before the release.

Pfoerd commented 2 years ago

I agree. I will raise a PR (will be a few days before I get around to doing that)

Pfoerd commented 2 years ago

@szpak PR is open. I got the following "nesting" of properties working:

pitest {
    pitVersion = ...
    ...
    reportAggregator {
        aggregatedTestStrengthThreshold.set(50)
        aggregatedMutationThreshold.set(40)
        aggregatedMaxSurviving.set(3)
    }
}

but I'm not a gradle expert, maybe there is a better solution?