Progress report#1469
Conversation
9f80dea to
6a2036f
Compare
|
Thanks for this. I think it should be possible to implement this as a org.pitest.mutationtest.MutationResultListener. This would require far less code change, and would automatically make it a configurable feature across all build tools, including gradle. The org.pitest.mutationtest.MutationResultListenerFactory allow a feature string to be configured, so this could be turned on with, for example +progress. Features can also accept simple parameters, so the interval could be set with +progress(interval[42]) The examples of MutationResultListenerFactory in the codebase don't demonstrate how the feature strings work (features were retrofitted to the interface for the benefit of external plugins). But if you look at the MutationInterceptorFactory there are several examples. |
|
Funny enough, after I implemented this I wanted to test it with my existing Gradle projects, and I realized the Gradle plugin only allows known configuration options. I then figured out that it does allow specifying arbitrary features, so I implemented a I'll have a look at the |
|
@hcoles The downside of the One solution would be to add the total number of mutation units to the Another option would be to expose more information about the scan on the |
92badc2 to
7b63005
Compare
The previous implementation conflated output format listeners (selected by name) with feature-activated listeners in a single predicate. This caused the unknown-format error check to be bypassed when any feature-activated listener factory was on the classpath.
Enabled with +progress or +progress(interval[N]) where N is seconds between reports (defaults to 30). Counts individual mutations completed and writes to stdout on a timer, giving a rough sense of how far the analysis phase has progressed.
7b63005 to
da9c47a
Compare
A feature-managed listener whose name happened to match an output format string would silently satisfy the validation check without actually producing output. Only legacy-mode factories should be eligible for output format name matching. Also renames isFeatureActivated to isActivatedByFeature for clarity.
eatdrinksleepcode
left a comment
There was a problem hiding this comment.
@hcoles I updated the implementation using a MutationResultListener as you suggested. It's definitely cleaner and does not require modifying core classes. The only downside is the loss of the denominator.
|
|
||
| final Collection<MutationResultListenerFactory> outputFormatListeners = FCollection | ||
| .filter(listeners, matchesOutputFormat(this.options.getOutputFormats())); | ||
| if (outputFormatListeners.size() < this.options.getOutputFormats().size()) { |
There was a problem hiding this comment.
Because there is now a MutationResultListenerFactory that does not use the LEGACY_MODE feature, this guard did not fire correctly when an invalid output format was specified, because there was 1 requested output format and 1 listener in the resulting collection (it was just the wrong listener). The corresponding test in SettingsFactoryTest caught the problem. Output formats are now verified against LEGACY_MODE listeners only; then feature-managed listeners are added in.
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class ProgressListenerFactoryTest { |
There was a problem hiding this comment.
Other listener factories don't have tests, but I felt like these are simple enough and have enough value to add. Can remove if you don't think they are worth keeping.
| aMutationTestResult().build(), | ||
| aMutationTestResult().build(), | ||
| aMutationTestResult().build())); | ||
| Thread.sleep(1500); |
There was a problem hiding this comment.
I don't love that this test has to sleep for a 1.5 seconds. One alternative is to express the interval in ProgressListener in milliseconds, and convert the values in ProgressListenerFactory. Then at least this test could sleep for a much shorter time. That's actually what I did in the previous version of this PR. But ultimately I decided the confusion of expressing the value with different units in different places wasn't worth it.
The mutation analysis phase can take a long time, but there's no indication of how far along it is. This adds periodic progress logging that reports how many mutation test units have completed out of the total, giving a rough sense of how much time remains.
MutationAnalysisExecutorthat logs completion count at a configurable intervalprogressInterval(in seconds) as a configuration option via CLI and Maven plugin