Test case for JENKINS-57253

[basil]

Adding a test case for JENKINS-57253. The last assertion currently fails due to a leaked executor, as described in the bug, so I have marked the test with @Ignore. When the bug is fixed, the @Ignore annotation can be removed.

[dwnusbaum]

Thanks for the PR! I am not sure if we can recover from this kind of error, we probably need to make groovy-cps fail during compilation when break (maybe also continue) is misused like this. In regular Groovy you get a compilation error with this message: "the break statement is only allowed inside loops or switches".

[dwnusbaum]

See cloudbees/groovy-cps#109 for the fix.

[dwnusbaum]

Also, did you happen to check whether things work correctly if you do put a loop or switch around the node step? If that causes problems, then we also need some kind of runtime behavior change in groovy-cps.

EDIT: Actually, I am pretty sure that break would be disallowed in something like for { node { break; } } because the closure is a new scope as far as break and continue are concerned.

[basil]

Thank you for the fix!

Actually, I am pretty sure that break would be disallowed in something like for { node { break; } } because the closure is a new scope as far as break and continue are concerned.

Indeed, I confirmed that for { node { break; } } fails at the compilation stage as you predicted.

[car-roll]

@basil fyi I merged cloudbees/groovy-cps#109 but I don't think it's getting released until we get the repo moved into jenkinsci.
I noticed that this test did not have an @Ignore. Is this issue already fixed via some other way?

[basil]

@basil fyi I merged cloudbees/groovy-cps#109 but I don't think it's getting released until we get the repo moved into jenkinsci. I noticed that this test did not have an @Ignore. Is this issue already fixed via some other way?

Sorry, when I was merging with master I accidentally added some unrelated changes to this. I just corrected that. It now has the right set of changes with @Ignore.

[car-roll]

@basil what confused me though was that the checks showed green. Did this test actually pass without any fix?

[basil]

@basil what confused me though was that the checks showed green. Did this test actually pass without any fix?

They were green because I had posted an unrelated set of changes that happened to pass.