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

incorrect event received with workflow-api 2.8

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      After upgrading workflow-api from 2.7 to 2.8, blueocean tests started failing. After debugging with Sam Van Oort 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.

        Attachments

          Issue Links

            Activity

            vivek Vivek Pandey created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Labels regression
            Hide
            jglick Jesse Glick added a comment -

            Presumably due to the fix of JENKINS-38536?

            Show
            jglick Jesse Glick added a comment - Presumably due to the fix of JENKINS-38536 ?
            jglick Jesse Glick made changes -
            Link This issue blocks JENKINS-38536 [ JENKINS-38536 ]
            Hide
            vivek Vivek Pandey added a comment -

            Jesse Glick Looks like it.

            Sam Van Oort 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.

            Show
            vivek Vivek Pandey added a comment - Jesse Glick Looks like it. Sam Van Oort 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.
            Hide
            jamesdumay James Dumay added a comment - - edited

            Jesse Glick Sam Van Oort this regression is important as it breaks the visualization. Where is this at?

            Show
            jamesdumay James Dumay added a comment - - edited Jesse Glick Sam Van Oort this regression is important as it breaks the visualization. Where is this at?
            Hide
            svanoort 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.

            Show
            svanoort 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.
            Hide
            svanoort Sam Van Oort added a comment -

            Vivek Pandey 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.

            Show
            svanoort Sam Van Oort added a comment - Vivek Pandey 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.
            Hide
            jamesdumay 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

            Show
            jamesdumay 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
            Hide
            jamesdumay James Dumay added a comment -

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

            Show
            jamesdumay James Dumay added a comment - Can't bump dependencies until we get this one fixed. See results .
            svanoort Sam Van Oort made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            svanoort Sam Van Oort made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            Hide
            svanoort Sam Van Oort added a comment -

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

            Show
            svanoort Sam Van Oort added a comment - @ Vivek Pandey This is awaiting your review (along with other fixes) in https://github.com/jenkinsci/workflow-api-plugin/pull/33
            recampbell Ryan Campbell made changes -
            Remote Link This issue links to "PR (Web Link)" [ 15480 ]
            Hide
            svanoort Sam Van Oort added a comment -

            Fixed with explicit test coverage in workflow-api 2.12

            Show
            svanoort Sam Van Oort added a comment - Fixed with explicit test coverage in workflow-api 2.12
            svanoort Sam Van Oort made changes -
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Closed [ 6 ]
            cloudbees CloudBees Inc. made changes -
            Remote Link This issue links to "CloudBees Internal OSS-1975 (Web Link)" [ 18488 ]
            cloudbees CloudBees Inc. made changes -
            Remote Link This issue links to "CloudBees Internal CLTS-1257 (Web Link)" [ 19163 ]

              People

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

                Dates

                Created:
                Updated:
                Resolved: