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

Incremental builds should include previous builds' modules if previous build failed

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Won't Fix
    • Component/s: maven-plugin
    • Labels:
      None
    • Environment:
      Platform: All, OS: All
    • Similar Issues:

      Description

      (pasted from message
      http://www.nabble.com/Problem-with-incremental-builds-td24799079.html )

      We're making use of the new incremental build feature, and I've noticed a problem.

      Let's say I have two modules, "library" and "app", with "app" depending on
      "library". If a commit fails a unit test in "library", Hudson correctly flags
      the build. However, if a subsequent commit occurs on "app" (and "app" only),
      Hudson will only execute the build of "app", and despite no one having committed
      code to fix the unit test, Hudson reports this subsequent build as "passed".
      This is because Hudson only built "app", and did not run library's unit tests.

      In my opinion, if a previous build has failed, Hudson should keep using this
      build's maven's "pl" argument, appending to it as necessary as more commits are
      made. So, in my example, the first build would be invoked "mvn -pl library"
      (since that's the module that has changed). The second build would be invoked
      "-ol library, app", which is "pl of last build + new changes.

        Attachments

          Issue Links

            Activity

            Hide
            abayer Andrew Bayer added a comment -

            Taking issue - still not entirely sure what the best resolution would be,
            though. While I definitely see your point, I don't know if we really want to
            keep building failed/unstable modules when nothing has changed in them since the
            previous build. The point of the incremental build is to only build those
            modules which have changed since the previous build - if module A has failed
            tests in build 1 but there are no changes in it for build 2, we already know
            what the result will be, so why run the build/tests again? It might make more
            sense to clarify the result of build 2 in that situation - if a module is
            skipped in a given build, reuse the result of its last build. I'll look into
            this more, but probably won't get much (if anything) done before next week.

            Show
            abayer Andrew Bayer added a comment - Taking issue - still not entirely sure what the best resolution would be, though. While I definitely see your point, I don't know if we really want to keep building failed/unstable modules when nothing has changed in them since the previous build. The point of the incremental build is to only build those modules which have changed since the previous build - if module A has failed tests in build 1 but there are no changes in it for build 2, we already know what the result will be, so why run the build/tests again? It might make more sense to clarify the result of build 2 in that situation - if a module is skipped in a given build, reuse the result of its last build. I'll look into this more, but probably won't get much (if anything) done before next week.
            Hide
            hopfrog238 hopfrog238 added a comment -

            > so why run the build/tests again?

            So Hudson can accurately report that the build is still failing.

            I suppose you could simply assume that, if no commits occurred in the failing
            module, subsequent builds are still unstable/failed. This would cut-down on the
            build time. However, in most cases, I'd expect the optimization gained would be
            negligible, and it seems more prudent just to endure the extra build duration.
            This is an area where I think it would be wise to side-step any logic
            determining which modules should be built. Brute-force in the name of caution.

            Whatever the resolution, having a CI Engine report a pass when you're unable to
            checkout and compile should be regarded as an egregious error. At the very
            least, I think providing a warning about the use of this feature is necessary.

            Show
            hopfrog238 hopfrog238 added a comment - > so why run the build/tests again? So Hudson can accurately report that the build is still failing. I suppose you could simply assume that, if no commits occurred in the failing module, subsequent builds are still unstable/failed. This would cut-down on the build time. However, in most cases, I'd expect the optimization gained would be negligible, and it seems more prudent just to endure the extra build duration. This is an area where I think it would be wise to side-step any logic determining which modules should be built. Brute-force in the name of caution. Whatever the resolution, having a CI Engine report a pass when you're unable to checkout and compile should be regarded as an egregious error. At the very least, I think providing a warning about the use of this feature is necessary.
            Hide
            abayer Andrew Bayer added a comment -
                • Issue 4243 has been marked as a duplicate of this issue. ***
            Show
            abayer Andrew Bayer added a comment - Issue 4243 has been marked as a duplicate of this issue. ***
            Hide
            djrobo djrobo added a comment -

            I have to agree that addressing this issue is vital to the usability of the
            incremental builds feature. Development teams have to be able to trust that a
            current successful build on the CI server indicates that the build is not
            secretly in a broken state.

            I can think of three possible ways of dealing with this case. t
            If a build is broken, and a new check-in occurs,
            1. Ignore the incremental builds this build and instead build all modules. This
            certainly has simplicity on its side, and may just be the best answer.
            2. Get the normal module list to build from the SCM changes, but then append to
            this build list the broken modules from the previous build. (Or for simplicity,
            you could append all modules from the previous build, whether they failed or
            not). This is slightly more complex, but would address the issue while
            maintaining the build-only-what-is-necessary mentality of incremental builds.
            3. You can detect if the previous failure took place in a module that will not
            get rebuilt due to not being in the modules/dependency tree that was checked
            into this time. In this case, the build could go as normal, but once complete
            Hudson would sill mark the build as a failure due to the previous build failure.
            This is the most efficient method (in terms of eliminating the rebuilding of
            projects that should still be in failure), but if a module had previously failed
            due to some transient issue in a test (temporary network connectivity issue for
            instance), then you are not giving it another chance that it may succeed on.

            I think clearly option 1 or 2 is the right choice here.

            Show
            djrobo djrobo added a comment - I have to agree that addressing this issue is vital to the usability of the incremental builds feature. Development teams have to be able to trust that a current successful build on the CI server indicates that the build is not secretly in a broken state. I can think of three possible ways of dealing with this case. t If a build is broken, and a new check-in occurs, 1. Ignore the incremental builds this build and instead build all modules. This certainly has simplicity on its side, and may just be the best answer. 2. Get the normal module list to build from the SCM changes, but then append to this build list the broken modules from the previous build. (Or for simplicity, you could append all modules from the previous build, whether they failed or not). This is slightly more complex, but would address the issue while maintaining the build-only-what-is-necessary mentality of incremental builds. 3. You can detect if the previous failure took place in a module that will not get rebuilt due to not being in the modules/dependency tree that was checked into this time. In this case, the build could go as normal, but once complete Hudson would sill mark the build as a failure due to the previous build failure. This is the most efficient method (in terms of eliminating the rebuilding of projects that should still be in failure), but if a module had previously failed due to some transient issue in a test (temporary network connectivity issue for instance), then you are not giving it another chance that it may succeed on. I think clearly option 1 or 2 is the right choice here.
            Hide
            abayer Andrew Bayer added a comment -

            I'm leaning towards #2, and am hoping to have that implemented in time for
            1.321, but it may not make it in until 1.322 - today was spent writing test
            infrastructure for incremental builds. =)

            Show
            abayer Andrew Bayer added a comment - I'm leaning towards #2, and am hoping to have that implemented in time for 1.321, but it may not make it in until 1.322 - today was spent writing test infrastructure for incremental builds. =)
            Hide
            djrobo djrobo added a comment -

            That is great to hear. Option number 2 is certainly what I would have leaned
            towards myself.

            Show
            djrobo djrobo added a comment - That is great to hear. Option number 2 is certainly what I would have leaned towards myself.
            Hide
            abayer Andrew Bayer added a comment -

            Ok, this definitely won't make it in until 1.322 at the earliest - I've got a
            bunch of actual paid work to do, and I'm having trouble getting my test
            SCM-with-changelog working for the ITs for this feature. But I'll keep plugging
            away.

            Show
            abayer Andrew Bayer added a comment - Ok, this definitely won't make it in until 1.322 at the earliest - I've got a bunch of actual paid work to do, and I'm having trouble getting my test SCM-with-changelog working for the ITs for this feature. But I'll keep plugging away.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in hudson
            User: : abayer
            Path:
            trunk/hudson/main/core/src/main/java/hudson/model/Run.java
            trunk/hudson/main/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java
            trunk/hudson/main/test/src/test/java/hudson/maven/MavenMultiModuleTest.java
            trunk/www/changelog.html
            http://fisheye4.cenqua.com/changelog/hudson/?cs=20953
            Log:
            [FIXED JENKINS-4152] Maven incremental build now will rebuild modules which were failed/unstable in previous build, even if there were no changes for those modules in this build.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : abayer Path: trunk/hudson/main/core/src/main/java/hudson/model/Run.java trunk/hudson/main/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java trunk/hudson/main/test/src/test/java/hudson/maven/MavenMultiModuleTest.java trunk/www/changelog.html http://fisheye4.cenqua.com/changelog/hudson/?cs=20953 Log: [FIXED JENKINS-4152] Maven incremental build now will rebuild modules which were failed/unstable in previous build, even if there were no changes for those modules in this build.
            Hide
            woutie wouter wendelen added a comment -

            i don't understand why you not choose option #3 as a solution.

            We currently have a complete build cycle of 4 hours. Whenever a certain module has failing tests and that module is a dependency of all other modules, everyting get's rebuilded on every checkin.

            That is just not correct.

            From my point of view, failed modules that fail because of network stuff or something else should be re-triggered by hand!

            Any option to change this behaviour again?

            Show
            woutie wouter wendelen added a comment - i don't understand why you not choose option #3 as a solution. We currently have a complete build cycle of 4 hours. Whenever a certain module has failing tests and that module is a dependency of all other modules, everyting get's rebuilded on every checkin. That is just not correct. From my point of view, failed modules that fail because of network stuff or something else should be re-triggered by hand! Any option to change this behaviour again?
            Hide
            timdrury Tim Drury added a comment -

            I see this behavior happening in Jenkins 1.596.1.

            A job is configured for incremental build.

            Module A fails due to a broken unit test.
            A change to module B is made.
            The next build builds only module B so the build goes green even though module A's unit test is still broken.

            Unstable and failed modules should be included in the next incremental build (and subsequent builds until the module is stable).

            Show
            timdrury Tim Drury added a comment - I see this behavior happening in Jenkins 1.596.1. A job is configured for incremental build. Module A fails due to a broken unit test. A change to module B is made. The next build builds only module B so the build goes green even though module A's unit test is still broken. Unstable and failed modules should be included in the next incremental build (and subsequent builds until the module is stable).
            Hide
            abayer Andrew Bayer added a comment -

            The Maven plugin is kinda awful, so I'd generally advise not to use it, and definitely don't use the incremental build option. =)

            Show
            abayer Andrew Bayer added a comment - The Maven plugin is kinda awful, so I'd generally advise not to use it, and definitely don't use the incremental build option. =)

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                hopfrog238 hopfrog238
              • Votes:
                1 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: