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

      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.

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

          Basil Crow added a comment -

          Thanks for reporting this, myon. 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.

          Basil Crow added a comment - Thanks for reporting this, myon . 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.

          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.

          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.

          Basil Crow added a comment -

          Hey myon, 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.

          Basil Crow added a comment - Hey myon , 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.

          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.

           

          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.  

          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.

          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.

          I just stumbled over this again. The part I had not realized yet is that on matrix jobs, "not built" set by text finder will even hide failing combinations. I'm now disabling it for my jobs where this combination can happen, but I'd still be interested in a solution.

          Christoph Berg added a comment - I just stumbled over this again. The part I had not realized yet is that on matrix jobs, "not built" set by text finder will even hide failing combinations. I'm now disabling it for my jobs where this combination can happen, but I'd still be interested in a solution.

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

              Created:
              Updated: