Merge pull request #206 from jglick/Itr-JENKINS-27421

[JENKINS-27421] Investigating ArrayList.Itr problem
jenkinsci · Sep 11, 2015 · 61b7d70 · 61b7d70
2 parents a24f130 + e916b5e
commit 61b7d70
Show file tree

Hide file tree

Showing 5 changed files with 188 additions and 160 deletions.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceFailureTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceFailureTest.java
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceProblemStep.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceProblemStep.java
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceProblemStepExecution.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/PersistenceProblemStepExecution.java
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/SerializationTest.java
@@ -0,0 +1,188 @@
+package org.jenkinsci.plugins.workflow;
+
+import hudson.model.Result;
+import hudson.slaves.DumbSlave;
+import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
+import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
+import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
+import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
+import org.jenkinsci.plugins.workflow.steps.Step;
+import org.jenkinsci.plugins.workflow.steps.StepExecution;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
+import org.junit.Test;
+import org.junit.runners.model.Statement;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.DataBoundConstructor;
+import static org.junit.Assert.assertTrue;
+import org.junit.Ignore;
+import org.jvnet.hudson.test.Issue;
+
+/**
+ * Tests related to serialization of program state.
+ */
+public class SerializationTest extends SingleJobTestBase {
+
+    /**
+     * When wokflow execution runs into a serialization problem, can we handle that situation gracefully?
+     */
+    @Test
+    public void stepExecutionFailsToPersist() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                p = jenkins().createProject(WorkflowJob.class, "demo");
+                p.setDefinition(new CpsFlowDefinition(join(
+                        "node {",
+                        "  persistenceProblem()",
+                        "}"
+                )));
+
+                startBuilding();
+                waitForWorkflowToSuspend();
+
+                // TODO: let the ripple effect of a failure run to the completion.
+                while (b.isBuilding())
+                    try {
+                        waitForWorkflowToSuspend();
+                    } catch (Exception x) {
+                        // ignore persistence failure
+                        if (!x.getMessage().contains("Failed to persist"))
+                            throw x;
+                    }
+
+                story.j.assertBuildStatus(Result.FAILURE, b);
+                story.j.assertLogContains("java.lang.RuntimeException: testing the forced persistence failure behaviour", b);
+            }
+        });
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                rebuildContext(story.j);
+
+                story.j.assertBuildStatus(Result.FAILURE, b);
+            }
+        });
+    }
+    /**
+     * {@link Step} that fails to persist. Used to test the behaviour of error reporting/recovery.
+     */
+    public static class PersistenceProblemStep extends AbstractStepImpl {
+        @DataBoundConstructor
+        public PersistenceProblemStep() {
+            super();
+        }
+        @TestExtension("stepExecutionFailsToPersist")
+        public static final class DescriptorImpl extends AbstractStepDescriptorImpl {
+            public DescriptorImpl() {
+                super(PersistenceProblemStepExecution.class);
+            }
+            @Override
+            public String getFunctionName() {
+                return "persistenceProblem";
+            }
+            @Override
+            public String getDisplayName() {
+                return "Problematic Persistence";
+            }
+        }
+        /**
+         * {@link StepExecution} that fails to serialize.
+         *
+         * Used to test the error recovery path of {@link WorkflowJob}.
+         */
+        public static class PersistenceProblemStepExecution extends AbstractStepExecutionImpl {
+            public final Object notSerializable = new Object();
+            private Object writeReplace() {
+                throw new RuntimeException("testing the forced persistence failure behaviour");
+            }
+            @Override
+            public boolean start() throws Exception {
+                return false;
+            }
+            @Override
+            public void stop(Throwable cause) throws Exception {
+                // nothing to do here
+            }
+        }
+    }
+
+    /**
+     * Workflow captures a stateful object, and we verify that it survives the restart
+     */
+    @Test public void persistEphemeralObject() throws Exception {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                jenkins().setNumExecutors(0);
+                DumbSlave s = createSlave(story.j);
+                String nodeName = s.getNodeName();
+
+                p = jenkins().createProject(WorkflowJob.class, "demo");
+                p.setDefinition(new CpsFlowDefinition(
+                    "def s = jenkins.model.Jenkins.instance.getComputer('" + nodeName + "')\n" +
+                    "def r = s.node.rootPath\n" +
+                    "def p = r.getRemote()\n" +
+
+                    "semaphore 'wait'\n" +
+
+                    // make sure these values are still alive
+                    "assert s.nodeName=='" + nodeName + "'\n" +
+                    "assert r.getRemote()==p : r.getRemote() + ' vs ' + p;\n" +
+                    "assert r.channel==s.channel : r.channel.toString() + ' vs ' + s.channel\n"));
+
+                startBuilding();
+                SemaphoreStep.waitForStart("wait/1", b);
+                assertTrue(b.isBuilding());
+            }
+        });
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                rebuildContext(story.j);
+                assertThatWorkflowIsSuspended();
+                SemaphoreStep.success("wait/1", null);
+                story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b));
+            }
+        });
+    }
+
+    @Ignore("TODO java.io.NotSerializableException: java.util.ArrayList$Itr")
+    @Issue("JENKINS-27421")
+    @Test public void listIterator() {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                p = jenkins().createProject(WorkflowJob.class, "demo");
+                p.setDefinition(new CpsFlowDefinition(
+                    "def arr = []; arr += 'one'; arr += 'two'\n" +
+                    "for (int i = 0; i < arr.size(); i++) {def elt = arr[i]; echo \"running C-style loop on ${elt}\"; semaphore \"C-${elt}\"}\n" +
+                    "for (def elt : arr) {echo \"running new-style loop on ${elt}\"; semaphore \"new-${elt}\"}"
+                    , true));
+                ScriptApproval.get().approveSignature("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods plus java.util.Collection java.lang.Object"); // TODO ought to be in generic-whitelist
+                startBuilding();
+                SemaphoreStep.waitForStart("C-one/1", b);
+                story.j.waitForMessage("running C-style loop on one", b);
+            }
+        });
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                rebuildContext(story.j);
+                SemaphoreStep.success("C-one/1", null);
+                SemaphoreStep.success("C-two/1", null);
+                story.j.waitForMessage("running C-style loop on two", b);
+                SemaphoreStep.waitForStart("new-one/1", b);
+                story.j.waitForMessage("running new-style loop on one", b);
+            }
+        });
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                rebuildContext(story.j);
+                SemaphoreStep.success("new-one/1", null);
+                SemaphoreStep.success("new-two/1", null);
+                story.j.waitForCompletion(b);
+                story.j.assertBuildStatusSuccess(b);
+                story.j.assertLogContains("running new-style loop on two", b);
+            }
+        });
+    }
+
+}
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
@@ -29,7 +29,6 @@
 import hudson.model.Queue;
 import hudson.model.TaskListener;
 import hudson.model.User;
-import hudson.slaves.DumbSlave;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -98,44 +97,6 @@ private void liveness() {
         assertFalse(e.isAlive());
     }
 
-    /**
-     * Workflow captures a stateful object, and we verify that it survives the restart
-     */
-    @Test public void persistEphemeralObject() throws Exception {
-        story.addStep(new Statement() {
-            @Override public void evaluate() throws Throwable {
-                jenkins().setNumExecutors(0);
-                DumbSlave s = createSlave(story.j);
-                String nodeName = s.getNodeName();
-
-                p = jenkins().createProject(WorkflowJob.class, "demo");
-                p.setDefinition(new CpsFlowDefinition(
-                    "def s = jenkins.model.Jenkins.instance.getComputer('" + nodeName + "')\n" +
-                    "def r = s.node.rootPath\n" +
-                    "def p = r.getRemote()\n" +
-
-                    "semaphore 'wait'\n" +
-
-                    // make sure these values are still alive
-                    "assert s.nodeName=='" + nodeName + "'\n" +
-                    "assert r.getRemote()==p : r.getRemote() + ' vs ' + p;\n" +
-                    "assert r.channel==s.channel : r.channel.toString() + ' vs ' + s.channel\n"));
-
-                startBuilding();
-                SemaphoreStep.waitForStart("wait/1", b);
-                assertTrue(b.isBuilding());
-            }
-        });
-        story.addStep(new Statement() {
-            @Override public void evaluate() throws Throwable {
-                rebuildContext(story.j);
-                assertThatWorkflowIsSuspended();
-                SemaphoreStep.success("wait/1", null);
-                story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b));
-            }
-        });
-    }
-
     /**
      * ability to invoke body needs to survive beyond Jenkins restart.
      */