Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12030

test-patch should only report on newly introduced findbugs warnings.

    Details

    • Type: Test
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: None
    • Labels:

      Description

      findbugs is currently reporting the total number of findbugs warnings for touched modules rather than just newly introduced bugs.

      1. HADOOP-12030.1.patch
        12 kB
        Sean Busbey
      2. HADOOP-12030.2.patch
        13 kB
        Sean Busbey

        Issue Links

          Activity

          Hide
          busbey Sean Busbey added a comment -

          -01

          • moves findbugs into a plugin
          • on preapply calculates findbugs report for changed modules
          • on postapply calculates the diff

          one thing I'm not sure of is passing in FINDBUGS_HOME as a cli arg. there's no good way to move that into a plugin.

          Show
          busbey Sean Busbey added a comment - -01 moves findbugs into a plugin on preapply calculates findbugs report for changed modules on postapply calculates the diff one thing I'm not sure of is passing in FINDBUGS_HOME as a cli arg. there's no good way to move that into a plugin.
          Hide
          hadoopqa Hadoop QA added a comment -

          A patch to test-patch or smart-apply-patch has been detected.
          Re-executing against the patched versions to perform further tests.
          The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/console in case of problems.

          Show
          hadoopqa Hadoop QA added a comment - A patch to test-patch or smart-apply-patch has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/console in case of problems.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s dev-support patch detected.
          0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
          0 @author 0m 0s Skipping @author checks as test-patch has been patched.
          +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings.
          -1 shellcheck 0m 8s The applied patch generated 3 new shellcheck (v0.3.3) issues (total was 38, now 40).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
              0m 35s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12735132/HADOOP-12030.1.patch
          Optional Tests shellcheck
          git revision trunk / ada233b
          shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/artifact/patchprocess/diffpatchshellcheck.txt
          Java 1.7.0_55
          uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s dev-support patch detected. 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. 0 @author 0m 0s Skipping @author checks as test-patch has been patched. +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings. -1 shellcheck 0m 8s The applied patch generated 3 new shellcheck (v0.3.3) issues (total was 38, now 40). +1 whitespace 0m 0s The patch has no lines that end in whitespace.     0m 35s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12735132/HADOOP-12030.1.patch Optional Tests shellcheck git revision trunk / ada233b shellcheck https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/artifact/patchprocess/diffpatchshellcheck.txt Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6817/console This message was automatically generated.
          Hide
          aw Allen Wittenauer added a comment -

          I'm not sure this is a good idea. We've definitely seen patches that pass findbugs only to come back in a subsequent test run blow up with errors. Plus findbugs are one of those things that being constantly reminded to fix is a good thing.

          Also:

          one thing I'm not sure of is passing in FINDBUGS_HOME as a cli arg. there's no good way to move that into a plugin.

          This is why findbugs wasn't made a plugin earlier. I always meant to go back and add some sort of plugin hook into parse_args but never did.

          Show
          aw Allen Wittenauer added a comment - I'm not sure this is a good idea. We've definitely seen patches that pass findbugs only to come back in a subsequent test run blow up with errors. Plus findbugs are one of those things that being constantly reminded to fix is a good thing. Also: one thing I'm not sure of is passing in FINDBUGS_HOME as a cli arg. there's no good way to move that into a plugin. This is why findbugs wasn't made a plugin earlier. I always meant to go back and add some sort of plugin hook into parse_args but never did.
          Hide
          busbey Sean Busbey added a comment -

          -02

          • moves findbugs back into test-patch proper
          • adds opt-in failure of pre-patch when extant warnings
          Show
          busbey Sean Busbey added a comment - -02 moves findbugs back into test-patch proper adds opt-in failure of pre-patch when extant warnings
          Hide
          busbey Sean Busbey added a comment -

          I agree that it's important to stay on top of findbugs problems, but it's also important that we properly indicate things that are problems before the patch and problems with the patch. otherwise folks will think of our warnings as false positives and stop listening to us.

          I'm also torn on failing pre-patch over findbugs. Nightlies are really the place to flag issues with the main code base, though I get the advantage of how much more visible precommit QA is. Also, projects that want to be strict about findbugs status could always tie things into their main build so that pre-patch javac would fail with findbugs warnings anyways (and they could even do this just in QA by using the project patch process profile).

          Anyhwo, I checked this version against HBase with HBASE-13716 with and without the cli option and it behaved correctly in both cases, either just saying "everything is fine" or "before this patch there are 60-ish findbugs things wrong"

          Show
          busbey Sean Busbey added a comment - I agree that it's important to stay on top of findbugs problems, but it's also important that we properly indicate things that are problems before the patch and problems with the patch. otherwise folks will think of our warnings as false positives and stop listening to us. I'm also torn on failing pre-patch over findbugs. Nightlies are really the place to flag issues with the main code base, though I get the advantage of how much more visible precommit QA is. Also, projects that want to be strict about findbugs status could always tie things into their main build so that pre-patch javac would fail with findbugs warnings anyways (and they could even do this just in QA by using the project patch process profile). Anyhwo, I checked this version against HBase with HBASE-13716 with and without the cli option and it behaved correctly in both cases, either just saying "everything is fine" or "before this patch there are 60-ish findbugs things wrong"
          Hide
          hadoopqa Hadoop QA added a comment -

          A patch to test-patch or smart-apply-patch has been detected.
          Re-executing against the patched versions to perform further tests.
          The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/6825/console in case of problems.

          Show
          hadoopqa Hadoop QA added a comment - A patch to test-patch or smart-apply-patch has been detected. Re-executing against the patched versions to perform further tests. The console is at https://builds.apache.org/job/PreCommit-HADOOP-Build/6825/console in case of problems.
          Hide
          hadoopqa Hadoop QA added a comment -



          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s dev-support patch detected.
          0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
          0 @author 0m 0s Skipping @author checks as test-patch has been patched.
          +1 release audit 0m 14s The applied patch does not increase the total number of release audit warnings.
          +1 shellcheck 0m 9s There were no new shellcheck (v0.3.3) issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
              0m 27s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12735307/HADOOP-12030.2.patch
          Optional Tests shellcheck
          git revision trunk / 9a3d617
          Java 1.7.0_55
          uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6825/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s dev-support patch detected. 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. 0 @author 0m 0s Skipping @author checks as test-patch has been patched. +1 release audit 0m 14s The applied patch does not increase the total number of release audit warnings. +1 shellcheck 0m 9s There were no new shellcheck (v0.3.3) issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace.     0m 27s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12735307/HADOOP-12030.2.patch Optional Tests shellcheck git revision trunk / 9a3d617 Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6825/console This message was automatically generated.
          Hide
          aw Allen Wittenauer added a comment -

          + 1 committed

          thanks

          Show
          aw Allen Wittenauer added a comment - + 1 committed thanks
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7916 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7916/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • dev-support/test-patch.sh
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7916 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7916/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) hadoop-common-project/hadoop-common/CHANGES.txt dev-support/test-patch.sh
          Hide
          busbey Sean Busbey added a comment -

          I updated the precommit H/H/Y/M builds to all include {{--findbugs-strict-precheck }}

          Show
          busbey Sean Busbey added a comment - I updated the precommit H/H/Y/M builds to all include {{--findbugs-strict-precheck }}
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #200 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/200/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • dev-support/test-patch.sh
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #200 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/200/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) hadoop-common-project/hadoop-common/CHANGES.txt dev-support/test-patch.sh
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #212 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/212/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • dev-support/test-patch.sh
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #212 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/212/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) hadoop-common-project/hadoop-common/CHANGES.txt dev-support/test-patch.sh
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #942 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/942/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • dev-support/test-patch.sh
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #942 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/942/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) hadoop-common-project/hadoop-common/CHANGES.txt dev-support/test-patch.sh
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2140 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2140/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • dev-support/test-patch.sh
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2140 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2140/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) dev-support/test-patch.sh hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #210 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/210/)
          HADOOP-12030. test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • dev-support/test-patch.sh
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #210 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/210/ ) HADOOP-12030 . test-patch should only report on newly introduced findbugs warnings. (Sean Busbey via aw) (aw: rev b01d33cf862a34f9988584d3d1f3995118110b90) hadoop-common-project/hadoop-common/CHANGES.txt dev-support/test-patch.sh

            People

            • Assignee:
              busbey Sean Busbey
              Reporter:
              busbey Sean Busbey
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development