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

text-finder sets whole matrix job to "not built" even if only one configuration matched

    Details

    • Similar Issues:

      Description

      When using the Text Finder Plugin with matrix jobs, if one configuration was set to "not built" from this plugin, the whole build will be marked as "not built", even if there were other configurations that passed successfully.

      My expectation would be that matrix jobs should be blue/green if there's any configurations that passed, and only be "not built" themselves if all configurations had that result.

        Attachments

          Activity

          Hide
          basil Basil Crow added a comment -

          Thanks for reporting this, Christoph Berg. Would you be willing to write a unit test that reproduces the issue, marked with @Ignore and @Issue("JENKINS-59730")? The existing tests should serve as a good example to follow, modulo using using MatrixProject instead of FreestyleProject via something like MatrixProject createMatrixProject = jenkins.createProject(MatrixProject.class, "mp"). This would greatly facilitate getting the issue fixed.

          Show
          basil Basil Crow added a comment - Thanks for reporting this, Christoph Berg . Would you be willing to write a unit test that reproduces the issue, marked with @Ignore and @Issue(" JENKINS-59730 ") ? The existing tests should serve as a good example to follow, modulo using using MatrixProject instead of FreestyleProject via something like MatrixProject createMatrixProject = jenkins.createProject(MatrixProject.class, "mp") . This would greatly facilitate getting the issue fixed.
          Hide
          myon Christoph Berg added a comment -

          I have little Java experience so I can't tell how much harder than copy-pasting some similar case would be. I might give it a try, but I'll be at PGconf.EU next week, so it might take a while before I get to it, if at all.

          Show
          myon Christoph Berg added a comment - I have little Java experience so I can't tell how much harder than copy-pasting some similar case would be. I might give it a try, but I'll be at PGconf.EU next week, so it might take a while before I get to it, if at all.
          Hide
          basil Basil Crow added a comment -

          Hey Christoph Berg, I looked into this further and saw that DefaultMatrixExecutionStrategy is setting the build result by using Result#combine, which "combines two Results and returns the worse one", where "worseness" is defined by a simple ordinal system in Jenkins core:

          public static final @Nonnull Result SUCCESS = new Result("SUCCESS",BallColor.BLUE,0,true);
          public static final @Nonnull Result UNSTABLE = new Result("UNSTABLE",BallColor.YELLOW,1,true);
          public static final @Nonnull Result FAILURE = new Result("FAILURE",BallColor.RED,2,true);
          public static final @Nonnull Result NOT_BUILT = new Result("NOT_BUILT",BallColor.NOTBUILT,3,false);
          public static final @Nonnull Result ABORTED = new Result("ABORTED",BallColor.ABORTED,4,false);
          

          Based on the semantics of Result#isWorseThan, NOT_BUILT is considered "worse than" SUCCESS, hence the behavior you are experiencing. A similar problem exists in workflow-cps plugin when using the parallel step without failFast. As of workflow-cps-plugin#325, the parallel step propagates the "worst" result to the overall build (just like with the Matrix Project plugin), but it uses the same definition of worseness provided by Jenkins core and hence has the same behavior you are seeing here.

          Coincidentally, I was the one who implemented workflow-cps-plugin#325, and I noticed this annoying behavior while writing the tests for that change. I proposed a workaround in workflow-cps-plugin. Maintainer Devin Nusbaum responded as follows:

          To elaborate on my earlier comment, the values in Result have always seemed weird to me. ABORTED and NOT_BUILT feel like a different kind of thing entirely than SUCCESS, FAILURE, and UNSTABLE. Sometimes I think it would have made more sense for results to have 2 parts, a "completion" result, like , NOT_BUILT, ABORTED, COMPLETED, and a "goodness" result, like SUCCESS, WARNING, ERROR. Then you could have something like a Pipeline step that is ABORTED and SUCCESS in the case of a user cancelling the build, or ABORTED and WARNING in the case of something like a timeout, NOT_BUILT and SUCCESS could be used in cases like a Declarative when expression that is false, and here in the parallel step, the completion result would always be COMPLETED, and only the "goodness" result would be determined by the results of the branches. Using two values creates other problems though, and not all combinations make sense. So all that to say, I agree that the behavior here with NOT_BUILT feels wrong, but I'm not sure if it makes sense to try to change it.

          Based on this response, I don't think there's any way to fix this bug from within the Text Finder plugin or even the Matrix Project plugin. The flaw is fundamental to the semantics of the Jenkins Result API.

          Show
          basil Basil Crow added a comment - Hey Christoph Berg , I looked into this further and saw that  DefaultMatrixExecutionStrategy is setting the build result by using Result#combine , which "combines two Result s and returns the worse one", where "worseness" is defined by a simple ordinal system in Jenkins core: public static final @Nonnull Result SUCCESS = new Result( "SUCCESS" ,BallColor.BLUE,0, true ); public static final @Nonnull Result UNSTABLE = new Result( "UNSTABLE" ,BallColor.YELLOW,1, true ); public static final @Nonnull Result FAILURE = new Result( "FAILURE" ,BallColor.RED,2, true ); public static final @Nonnull Result NOT_BUILT = new Result( "NOT_BUILT" ,BallColor.NOTBUILT,3, false ); public static final @Nonnull Result ABORTED = new Result( "ABORTED" ,BallColor.ABORTED,4, false ); Based on the semantics of Result#isWorseThan , NOT_BUILT is considered "worse than" SUCCESS , hence the behavior you are experiencing. A similar problem exists in workflow-cps plugin when using the parallel step without failFast . As of workflow-cps-plugin#325 , the parallel step propagates the "worst" result to the overall build (just like with the Matrix Project plugin), but it uses the same definition of worseness provided by Jenkins core and hence has the same behavior you are seeing here. Coincidentally, I was the one who implemented workflow-cps-plugin#325 , and I noticed this annoying behavior while writing the tests for that change. I proposed a workaround in workflow-cps-plugin . Maintainer Devin Nusbaum responded as follows : To elaborate on my earlier comment, the values in Result have always seemed weird to me. ABORTED and NOT_BUILT feel like a different kind of thing entirely than SUCCESS , FAILURE , and UNSTABLE . Sometimes I think it would have made more sense for results to have 2 parts, a "completion" result, like , NOT_BUILT , ABORTED , COMPLETED , and a "goodness" result, like SUCCESS , WARNING , ERROR . Then you could have something like a Pipeline step that is ABORTED and SUCCESS in the case of a user cancelling the build, or ABORTED and WARNING in the case of something like a timeout, NOT_BUILT and SUCCESS could be used in cases like a Declarative when expression that is false, and here in the parallel step, the completion result would always be COMPLETED , and only the "goodness" result would be determined by the results of the branches. Using two values creates other problems though, and not all combinations make sense. So all that to say, I agree that the behavior here with NOT_BUILT feels wrong, but I'm not sure if it makes sense to try to change it. Based on this response, I don't think there's any way to fix this bug from within the Text Finder plugin or even the Matrix Project plugin. The flaw is fundamental to the semantics of the Jenkins Result API.
          Hide
          myon Christoph Berg added a comment -

          Thanks for looking into this.

          I understand the technical issues, but wanted to note a (maybe trivial) observation: Since the "Matrix Reloaded Plugin" is able to re-run matrix jobs partially, and the "Matrix Combinations Plugin" is even able to start only parts of a matrix, maybe there is some way in the Matrix Plugin to make "Not Built" and "hasn't built in this run" the same thing, especially as these are separate plugins from the Matrix Plugin itself.

           

          Show
          myon Christoph Berg added a comment - Thanks for looking into this. I understand the technical issues, but wanted to note a (maybe trivial) observation: Since the " Matrix Reloaded Plugin " is able to re-run matrix jobs partially, and the " Matrix Combinations Plugin " is even able to start only parts of a matrix, maybe there is some way in the Matrix Plugin to make "Not Built" and "hasn't built in this run" the same thing, especially as these are separate plugins from the Matrix Plugin itself.  
          Hide
          basil Basil Crow added a comment -

          maybe there is some way in the Matrix Plugin to make "Not Built" and "hasn't built in this run" the same thing, especially as these are separate plugins from the Matrix Plugin itself

          Great question! I am moving this issue from the "Text Finder" component to the "Matrix Plugin" component. Perhaps the maintainer of the Matrix Plugin has an answer.

          Show
          basil Basil Crow added a comment - maybe there is some way in the Matrix Plugin to make "Not Built" and "hasn't built in this run" the same thing, especially as these are separate plugins from the Matrix Plugin itself Great question! I am moving this issue from the "Text Finder" component to the "Matrix Plugin" component. Perhaps the maintainer of the Matrix Plugin has an answer.

            People

            • Assignee:
              Unassigned
              Reporter:
              myon Christoph Berg
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: