Merge pull request #343 from FlowCI/fix/1425

fix step failure but goes to next

Author: gy2006

core/src/main/java/com/flowci/core/job/service/JobActionServiceImpl.java

@@ -662,7 +662,7 @@ private void dispatch(Job job, Agent agent) {
     /**
      * Dispatch next step to agent
      *
-     * @return true if next step dispatched, false if no more steps
+     * @return true if next step dispatched, false if no more steps or failure
      */
     private boolean toNextStep(JobSmContext context) {
         Job job = context.job;
@@ -683,7 +683,7 @@ private boolean toNextStep(JobSmContext context) {
         NodeTree tree = ymlManager.getTree(job);
         StepNode node = tree.get(currentPath);
         updateJobTime(job, step, tree, node);
-        updateJobStatusAndContext(job, node, step);
+        updateJobContextAndLatestStatus(job, node, step);
 
         // to next step
         Optional<StepNode> next = findNext(tree, node, step.isSuccess());
@@ -739,7 +739,7 @@ private void updateJobTime(Job job, ExecutedCmd execCmd, NodeTree tree, StepNode
         job.setFinishAt(execCmd.getFinishAt());
     }
 
-    private void updateJobStatusAndContext(Job job, StepNode node, ExecutedCmd cmd) {
+    private void updateJobContextAndLatestStatus(Job job, StepNode node, ExecutedCmd cmd) {
         // merge output to job context
         Vars<String> context = job.getContext();
         context.merge(cmd.getOutput());
@@ -748,15 +748,15 @@ private void updateJobStatusAndContext(Job job, StepNode node, ExecutedCmd cmd)
         context.put(Variables.Job.FinishAt, job.finishAtInStr());
         context.put(Variables.Job.Steps, stepService.toVarString(job, node));
 
-        // after status not apart of job status
+        // latest status saved in context apart from job status property
         job.setStatusToContext(StatusHelper.convert(cmd));
         job.setErrorToContext(cmd.getError());
     }
 
     private Optional<StepNode> findNext(NodeTree tree, Node current, boolean isSuccess) {
         StepNode next = tree.next(current.getPath());
 
-        if (Objects.isNull(next)) {
+        if (Objects.isNull(next) || !isSuccess) {
             return Optional.empty();
         }
 

core/src/test/java/com/flowci/core/test/job/JobServiceTest.java

@@ -360,6 +360,28 @@ public void should_handle_cmd_callback_for_success_status() {
         Assert.assertEquals(Status.SUCCESS, job.getStatus());
     }
 
+    @Test
+    public void should_handle_cmd_callback_for_failure_status() throws IOException {
+        // init: agent and job
+        yml = ymlService.saveYml(flow, StringHelper.toString(load("flow-with-failure.yml")));
+        Agent agent = agentService.create("hello.agent", null, Optional.empty());
+        Job job = prepareJobForRunningStatus(agent);
+
+        NodeTree tree = ymlManager.getTree(job);
+        StepNode firstNode = tree.next(tree.getRoot().getPath());
+        ExecutedCmd firstStep = stepService.get(job.getId(), firstNode.getPathAsString());
+
+        // when: cmd of first node with failure
+        firstStep.setStatus(ExecutedCmd.Status.EXCEPTION);
+        executedCmdDao.save(firstStep);
+        jobEventService.handleCallback(firstStep);
+
+        // then: job should be failure
+        job = jobDao.findById(job.getId()).get();
+        Assert.assertEquals(Status.FAILURE, job.getStatus());
+        Assert.assertEquals("hello/step-1", job.getCurrentPath());
+    }
+
     @Test
     public void should_handle_cmd_callback_for_failure_status_but_allow_failure() throws IOException {
         // init: agent and job

core/src/test/resources/flow-failure-with-after.yml renamed to core/src/test/resources/flow-with-failure.yml

@@ -7,4 +7,7 @@ steps:
     FLOW_WORKSPACE: "echo step"
     FLOW_VERSION: "echo step version"
   script: |
-    echo hello
+    echo shoulde failure
+
+- script: |
+    echo cannot run