Use correct variable for nodeAfter in MemoryFlowChunk constructor #89
Conversation
jglick
left a comment
jglick
left a comment
There was a problem hiding this comment.
Seems right by visual inspection, but I find it disturbing that there is no test coverage for this constructor.
From what I can tell, this is because the normal usage idiom is to maintain a single I will go ahead and add a basic unit test for the constructor. |
| assertThat(chunk.getNodeBefore(), equalTo(start)); | ||
| assertThat(chunk.getFirstNode(), equalTo(blockStart)); | ||
| assertThat(chunk.getLastNode(), equalTo(blockEnd)); | ||
| assertThat(chunk.getNodeAfter(), equalTo(end)); |
There was a problem hiding this comment.
Test fails here if the change in src/main is reverted.
…r882853693 Example from `ExecutorStepDynamicContextTest.parallelNodeDisappearance`: ``` "Computer.threadPoolForRemoting [#3]" jenkinsci#89 daemon prio=5 os_prio=0 tid=0x00007f30d0c6a800 nid=0x6d0f9 in Object.wait() [0x00007f31047fe000] java.lang.Thread.State: TIMED_WAITING (on object monitor) at java.lang.Object.wait(Native Method) at java.lang.Object.wait(Object.java:460) at hudson.remoting.AsyncFutureImpl.get(AsyncFutureImpl.java:97) - locked <0x00000000f83dd1e8> (a hudson.remoting.AsyncFutureImpl) at org.jenkinsci.plugins.workflow.support.steps.ExecutorStepDynamicContext.resume(ExecutorStepDynamicContext.java:108) at org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution.onResume(ExecutorStepExecution.java:201) at org.jenkinsci.plugins.workflow.flow.FlowExecutionList$ParallelResumer.lambda$run$5(FlowExecutionList.java:369) at org.jenkinsci.plugins.workflow.flow.FlowExecutionList$ParallelResumer$$Lambda$350/265274739.run(Unknown Source) at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28) at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68) at … ```
3 participants
Noticed while working on adding some new tests in pipeline-graph-analysis. Thankfully, it doesn't look like there are any callers currently using this overload of the constructor (here is a GitHub search for MemoryFlowChunk in the jenkinsci org).