Bookkeeper
  1. Bookkeeper
  2. BOOKKEEPER-483

precommit tests only check toplevel rat file, not the one for submodules.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.2.0
    • Component/s: None
    • Labels:
      None

      Description

      Therefore, not all rat errors are detected.

        Activity

        Hide
        Hadoop QA added a comment -

        Testing JIRA BOOKKEEPER-483

        WARNING: Running test-patch on a dirty local svn workspace

        Patch <a href="/jira/secure/attachment/12555013/0001-BOOKKEEPER-483check-all-modules-for-rat-errors.patch">/jira/secure/attachment/12555013/0001BOOKKEEPER-483-check-all-modules-for-rat-errors.patch</a> downloaded at Tue Nov 27 18:12:18 UTC 2012

        ----------------------------

        +1 PATCH_APPLIES
        +1 CLEAN
        +1 RAW_PATCH_ANALYSIS
        . +1 the patch does not introduce any @author tags
        . +1 the patch does not introduce any tabs
        . +1 the patch does not introduce any trailing spaces
        . +1 the patch does not introduce any line longer than 120
        . +1 the patch does adds/modifies 1 testcase(s)
        +1 RAT
        . +1 the patch does not seem to introduce new RAT warnings
        +1 JAVADOC
        . +1 the patch does not seem to introduce new Javadoc warnings
        . WARNING: the current HEAD has 8 Javadoc warning(s)
        +1 COMPILE
        . +1 HEAD compiles
        . +1 patch compiles
        . +1 the patch does not seem to introduce new javac warnings
        . WARNING: the current HEAD has 9 javac warning(s)
        +1 FINDBUGS
        . +1 the patch does not seem to introduce new Findbugs warnings
        . WARNING: the current HEAD has Findbugs warning(s), they should be addressed ASAP
        +1 TESTS
        . Tests run: 389
        +1 DISTRO
        . +1 distro tarball builds with the patch

        ----------------------------
        +1 Overall result, good!, no -1s

        . There is at least one warning, please check

        The full output of the test-patch run is available at

        . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/43/

        Show
        Hadoop QA added a comment - Testing JIRA BOOKKEEPER-483 WARNING: Running test-patch on a dirty local svn workspace Patch <a href="/jira/secure/attachment/12555013/0001- BOOKKEEPER-483 check-all-modules-for-rat-errors.patch">/jira/secure/attachment/12555013/0001 BOOKKEEPER-483 -check-all-modules-for-rat-errors.patch</a> downloaded at Tue Nov 27 18:12:18 UTC 2012 ---------------------------- +1 PATCH_APPLIES +1 CLEAN +1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 120 . +1 the patch does adds/modifies 1 testcase(s) +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings . WARNING : the current HEAD has 8 Javadoc warning(s) +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings . WARNING : the current HEAD has 9 javac warning(s) +1 FINDBUGS . +1 the patch does not seem to introduce new Findbugs warnings . WARNING: the current HEAD has Findbugs warning(s), they should be addressed ASAP +1 TESTS . Tests run: 389 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- +1 Overall result, good!, no -1s . There is at least one warning, please check The full output of the test-patch run is available at . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/43/
        Hide
        Flavio Junqueira added a comment - - edited

        The patch is quite simple and it looks good to me. I checked the output of QA and it looks right to me. I just need a quick clarification: should we see the execution of "copyRatFiles clean" in the console output of the build? I can see the ones of "copyRatFiles patch" but not the one for clean.

        Show
        Flavio Junqueira added a comment - - edited The patch is quite simple and it looks good to me. I checked the output of QA and it looks right to me. I just need a quick clarification: should we see the execution of "copyRatFiles clean" in the console output of the build? I can see the ones of "copyRatFiles patch" but not the one for clean.
        Hide
        Ivan Kelly added a comment -

        because when the RAT task runs for clean, this patch has not been applied, therefore it won't do the copy. Once this patch is in, you'll see it for both.

        Show
        Ivan Kelly added a comment - because when the RAT task runs for clean, this patch has not been applied, therefore it won't do the copy. Once this patch is in, you'll see it for both.
        Hide
        Flavio Junqueira added a comment -

        Perhaps I don't understand very well what's going on and my observation might be naive. I thought that with the pre-commit I was supposed to see it because it runs with the patch applied, but perhaps it still uses the unpatched scripts. Could you clarify just for my own understanding, please?

        Show
        Flavio Junqueira added a comment - Perhaps I don't understand very well what's going on and my observation might be naive. I thought that with the pre-commit I was supposed to see it because it runs with the patch applied, but perhaps it still uses the unpatched scripts. Could you clarify just for my own understanding, please?
        Hide
        Flavio Junqueira added a comment -

        Ivan Kelly Waiting for a clarification.

        Show
        Flavio Junqueira added a comment - Ivan Kelly Waiting for a clarification.
        Hide
        Ivan Kelly added a comment -

        The precommit runs the the rat check twice. Once with clean trunk, and once with patch applied, and compares the output.

        The precommit build for this patch is going to be comparing what trunk generates (no copyRatFiles) and what the patch generates (with copyRatFiles).

        Show
        Ivan Kelly added a comment - The precommit runs the the rat check twice. Once with clean trunk, and once with patch applied, and compares the output. The precommit build for this patch is going to be comparing what trunk generates (no copyRatFiles) and what the patch generates (with copyRatFiles).
        Hide
        Flavio Junqueira added a comment -

        +1, Thanks, Ivan. Committed revision 1421346.

        Show
        Flavio Junqueira added a comment - +1, Thanks, Ivan. Committed revision 1421346.
        Hide
        Hudson added a comment -

        Integrated in bookkeeper-trunk #860 (See https://builds.apache.org/job/bookkeeper-trunk/860/)
        BOOKKEEPER-483: precommit tests only check toplevel rat file, not the one for submodules. (ivank via fpj) (Revision 1421346)

        Result = FAILURE
        fpj :
        Files :

        • /zookeeper/bookkeeper/trunk/CHANGES.txt
        • /zookeeper/bookkeeper/trunk/bin/test-patch-08-rat
        Show
        Hudson added a comment - Integrated in bookkeeper-trunk #860 (See https://builds.apache.org/job/bookkeeper-trunk/860/ ) BOOKKEEPER-483 : precommit tests only check toplevel rat file, not the one for submodules. (ivank via fpj) (Revision 1421346) Result = FAILURE fpj : Files : /zookeeper/bookkeeper/trunk/CHANGES.txt /zookeeper/bookkeeper/trunk/bin/test-patch-08-rat

          People

          • Assignee:
            Ivan Kelly
            Reporter:
            Ivan Kelly
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development