szpak / gradle-pitest-plugin

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

Print report paths as clickable URIs at end of execution (optional?) #296

Open davidburstrom opened 2 years ago

davidburstrom commented 2 years ago

I often want to open the Pitest report after the task has finished. Just like e.g. the PMD tasks print the location of the report, I think it would be handy if the report path is printed as a clickable URI.

I propose an option to print the report path. Maybe it should be ternary: "off" (default), "on-failure", or "always", but for my purposes I'd be happy with either "off" or "always".

szpak commented 2 years ago

I like the idea. Very often, I click the link to the Gradle test report which is displayed on failure. To keep the task not very verbose by default (in the context of #147 and #267 :-) ), I would show it on-failure by default, with the 3 aforementioned options available to set. WDYT?

davidburstrom commented 2 years ago

Sounds good, I've got something in the works.

davidburstrom commented 2 years ago

There we go, how does this look?

davidburstrom commented 2 years ago

I notice when trying this out locally that if the line coverage calculation fails AND there is a pre-existing non-timestamped report, the report path will be printed anyway. I reckon this is fixable either by actually capturing the output to see what's wrong (which could be brittle given future changes to Pitest) or that the output directory is cleaned prior to execution (if non-timestamped runs should execute).

szpak commented 2 years ago

Yes, it might be tricky to determine a status based on the PIT output. However, for non-timestamped report, we could try to check its modification date before execution and, in case of failure, display the link (if enabled for failure) only if that date has changed. Otherwise, the report is outdated (from the previous execution) and could lead to confusion. For "always" displaying the report, it could be more tricky. For successful execution, probably we should always display the link (even for outdated report - the execution could be skipped/cached). For failure? I'm not sure, if there is a sense to display a link if the report hasn't been updated (to avoid confusion). However, there could be corner cases. WDYT?

Btw, I started a review, but I have to re-think some things (e.g. a chance to test some corner cases without the functional tests - it can increase the execution time). I will come back to it "soon", probably I will have some more suggestions.

P.S. Yesterday, I brought our main Travis CI back to life - #290 :-).

davidburstrom commented 2 years ago

Yes, it might be tricky to determine a status based on the PIT output. However, for non-timestamped report, we could try to check its modification date before execution and, in case of failure, display the link (if enabled for failure) only if that date has changed. Otherwise, the report is outdated (from the previous execution) and could lead to confusion. For "always" displaying the report, it could be more tricky. For successful execution, probably we should always display the link (even for outdated report - the execution could be skipped/cached). For failure? I'm not sure, if there is a sense to display a link if the report hasn't been updated (to avoid confusion). However, there could be corner cases. WDYT?

I think the modification date is a good enough indication that the non-timestamped report has been updated and it would work for both "on-failure" and "always". Btw, if the execution is skipped/cached, the task action won't be executed and won't provide a chance to print it. The report URI could be printed by a finalizer task, but I hesitate to introduce that.

Btw, I started a review, but I have to re-think some things (e.g. a chance to test some corner cases without the functional tests - it can increase the execution time). I will come back to it "soon", probably I will have some more suggestions.

Agreed, invoking a Gradle build for the test could be overkill. When it all boils down to it, the task action could call into another class that is inherently more testable, and where the output from the Pitest execution can be faked.

P.S. Yesterday, I brought our main Travis CI back to life - #290 :-).

Great news :)

szpak commented 2 years ago

I spent definitely too much time working on some minor tweaks for the release of 1.7.0. Let's move it to 1.7.1 and the first time slot intended for this plugin development, I will have...