Avoid printing warnings from CpsStepContext.completed

[jglick]

When a StepContext is completed twice, only the first result is retained, so this situation might be a bug. On the other hand, it might not; some logic in #76 tried to suppress noisy messages under certain circumstances involving stop(). In jenkinsci/workflow-durable-task-step-plugin#180 I am finding another such case, where one FlowInterruptedException with RemovedNodeCause terminates the body of a node block; subsequently, another such termination with QueueTaskCancelled is thrown to the same context. Since there is no API in StepContext to determine whether it is already complete (so there is no need to mark it complete again), and this situation is typically harmless, I am just tuning all the logging down to FINE to avoid pointless warnings.

[dwnusbaum]

Since there is no API in StepContext to determine whether it is already complete

FWIW I think such an API might be useful for jenkinsci/workflow-durable-task-step-plugin#185 as well, and I would be in favor of adding it.

[jglick]

Perhaps so. Feel free to propose an API in workflow-step-api + impl in workflow-cps and see if the fix from jenkinsci/workflow-durable-task-step-plugin#185 can be reworked to be safer.

As it turns out in jenkinsci/workflow-durable-task-step-plugin#180 I no longer need this as I found there was already a stopping field which suppresses the listener, preventing it from trying to redundantly cancel the step body.

[car-roll]

@jglick Do you feel there is any worth in including this PR? If not, I can go ahead and close it

[jglick]

No strong feeling either way. I think this warning is usually just noise; but I also do not have a specific example at the moment of a case where it is difficult to avoid by other means (other than the category of cases involving stop for which the warning is already suppressed). Would recommend merging rather than closing. @dwnusbaum any particular opinion?

[dwnusbaum]

I don't feel too strongly either way, and I don't recall ever seeing these messages as part of an actual bug. Maybe it would be worth using something like Main.isUnitTest ? Level.WARNING : Level.FINE just so developers will still see the messages in unit tests in case they make a change that would introduce an issue.