Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurable thresholds to report aggregator #319

Closed
szpak opened this issue Oct 14, 2022 · 6 comments · Fixed by #325
Closed

Add configurable thresholds to report aggregator #319

szpak opened this issue Oct 14, 2022 · 6 comments · Fixed by #325

Comments

@szpak
Copy link
Owner

szpak commented Oct 14, 2022

As implemented in hcoles/pitest#1095 . PIT 1.9.9 (most likely - currently in master).

@Pfoerd
Copy link
Contributor

Pfoerd commented Nov 11, 2022

1.9.9 was just released so this can now be implemented 🚀

@szpak
Copy link
Owner Author

szpak commented Nov 11, 2022

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

@Pfoerd
Copy link
Contributor

Pfoerd commented Nov 11, 2022

@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
Copy link
Owner Author

szpak commented Nov 12, 2022

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
Copy link
Contributor

Pfoerd commented Nov 16, 2022

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

@Pfoerd
Copy link
Contributor

Pfoerd commented Nov 21, 2022

@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?

@szpak szpak added this to the 1.9.1 milestone Nov 25, 2022
szpak added a commit that referenced this issue Nov 25, 2022
szpak added a commit that referenced this issue Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants