Hadoop Common
  1. Hadoop Common
  2. HADOOP-10325

improve jenkins javadoc warnings from test-patch.sh

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0
    • Fix Version/s: 3.0.0
    • Component/s: None
    • Labels:
      None
    • Target Version/s:

      Description

      Currently test-patch.sh uses OK_JAVADOC_WARNINGS to know how many warnings trunk is expected to have. However, this is a fragile and difficult to use system, since different build slaves may generate different numbers of warnings (based on compiler revision, etc.). Also, programmers must remember to update OK_JAVADOC_WARNINGS, which they don't always. Finally, there is no easy way to find what the new javadoc warnings are in the huge pile of warnings.

      We should change this to work the same way the javac warnings code does: to simply build with and without the patch and do a diff. The diff should be saved for easy perusal. We also should not complain about warnings being removed.

      1. HADOOP-10325.001.patch
        3 kB
        Colin Patrick McCabe
      2. HADOOP-10325.002.patch
        3 kB
        Colin Patrick McCabe
      3. HADOOP-10325.003.patch
        3 kB
        Colin Patrick McCabe
      4. HADOOP-10325.004.patch
        4 kB
        Colin Patrick McCabe

        Activity

        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1690 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1690/)
        HADOOP-10325. Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010)

        • /hadoop/common/trunk/dev-support/test-patch.sh
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1690 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1690/ ) HADOOP-10325 . Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010 ) /hadoop/common/trunk/dev-support/test-patch.sh /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #1665 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1665/)
        HADOOP-10325. Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010)

        • /hadoop/common/trunk/dev-support/test-patch.sh
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1665 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1665/ ) HADOOP-10325 . Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010 ) /hadoop/common/trunk/dev-support/test-patch.sh /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #473 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/473/)
        HADOOP-10325. Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010)

        • /hadoop/common/trunk/dev-support/test-patch.sh
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #473 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/473/ ) HADOOP-10325 . Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010 ) /hadoop/common/trunk/dev-support/test-patch.sh /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #5115 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5115/)
        HADOOP-10325. Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010)

        • /hadoop/common/trunk/dev-support/test-patch.sh
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #5115 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5115/ ) HADOOP-10325 . Improve Jenkins Javadoc warnings from test-patch.sh (cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1565010 ) /hadoop/common/trunk/dev-support/test-patch.sh /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12627228/HADOOP-10325.004.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3540//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3540//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627228/HADOOP-10325.004.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3540//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3540//console This message is automatically generated.
        Hide
        Andrew Wang added a comment -

        +1 pending Jenkins, nice work Colin! I'm looking forward to a more intelligent QA bot.

        Show
        Andrew Wang added a comment - +1 pending Jenkins, nice work Colin! I'm looking forward to a more intelligent QA bot.
        Hide
        Colin Patrick McCabe added a comment -

        Good idea. Let me grep for "warning" first. I'll make it case insensitive to avoid being too selective. (The reason why I think case insensitive is better is that I think it's better to allow a little spew through than to chop something really helpful.) The main point is just to let us find the actual problem in 30 seconds or less, rather than having to search every warning or download logs.

        Show
        Colin Patrick McCabe added a comment - Good idea. Let me grep for "warning" first. I'll make it case insensitive to avoid being too selective. (The reason why I think case insensitive is better is that I think it's better to allow a little spew through than to chop something really helpful.) The main point is just to let us find the actual problem in 30 seconds or less, rather than having to search every warning or download logs.
        Hide
        Andrew Wang added a comment -

        Sorry about the RAT warnings, I just fixed the missing license header in the test file.

        One q about the diff though, can we do a bit better than diffing the full maven outputs? Maybe I missed where it filters, but a full diff doesn't seem that helpful. I was thinking that a {{grep -A $(( 3 * numWarnings )) }} for each "# warnings" line thing would be accurate enough, or we just do like the old script and chop+grep and diff that.

        Show
        Andrew Wang added a comment - Sorry about the RAT warnings, I just fixed the missing license header in the test file. One q about the diff though, can we do a bit better than diffing the full maven outputs? Maybe I missed where it filters, but a full diff doesn't seem that helpful. I was thinking that a {{grep -A $(( 3 * numWarnings )) }} for each "# warnings" line thing would be accurate enough, or we just do like the old script and chop+grep and diff that.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12627179/HADOOP-10325.003.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 1 release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627179/HADOOP-10325.003.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 1 release audit warnings. +1 core tests . The patch passed unit tests in . +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3539//console This message is automatically generated.
        Hide
        Andrew Wang added a comment -

        That'd definitely be nice, agree. It's unfortunate that the # of lines per warning is variable, else we could do very accurate diffs with grep -A.

        I'm fine with whatever you come up with diff wise though, assuming it mostly works.

        Show
        Andrew Wang added a comment - That'd definitely be nice, agree. It's unfortunate that the # of lines per warning is variable, else we could do very accurate diffs with grep -A . I'm fine with whatever you come up with diff wise though, assuming it mostly works.
        Hide
        Colin Patrick McCabe added a comment -

        OK. I agree that summing the 'N warnings' messages is probably best. I still want to provide a diff between old and new warning files to make things easier on developers-- even if it may contain a few spurious entries in a patch that adds javac warnings.

        Show
        Colin Patrick McCabe added a comment - OK. I agree that summing the 'N warnings' messages is probably best. I still want to provide a diff between old and new warning files to make things easier on developers-- even if it may contain a few spurious entries in a patch that adds javac warnings.
        Hide
        Andrew Wang added a comment -

        I looked at the previous and new proposed behavior for this patch, and I don't think either are really counting the number of Javadoc warnings. The old behavior looks at the full maven output, chops off everything after the first "Javadoc Warnings", and then counts anything with "[WARNING]" and "warning" in it. Unfortunately, that includes javac warnings as well. The new behavior doesn't bother with the chop, and thus results in counting more warnings, but still not the actual number of Javadoc warnings.

        I tried this, and it seems to work more reliably:

        cat patchJavadocWarnings.txt | egrep "^[0-9]+ warnings$" | awk '{sum+=$1} END {print sum}'
        

        We could also be more sure by doing something like "egrep -A 1 ..." and making sure the same number of " # warnings" lines matches the number of lines with "[WARNING] Javadoc Warnings" (i.e., each "# warning" line has an immediately following "[WARNING] Javadoc Warnings").

        Show
        Andrew Wang added a comment - I looked at the previous and new proposed behavior for this patch, and I don't think either are really counting the number of Javadoc warnings. The old behavior looks at the full maven output, chops off everything after the first "Javadoc Warnings", and then counts anything with " [WARNING] " and "warning" in it. Unfortunately, that includes javac warnings as well. The new behavior doesn't bother with the chop, and thus results in counting more warnings, but still not the actual number of Javadoc warnings. I tried this, and it seems to work more reliably: cat patchJavadocWarnings.txt | egrep "^[0-9]+ warnings$" | awk '{sum+=$1} END {print sum}' We could also be more sure by doing something like "egrep -A 1 ..." and making sure the same number of " # warnings" lines matches the number of lines with " [WARNING] Javadoc Warnings" (i.e., each "# warning" line has an immediately following " [WARNING] Javadoc Warnings").
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12627039/HADOOP-10325.002.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3536//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3536//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627039/HADOOP-10325.002.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3536//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3536//console This message is automatically generated.
        Hide
        Colin Patrick McCabe added a comment -

        btw, I ran a local test on v2 of this and it seems to work pretty well.

        Show
        Colin Patrick McCabe added a comment - btw, I ran a local test on v2 of this and it seems to work pretty well.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12627016/HADOOP-10325.001.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3534//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3534//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12627016/HADOOP-10325.001.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3534//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3534//console This message is automatically generated.
        Hide
        Colin Patrick McCabe added a comment -

        fix typo

        Show
        Colin Patrick McCabe added a comment - fix typo

          People

          • Assignee:
            Colin Patrick McCabe
            Reporter:
            Colin Patrick McCabe
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development