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

Build should not be marked as failed when tests do not pass

    Details

    • Similar Issues:

      Description

      When a build contains tests that have failed, hudson marks the entire build as failed, which is a bit over the top if we look at the definitions on http://wiki.jenkins-ci.org/display/JENKINS/Terminology

      So I think this is not the right behaviour, the result of the build should be 'unstable'. I can imagine this has something to do with the exit code of the grails command that is run by hudson. Is that what should be fixed? If so I will post an issue in the grails jira.

        Attachments

          Activity

          hanswesterbeek hanswesterbeek created issue -
          Hide
          jeffg2one jeffg2one added a comment -

          I think the builder's perform() method is returning false if grails returns non-zero. It should probably should only return false if something really went wrong. Failed tests don't warrant returning false here if I understand the Hudson docs properly.

          Show
          jeffg2one jeffg2one added a comment - I think the builder's perform() method is returning false if grails returns non-zero. It should probably should only return false if something really went wrong. Failed tests don't warrant returning false here if I understand the Hudson docs properly.
          Hide
          hanswesterbeek hanswesterbeek added a comment -

          So you're saying I should report this to the Grails Jira?

          Show
          hanswesterbeek hanswesterbeek added a comment - So you're saying I should report this to the Grails Jira?
          Hide
          xnickmx Nick Martin added a comment -

          I would prefer that the build fails when the tests fail.

          Perhaps there should be a checkbox option that says something like "Treat test failure as unstable" that defaults to false. This would preserve the current behavior while providing an option to treat a build as unstable, as hanswesterbeek suggests.

          Show
          xnickmx Nick Martin added a comment - I would prefer that the build fails when the tests fail. Perhaps there should be a checkbox option that says something like "Treat test failure as unstable" that defaults to false. This would preserve the current behavior while providing an option to treat a build as unstable, as hanswesterbeek suggests.
          Hide
          hanswesterbeek hanswesterbeek added a comment -

          Nick I don't think that's a good idea. In regular java projects a build is not marked 'failed' when one tests fails, but 'unstable'. Grails should do the same. Check the Jenkins terminology page.

          Show
          hanswesterbeek hanswesterbeek added a comment - Nick I don't think that's a good idea. In regular java projects a build is not marked 'failed' when one tests fails, but 'unstable'. Grails should do the same. Check the Jenkins terminology page.
          Hide
          glen_a_smith Glen Smith added a comment -

          I'm with @hanswesterbeek on this. A "broken" (red) build in Jenkins usually means that the build process itself is failing (and this is good to know about if you're doing a Grails "autoupdate" build and pulling in tons of plugins which might cause the Grails build itself to fail for a variety of reasons).

          I like the idea of the Grails plugin matching the standard Java one: build process is broken, show red; tests are failing, show yellow.

          We can always stop downstream jobs from running if tests are failing, so @Nick's scenario is supported by Jenkins out of the box even if we get this issue implemented to match the Java project behaviour.

          Show
          glen_a_smith Glen Smith added a comment - I'm with @hanswesterbeek on this. A "broken" (red) build in Jenkins usually means that the build process itself is failing (and this is good to know about if you're doing a Grails "autoupdate" build and pulling in tons of plugins which might cause the Grails build itself to fail for a variety of reasons). I like the idea of the Grails plugin matching the standard Java one: build process is broken, show red; tests are failing, show yellow. We can always stop downstream jobs from running if tests are failing, so @Nick's scenario is supported by Jenkins out of the box even if we get this issue implemented to match the Java project behaviour.
          Hide
          fletchgqc John Fletcher added a comment -

          hanswesterbeek is correct. If you want any other behavior, then you must change the Terminology page.

          Show
          fletchgqc John Fletcher added a comment - hanswesterbeek is correct. If you want any other behavior, then you must change the Terminology page.
          Hide
          alxndrsn Alex Anderson added a comment -

          jeff2one said "I think the builder's perform() method is returning false if grails returns non-zero." Where would I find the code for this builder? More than happy to take a look if someone can give me some pointers.

          Show
          alxndrsn Alex Anderson added a comment - jeff2one said "I think the builder's perform() method is returning false if grails returns non-zero." Where would I find the code for this builder? More than happy to take a look if someone can give me some pointers.
          Hide
          hanswesterbeek hanswesterbeek added a comment - - edited

          Hi Alex... I got it from here: https://github.com/jenkinsci/grails-plugin

          Also, I have done some more in-depth research and posted to the grails-dev mailinglist about my findings until I got short on time. You might find them useful:
          http://grails.1312388.n4.nabble.com/jenkins-plugin-and-grails-commandline-exit-code-td3773354.html

          Show
          hanswesterbeek hanswesterbeek added a comment - - edited Hi Alex... I got it from here: https://github.com/jenkinsci/grails-plugin Also, I have done some more in-depth research and posted to the grails-dev mailinglist about my findings until I got short on time. You might find them useful: http://grails.1312388.n4.nabble.com/jenkins-plugin-and-grails-commandline-exit-code-td3773354.html
          Hide
          fk Evaldas Miliauskas added a comment -

          Another valid reason to have this, when you want to have constant deployments on dev servers even if test fails. But you still prevent from unstable versions going to qa environment etc.

          Show
          fk Evaldas Miliauskas added a comment - Another valid reason to have this, when you want to have constant deployments on dev servers even if test fails. But you still prevent from unstable versions going to qa environment etc.
          Hide
          alxndrsn Alex Anderson added a comment - - edited

          Would the following change be suitable to https://github.com/jenkinsci/grails-plugin/blob/master/src/main/java/com/g2one/hudson/grails/GrailsBuilder.java#L224?

          if (r != 0) return false;
          

          to

          if (r != 0) {
              if (isBuildFailureDueToFailingTests()) {
                  listener.finished(Result.UNSTABLE);
              } else {
                  return false;
              }
          }
          

          If this isn't going to be fixed in Grails, `isBuildFailureDueToFailingTests()` could analyse the console output of the grails build and check for relevant string ("Tests PASSED" / "Tests FAILED"). What is a suitable approach for this? Presumably Jenkins is already capturing this output somewhere.

          Show
          alxndrsn Alex Anderson added a comment - - edited Would the following change be suitable to https://github.com/jenkinsci/grails-plugin/blob/master/src/main/java/com/g2one/hudson/grails/GrailsBuilder.java#L224? if (r != 0) return false; to if (r != 0) { if (isBuildFailureDueToFailingTests()) { listener.finished(Result.UNSTABLE); } else { return false; } } If this isn't going to be fixed in Grails, `isBuildFailureDueToFailingTests()` could analyse the console output of the grails build and check for relevant string ("Tests PASSED" / "Tests FAILED"). What is a suitable approach for this? Presumably Jenkins is already capturing this output somewhere.
          Show
          alxndrsn Alex Anderson added a comment - (Have posted further thoughts to http://grails.1312388.n4.nabble.com/jenkins-plugin-and-grails-commandline-exit-code-td3773354.html )
          Hide
          alxndrsn Alex Anderson added a comment -

          Now have a Jenkins JIRA ticket to fix the response code: http://jira.grails.org/browse/GRAILS-9481.

          Would be great to get a fix earlier than that though for projects using older versions of grails.

          Show
          alxndrsn Alex Anderson added a comment - Now have a Jenkins JIRA ticket to fix the response code: http://jira.grails.org/browse/GRAILS-9481 . Would be great to get a fix earlier than that though for projects using older versions of grails.
          Hide
          alxndrsn Alex Anderson added a comment -

          Have fixed this and submitted a pull request on Github: https://github.com/jenkinsci/grails-plugin/pull/3

          Show
          alxndrsn Alex Anderson added a comment - Have fixed this and submitted a pull request on Github: https://github.com/jenkinsci/grails-plugin/pull/3
          Hide
          gpinkham Gary Pinkham added a comment -

          @Alex.. forgive me for muddying Jira with the question but seems like the one that gets installed via the plugins interface in jenkins is quite old.. how does one get the version with your changes.. just clone from github and build myself? (I should probably RTFM eh?)

          Show
          gpinkham Gary Pinkham added a comment - @Alex.. forgive me for muddying Jira with the question but seems like the one that gets installed via the plugins interface in jenkins is quite old.. how does one get the version with your changes.. just clone from github and build myself? (I should probably RTFM eh?)
          Hide
          alxndrsn Alex Anderson added a comment -

          IIRC (it's been a few months), `git clone ___ jenkins-grails; cd jenkins-grails; mvn package`.

          This should make an hpi file in ./target/, which you can then install via the Jenkins management GUI.

          Show
          alxndrsn Alex Anderson added a comment - IIRC (it's been a few months), `git clone ___ jenkins-grails; cd jenkins-grails; mvn package`. This should make an hpi file in ./target/, which you can then install via the Jenkins management GUI.
          rtyler R. Tyler Croy made changes -
          Field Original Value New Value
          Workflow JNJira [ 138645 ] JNJira + In-Review [ 174947 ]

            People

            • Assignee:
              jeffg2one jeffg2one
              Reporter:
              hanswesterbeek hanswesterbeek
            • Votes:
              5 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: