Skip to content
Permalink
Browse files

Merge pull request #3 from jglick/JENKINS-26156-BodyInvoker.withDispl…

…ayName

[JENKINS-26156] BodyInvoker.displayName was broken
  • Loading branch information...
jglick committed Apr 12, 2016
2 parents 654e8b4 + 5827a00 commit 5e8cbc2be2b340c2532d4720c7a24ca44d53c4c3
@@ -6,7 +6,6 @@
import com.cloudbees.groovy.cps.Outcome;
import com.cloudbees.groovy.cps.impl.CpsCallableInvocation;
import com.cloudbees.groovy.cps.impl.FunctionCallEnv;
import com.cloudbees.groovy.cps.impl.SourceLocation;
import com.cloudbees.groovy.cps.impl.TryBlockEnv;
import com.cloudbees.groovy.cps.sandbox.SandboxInvoker;
import com.google.common.util.concurrent.FutureCallback;
@@ -18,14 +17,12 @@
import org.jenkinsci.plugins.workflow.cps.nodes.StepEndNode;
import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode;
import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.BodyExecution;
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepExecution;

import javax.annotation.CheckForNull;
import javax.annotation.concurrent.GuardedBy;
import java.io.IOException;
import java.util.Collection;
@@ -37,6 +34,7 @@
import java.util.logging.Logger;

import static java.util.logging.Level.*;
import javax.annotation.Nonnull;
import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.*;

/**
@@ -86,15 +84,9 @@
@GuardedBy("this")
private Outcome outcome;

/**
* @see CpsBodyInvoker#createBodyBlockNode
*/
private final boolean createBodyBlockNode;

public CpsBodyExecution(CpsStepContext context, List<BodyExecutionCallback> callbacks, boolean createBodyBlockNode) {
CpsBodyExecution(CpsStepContext context, List<BodyExecutionCallback> callbacks) {
this.context = context;
this.callbacks = callbacks;
this.createBodyBlockNode = createBodyBlockNode;
}

/**
@@ -117,7 +109,7 @@ public CpsBodyExecution(CpsStepContext context, List<BodyExecutionCallback> call
sn.addAction(a);
}

StepContext sc = subContext(sn);
StepContext sc = new CpsBodySubContext(context, sn);
for (BodyExecutionCallback c : callbacks) {
c.onStart(sc);
}
@@ -289,7 +281,7 @@ public Next receive(Object o) {
en.addAction(new ErrorAction(t));

setOutcome(new Outcome(null,t));
StepContext sc = subContext(en);
StepContext sc = new CpsBodySubContext(context, en);
for (BodyExecutionCallback c : callbacks) {
c.onFailure(sc, t);
}
@@ -306,7 +298,7 @@ public Next receive(Object o) {
StepEndNode en = addBodyEndFlowNode();

setOutcome(new Outcome(o,null));
StepContext sc = subContext(en);
StepContext sc = new CpsBodySubContext(context, en);
for (BodyExecutionCallback c : callbacks) {
c.onSuccess(sc, o);
}
@@ -317,58 +309,38 @@ public Next receive(Object o) {
private static final long serialVersionUID = 1L;
}

/**
* Creates a sub-context to call {@link BodyExecutionCallback}.
* If {@link #createBodyBlockNode} is false, then we don't have distinctive
* {@link FlowNode}, so we just hand out the master context.
*/
private StepContext subContext(FlowNode n) {
if (n==null)
return context;
else
return new CpsBodySubContext(context,n);
}

/**
* Inserts the flow node that indicates the beginning of the body invocation.
*
* @see #addBodyEndFlowNode()
*/
private @CheckForNull StepStartNode addBodyStartFlowNode(FlowHead head) {
if (createBodyBlockNode) {
StepStartNode start = new StepStartNode(head.getExecution(),
context.getStepDescriptor(), head.get());
this.startNodeId = start.getId();
start.addAction(new BodyInvocationAction());
head.setNewHead(start);
return start;
} else {
return null;
}
private @Nonnull StepStartNode addBodyStartFlowNode(FlowHead head) {
StepStartNode start = new StepStartNode(head.getExecution(),
context.getStepDescriptor(), head.get());
this.startNodeId = start.getId();
start.addAction(new BodyInvocationAction());
head.setNewHead(start);
return start;
}

/**
* Inserts the flow node that indicates the beginning of the body invocation.
*
* @see #addBodyStartFlowNode(FlowHead)
*/
private @CheckForNull StepEndNode addBodyEndFlowNode() {
if (createBodyBlockNode) {
try {
FlowHead head = CpsThread.current().head;

StepEndNode end = new StepEndNode(head.getExecution(),
getBodyStartNode(), head.get());
end.addAction(new BodyInvocationAction());
head.setNewHead(end);

return end;
} catch (IOException e) {
LOGGER.log(WARNING, "Failed to grow the flow graph", e);
throw new Error(e);
}
} else {
return null;
private @Nonnull StepEndNode addBodyEndFlowNode() {
try {
FlowHead head = CpsThread.current().head;

StepEndNode end = new StepEndNode(head.getExecution(),
getBodyStartNode(), head.get());
end.addAction(new BodyInvocationAction());
head.setNewHead(end);

return end;
} catch (IOException e) {
LOGGER.log(WARNING, "Failed to grow the flow graph", e);
throw new Error(e);
}
}

@@ -27,18 +27,16 @@
import com.google.common.util.concurrent.FutureCallback;
import hudson.model.Action;
import org.jenkinsci.plugins.workflow.actions.LabelAction;
import org.jenkinsci.plugins.workflow.cps.nodes.StepEndNode;
import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode;
import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
import org.jenkinsci.plugins.workflow.steps.BodyInvoker;

import javax.annotation.Nullable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import javax.annotation.Nonnull;

import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.*;

@@ -67,11 +65,6 @@

private String displayName;

/**
* If false, do not create inner {@link StepStartNode}/{@link StepEndNode}.
*/
private boolean createBodyBlockNode = true;

/**
* Set to non-null once {@linkplain #start() started}.
*/
@@ -106,9 +99,8 @@ public CpsBodyInvoker withCallback(BodyExecutionCallback callback) {
}

@Override
public CpsBodyInvoker withDisplayName(@Nullable String name) {
public CpsBodyInvoker withDisplayName(@Nonnull String name) {
this.displayName = name;
createBodyBlockNode = (name==null);
return this;
}

@@ -120,16 +112,11 @@ public CpsBodyInvoker withDisplayName(@Nullable String name) {
@Override
public CpsBodyExecution start() {
if (execution!=null) throw new IllegalStateException("Already started");
execution = new CpsBodyExecution(owner, callbacks, createBodyBlockNode);
execution = new CpsBodyExecution(owner, callbacks);

if (displayName!=null)
startNodeActions.add(new LabelAction(displayName));

if (!createBodyBlockNode) {
if (!startNodeActions.isEmpty())
throw new IllegalStateException("Can't specify Actions if there will be no StepStartNode");
}

if (owner.isCompleted()) {
// if this step is already done, no further body invocations can happen doing so will end up
// causing two CpsThreads competing on the same FlowHead.
@@ -1,7 +1,6 @@
package org.jenkinsci.plugins.workflow.cps.nodes;

import hudson.model.Action;
import org.jenkinsci.plugins.workflow.actions.BodyInvocationAction;
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
import org.jenkinsci.plugins.workflow.graph.BlockEndNode;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
@@ -39,12 +38,12 @@ protected String getTypeDisplayName() {

@Override
protected String getTypeFunctionName() {
StepDescriptor d = getDescriptor();
boolean isBody = getStartNode().isBody();
if (isBody) {
return "} //" + (d != null ? d.getFunctionName() : getStartNode().getStepName());
return "}";
} else {
return getStartNode().getStepName() + " : End";
StepDescriptor d = getDescriptor();
return "// " + (d != null ? d.getFunctionName() : getStartNode().getStepName());
}
}

@@ -49,11 +49,11 @@ protected String getTypeDisplayName() {

@Override
protected String getTypeFunctionName() {
StepDescriptor d = getDescriptor();
if (isBody()) {
return (d != null ? d.getFunctionName() : descriptorId) + " {";
return "{";
} else {
return getStepName() + " : Start";
StepDescriptor d = getDescriptor();
return (d != null ? d.getFunctionName() : descriptorId);
}
}

@@ -82,7 +82,7 @@
context.newBodyInvoker().
withContexts(EnvironmentExpander.merge(context.get(EnvironmentExpander.class), new ExpanderImpl(this))).
withCallback(BodyExecutionCallback.wrap(context)).
withDisplayName(null).start();
start();
return false;
}
@Override public void onResume() {
@@ -49,6 +49,8 @@
import org.jvnet.hudson.test.JenkinsRule;

import javax.mail.Folder;
import org.junit.ClassRule;
import org.jvnet.hudson.test.BuildWatcher;

/**
* Test of {@link WorkflowJob} that doesn't involve Jenkins restarts.
@@ -57,6 +59,8 @@
*/
public class WorkflowJobNonRestartingTest extends AbstractCpsFlowTest {

@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();

WorkflowJob p;

@Before public void setUp() throws Exception {
@@ -132,7 +136,6 @@ public void testRetry() throws Exception {

String log = JenkinsRule.getLog(b);
jenkins.assertLogNotContains("\tat ", b);
System.err.println(log);

int idx = 0;
for (String msg : new String[] {
@@ -152,14 +155,14 @@ public void testRetry() throws Exception {

idx = 0;
for (String msg : new String[] {
"[Pipeline] Retry the body up to N times : Start",
"[Pipeline] retry {",
"[Pipeline] } //retry",
"[Pipeline] retry {",
"[Pipeline] } //retry",
"[Pipeline] retry {",
"[Pipeline] } //retry",
"[Pipeline] Retry the body up to N times : End",
"[Pipeline] retry",
"[Pipeline] {",
"[Pipeline] }",
"[Pipeline] {",
"[Pipeline] }",
"[Pipeline] {",
"[Pipeline] }",
"[Pipeline] // retry",
}) {
idx = log.indexOf(msg, idx + 1);
assertTrue(msg + " not found", idx != -1);

0 comments on commit 5e8cbc2

Please sign in to comment.
You can’t perform that action at this time.