Address how and where we end the AsynchronousExecution

[svanoort]

Corrects for cases where the AsynchronousExecution could be left dangling inadvertently.

[jglick]

As an amendment to jenkinsci#27.

[svanoort]

Precisely.

[jglick]

This is very close to what I had initially, and it does not work, as explained in 18d78f3.

[jglick]

Looks OK. Do tests pass with this?

[jglick]

Not really related to changes in JEP-210, right?

[svanoort]

Not as such, but more correcting for some issues in the logic.

[jglick]

could revert

[jglick]

Passes for me, with some flakes such as

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.jenkinsci.plugins.workflow.job.WorkflowRunRestartTest.lambda$resumeDisabled$3(WorkflowRunRestartTest.java:101)

from

r.waitForCompletion(b);
assertFalse(b.executionLoaded); // here

which I suspect are due to a race condition between WorkflowRun.isBuilding and something else. But possibly a preëxisting condition?

[svanoort]

@jglick Still sorting out some test oddities

[jglick]

Still getting some flakes:

java.lang.AssertionError: expected:<1> but was:<0>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:645)
	at org.junit.Assert.assertEquals(Assert.java:631)
	at org.jenkinsci.plugins.workflow.job.WorkflowRunRestartTest.lambda$flowExecutionListener$11(WorkflowRunRestartTest.java:295)

and

java.lang.AssertionError: expected null, but was:<Owner[p/1:p #1]>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotNull(Assert.java:755)
	at org.junit.Assert.assertNull(Assert.java:737)
	at org.junit.Assert.assertNull(Assert.java:747)
	at org.jenkinsci.plugins.workflow.job.WorkflowRunRestartTest.lambda$lazyLoadExecution$5(WorkflowRunRestartTest.java:132)

[svanoort]

For whatever reason, what should be a trivial fix is not, and oddly the original does not trigger failures (CI only for me, see to be locally irreproducible), so... I guess we roll with it.