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

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

Open
davidburstrom opened this issue Sep 13, 2021 · 7 comments · May be fixed by #297
Open

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

davidburstrom opened this issue Sep 13, 2021 · 7 comments · May be fixed by #297
Milestone

Comments

@davidburstrom
Copy link
Contributor

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

szpak commented Sep 13, 2021

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?

@szpak szpak added this to the 1.7.0 milestone Sep 13, 2021
@davidburstrom
Copy link
Contributor Author

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

davidburstrom added a commit to davidburstrom/gradle-pitest-plugin that referenced this issue Sep 13, 2021
@davidburstrom
Copy link
Contributor Author

There we go, how does this look?

@davidburstrom
Copy link
Contributor Author

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

szpak commented Sep 16, 2021

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

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 szpak modified the milestones: 1.7.0, 1.7.1 Sep 19, 2021
@szpak
Copy link
Owner

szpak commented Sep 20, 2021

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...

@szpak szpak modified the milestones: 1.9.0, 1.9.1 Aug 19, 2022
@szpak szpak modified the milestones: 1.9.11, 1.9.12 Nov 27, 2022
@szpak szpak modified the milestones: 1.14.0, 1.14.1 Sep 1, 2023
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