GH4475: Always show task execution summary #4486
Conversation
|
The output of these changes can be seen here:
For both the CakeSpectreReportPrinter and the CakeReportPrinter. |
|
When attempting to solve this problem, I tried to:
While this is technically a breaking change, since the exception type that is returned is different, the original exception type is still available to be found, so I personally think that this is "ok". Happy to hear any other ideas about what can be done here. |
Currently, Cake prints out a task execution summary on each successful execution. This summary includes which tasks were executed or skipped, and the duration of the task. However, then an execution of Cake fails, the task summary is not printed, and it is necessary to scroll through the Cake output, to find out what task failed. You also lose the information about which tasks, if any, were skipped. Any changes to this workflow will have to be done carefully though, as we don't want to alter how tasks are executed, in what order, or what is done in the cases of ContinueOnError, etc. There are a number of unit tests that check for this execution, and these will need to continue to pass. This commit aims to address this problem by introducing a new CakeReportException, which has a property called Report of type CakeReport. That way, when an exception needs to be thrown, the current execution status report can be passed along with it, and from there, the task execution summary can be printed. The introduction of the CakeReportException has meant that a number of unit tests have had to be updated, since there is an expectation that an InvalidOperationException will be returned, however, I think that this is "ok", since it hasn't meant a change to how the workflow of task executions completes.
|
@patriksvensson no objections to the possible breaking change here? Or do you not consider it a break? |
Internal bits so think it's fine. |
|
No, changing the exception type (if I understand correctly) might theoretically be a breaking change but I can't see how this will impact anyone. |
devlead
changed the title
|
Perhaps make |
No objections to doing that. Are you both happy with |
|
@patriksvensson running into this...
Should I unseal CakeException? Or what is the correct course of action here? |
|
Missed last comment after enabled automerge. Agree CakeReportException should inherit CakeException. |
|
@devlead happy to follow up with another PR, but I am unsure about unsealing |
3 participants
Currently, Cake prints out a task execution summary on each successful execution. This summary includes which tasks were executed or skipped, and the duration of the task. However, then an execution of Cake fails, the task summary is not printed, and it is necessary to scroll through the Cake output, to find out what task failed. You also lose the information about which tasks, if any, were skipped.
Any changes to this workflow will have to be done carefully though, as we don't want to alter how tasks are executed, in what order, or what is done in the cases of ContinueOnError, etc. There are a number of unit tests that check for this execution, and these will need to continue to pass.
This commit aims to address this problem by introducing a new CakeReportException, which has a property called Report of type CakeReport. That way, when an exception needs to be thrown, the current execution status report can be passed along with it, and from there, the task execution summary can be printed.
The introduction of the CakeReportException has meant that a number of unit tests have had to be updated, since there is an expectation that an InvalidOperationException will be returned, however, I think that this is "ok", since it hasn't meant a change to how the workflow of task executions completes.