Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-41685

incorrect event received with workflow-api 2.8

      After upgrading workflow-api from 2.7 to 2.8, blueocean tests started failing. After debugging with svanoort it was discovered an extra event from next branch was was received (so two input step events for left branch in stead of one) from this sample pipeline script below. It results in to incorrect visualization of steps when user selects parallel branch nodes.

      node {
          stage "hey"
          sh "echo yeah"
          
          stage "par"
          
          parallel left : {
                  sh "echo OMG BS"
                  echo "running"
                  def branchInput = input message: 'Please input branch to test against', parameters: [[$class: 'StringParameterDefinition', defaultValue: 'master', description: '', name: 'branch']]
                  echo "BRANCH NAME: ${branchInput}"
                  sh "echo yeah"
              }, 
              
              right : {
                  sh "echo wozzle"
                  def branchInput = input message: 'MF Please input branch to test against', parameters: [[$class: 'StringParameterDefinition', defaultValue: 'master', description: '', name: 'branch']]
                  echo "BRANCH NAME: ${branchInput}"
              }
          
          stage "ho"
              sh "echo done"
      }
      

      Here are the events generated and this is extra event generated failing test: https://gist.github.com/vivek/ccf3a4ef25fbff267c76c962d265041d#file-event-log-log-L3.

          [JENKINS-41685] incorrect event received with workflow-api 2.8

          Jesse Glick added a comment -

          Presumably due to the fix of JENKINS-38536?

          Jesse Glick added a comment - Presumably due to the fix of JENKINS-38536 ?

          Vivek Pandey added a comment -

          jglick Looks like it.

          svanoort is there fix in progress? This is messing up per node steps visualization in blueocean. A quick fix/release will be great.

          Also as part of the fix better to add/improve tests around ordering/events so that we can catch such regressions early. on.

          Vivek Pandey added a comment - jglick Looks like it. svanoort is there fix in progress? This is messing up per node steps visualization in blueocean. A quick fix/release will be great. Also as part of the fix better to add/improve tests around ordering/events so that we can catch such regressions early. on.

          James Dumay added a comment - - edited

          jglick svanoort this regression is important as it breaks the visualization. Where is this at?

          James Dumay added a comment - - edited jglick svanoort this regression is important as it breaks the visualization. Where is this at?

          Sam Van Oort added a comment -

          Yes, I've been working on this continuously. I've had to extend the test coverage somewhat to address this and some linked concerns, and am doing fixups for the issues encountered so you should get several fixes for the price of one.

          Please kindly avoid double at-pings.

          Sam Van Oort added a comment - Yes, I've been working on this continuously. I've had to extend the test coverage somewhat to address this and some linked concerns, and am doing fixups for the issues encountered so you should get several fixes for the price of one. Please kindly avoid double at-pings.

          Sam Van Oort added a comment -

          vivek After quite a bit of digging, I've got the full cause of the issue, tests that reproduce it and greatly extend test coverage, and think I know how to solve this, JENKINS-38536, and another bug or two in one swoop – though it is quite complex to implement.

          I may have to rewrite some of the more complex internal pieces to do this the right way though.

          Sam Van Oort added a comment - vivek After quite a bit of digging, I've got the full cause of the issue, tests that reproduce it and greatly extend test coverage, and think I know how to solve this, JENKINS-38536 , and another bug or two in one swoop – though it is quite complex to implement. I may have to rewrite some of the more complex internal pieces to do this the right way though.

          James Dumay added a comment -

          Note that we still have these tests failing after upgrading workflow-api

          io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.abortInput
          io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.waitForInputTest
          io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.submitInput
          io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.nodesWithPartialParallels

          James Dumay added a comment - Note that we still have these tests failing after upgrading workflow-api io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.abortInput io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.waitForInputTest io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.submitInput io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeTest.nodesWithPartialParallels

          James Dumay added a comment -

          Can't bump dependencies until we get this one fixed. See results.

          James Dumay added a comment - Can't bump dependencies until we get this one fixed. See results .

          Sam Van Oort added a comment -

          @vivek This is awaiting your review (along with other fixes) in https://github.com/jenkinsci/workflow-api-plugin/pull/33

          Sam Van Oort added a comment - @ vivek This is awaiting your review (along with other fixes) in https://github.com/jenkinsci/workflow-api-plugin/pull/33

          Sam Van Oort added a comment -

          Fixed with explicit test coverage in workflow-api 2.12

          Sam Van Oort added a comment - Fixed with explicit test coverage in workflow-api 2.12

            svanoort Sam Van Oort
            vivek Vivek Pandey
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: