Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3606

libhdfs: create self-contained unit test

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: libhdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We should have a self-contained unit test for libhdfs and also for FUSE.
      We do have hdfs_test, but it is not self-contained (it requires a cluster to already be running before it can be used.)

      1. HDFS-3606.004.patch
        23 kB
        Colin Patrick McCabe
      2. HDFS-3606.003.patch
        24 kB
        Colin Patrick McCabe
      3. HDFS-3606.001.patch
        17 kB
        Colin Patrick McCabe

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1136 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1136/)
          HDFS-3606. libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1136 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1136/ ) HDFS-3606 . libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1103 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1103/)
          HDFS-3606. libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1103 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1103/ ) HDFS-3606 . libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2487 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2487/)
          HDFS-3606. libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2487 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2487/ ) HDFS-3606 . libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2468 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2468/)
          HDFS-3606. libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2468 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2468/ ) HDFS-3606 . libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2533 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2533/)
          HDFS-3606. libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2533 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2533/ ) HDFS-3606 . libhdfs: create self-contained unit test. Contributed by Colin Patrick McCabe (Revision 1361440) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361440 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/pom.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/CMakeLists.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/expect.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/native_mini_dfs.h /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_libhdfs_threaded.c /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/native/test_native_mini_dfs.c
          Hide
          Eli Collins added a comment -

          I've committed this and merged to branch-2. Thanks Colin.

          Show
          Eli Collins added a comment - I've committed this and merged to branch-2. Thanks Colin.
          Hide
          Eli Collins added a comment -

          Got it, please file a follow-up jira for that. +1 on the latest patch. Test failures are unrelated.

          Show
          Eli Collins added a comment - Got it, please file a follow-up jira for that. +1 on the latest patch. Test failures are unrelated.
          Hide
          Colin Patrick McCabe added a comment -

          I created HDFS-3657 to look into testing non-recursive delete in this unit test

          Show
          Colin Patrick McCabe added a comment - I created HDFS-3657 to look into testing non-recursive delete in this unit test
          Hide
          Colin Patrick McCabe added a comment -

          The following should be updated now that HDFS-3633 is in right? // TODO: Non-recursive delete should fail?

          Unfortunately, that still doesn't work when I un-comment it. I'm not sure exactly why hdfsDelete is behaving this way. It definitely seems like it should give ENOTDIR or something, but instead it returns 0.

          I guess we can check up on this issue later (not sure if it merits a JIRA or not). For now, this test will make things a lot better...

          Show
          Colin Patrick McCabe added a comment - The following should be updated now that HDFS-3633 is in right? // TODO: Non-recursive delete should fail? Unfortunately, that still doesn't work when I un-comment it. I'm not sure exactly why hdfsDelete is behaving this way. It definitely seems like it should give ENOTDIR or something, but instead it returns 0. I guess we can check up on this issue later (not sure if it merits a JIRA or not). For now, this test will make things a lot better...
          Hide
          Eli Collins added a comment -

          The following should be updated now that HDFS-3633 is in right?

              // TODO: Non-recursive delete should fail?
              //EXPECT_NONZERO(hdfsDelete(fs, prefix, 0));
          

          Otherwise looks great. Agree with all of Andy's (excellent) feedback.

          Show
          Eli Collins added a comment - The following should be updated now that HDFS-3633 is in right? // TODO: Non-recursive delete should fail? //EXPECT_NONZERO(hdfsDelete(fs, prefix, 0)); Otherwise looks great. Agree with all of Andy's (excellent) feedback.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 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 did not generate any 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.TestBackupNode
          org.apache.hadoop.hdfs.server.common.TestJspHelper

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2805//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2805//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/12536173/HDFS-3606.004.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 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 did not generate any 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.TestBackupNode org.apache.hadoop.hdfs.server.common.TestJspHelper +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2805//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2805//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -

          You're absolutely right about the references-- after the MiniDFSCluster#Builder methods that return a reference to the Builder, we will have to free that reference.

          I made the other changes you suggested.

          expect.h is just a few macros to make things shorter in the libhdfs unit test. It's not really a framework per se. It's very specific to libhdfs because it does things like print out errno (we have that error convention in libhdfs, etc.)

          I changed the native_mini_dfs methods to return negative error codes. It just makes more sense since nmdGetNameNodePort returns a positive port number on success. In general I try to return negative codes unless someone has already established the opposite convention, and I don't want to change it too much.

          Show
          Colin Patrick McCabe added a comment - You're absolutely right about the references-- after the MiniDFSCluster#Builder methods that return a reference to the Builder, we will have to free that reference. I made the other changes you suggested. expect.h is just a few macros to make things shorter in the libhdfs unit test. It's not really a framework per se. It's very specific to libhdfs because it does things like print out errno (we have that error convention in libhdfs, etc.) I changed the native_mini_dfs methods to return negative error codes. It just makes more sense since nmdGetNameNodePort returns a positive port number on success. In general I try to return negative codes unless someone has already established the opposite convention, and I don't want to change it too much.
          Hide
          Andy Isaacson added a comment -

          Structure comment – I wasn't expecting to find a whole bespoke testing framework in expect.h when I opened this patch. That deserves a mention in the description and/or upload comments for the diff.

          (The framework is fine, I was just not expecting to see it here based on the descriptions.)

          Minor comment on the framework:

          +#define EXPECT_LT(x, y) \
          +    do { \
          +        if ((x) >= (y)) { \
          +            fprintf(stderr, "TEST_ERROR: failed on line %d: %s\n",\
          +                __LINE__, #x); \
          

          we might as well print #y too, even though this is pretty reasonable as it stands.
          Similarly for all the other EXPECT_foo(x,y) macros.

          +    cobj = constructNewObjectOfClass(env, NULL, HADOOP_CONF, "()V");
          ...
          +    bld = constructNewObjectOfClass(env, NULL, MINIDFS_CLUSTER_BUILDER,
          +                    "(L"HADOOP_CONF";)V", cobj);
          +    if (!bld) {
          +        fprintf(stderr, "nmdCreate: unable to construct "
          +                "NativeMiniDfsCluster#Builder\n");
          +        goto error_free_cl;
          

          I could be missing something, but ... don't we leak cobj here? I think this error exit should be goto error_dlr_cobj.

          +    if (invokeMethod(env, &val, NULL, INSTANCE, bld,
          +            MINIDFS_CLUSTER_BUILDER, "format",
          ...
          
          +    if (invokeMethod(env, &val, NULL, INSTANCE, bld,
          +            MINIDFS_CLUSTER_BUILDER, "build",
          ...
          +    (*env)->DeleteLocalRef(env, val.l);
          

          In the success code path, do we need to DeleteLocalRef the object returned from the first call, to invokeMethod(..., "format", ...)?

          The various nmdFoo functions return a mix of EIO and -EIO. I feel like EIO makes a little more sense since the only answers are either "success" or "failure". Except for nmdGetNameNodePort which returns a small positive integer on success ... so maybe standardizing on -EIO makes more sense. Your call.

          +    eret = strlen(prefix);
          +    ret = hdfsWrite(fs, file, prefix, eret);
          

          Could we name this expected or buflen rather than eret? Sorry for the nitpick but it really threw me off reading the code.

          Show
          Andy Isaacson added a comment - Structure comment – I wasn't expecting to find a whole bespoke testing framework in expect.h when I opened this patch. That deserves a mention in the description and/or upload comments for the diff. (The framework is fine, I was just not expecting to see it here based on the descriptions.) Minor comment on the framework: +#define EXPECT_LT(x, y) \ + do { \ + if ((x) >= (y)) { \ + fprintf(stderr, "TEST_ERROR: failed on line %d: %s\n" ,\ + __LINE__, #x); \ we might as well print #y too, even though this is pretty reasonable as it stands. Similarly for all the other EXPECT_foo(x,y) macros. + cobj = constructNewObjectOfClass(env, NULL, HADOOP_CONF, "()V" ); ... + bld = constructNewObjectOfClass(env, NULL, MINIDFS_CLUSTER_BUILDER, + "(L" HADOOP_CONF ";)V" , cobj); + if (!bld) { + fprintf(stderr, "nmdCreate: unable to construct " + "NativeMiniDfsCluster#Builder\n" ); + goto error_free_cl; I could be missing something, but ... don't we leak cobj here? I think this error exit should be goto error_dlr_cobj . + if (invokeMethod(env, &val, NULL, INSTANCE, bld, + MINIDFS_CLUSTER_BUILDER, "format" , ... + if (invokeMethod(env, &val, NULL, INSTANCE, bld, + MINIDFS_CLUSTER_BUILDER, "build" , ... + (*env)->DeleteLocalRef(env, val.l); In the success code path, do we need to DeleteLocalRef the object returned from the first call, to invokeMethod(..., "format", ...) ? The various nmdFoo functions return a mix of EIO and -EIO . I feel like EIO makes a little more sense since the only answers are either "success" or "failure". Except for nmdGetNameNodePort which returns a small positive integer on success ... so maybe standardizing on -EIO makes more sense. Your call. + eret = strlen(prefix); + ret = hdfsWrite(fs, file, prefix, eret); Could we name this expected or buflen rather than eret ? Sorry for the nitpick but it really threw me off reading the code.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 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 did not generate any warning messages.

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

          -1 findbugs. The patch appears to introduce 2 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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//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/12535958/HDFS-3606.003.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 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 did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 2 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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2786//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -
          • remove the 'extern' directives from the header file (style suggested by eli)
          Show
          Colin Patrick McCabe added a comment - remove the 'extern' directives from the header file (style suggested by eli)
          Hide
          Colin Patrick McCabe added a comment -
          • test multiple threads, opening, writing, reading, and closing files
          Show
          Colin Patrick McCabe added a comment - test multiple threads, opening, writing, reading, and closing files
          Hide
          Eli Collins added a comment -

          Looks good, minor comments

          • This function doesn't actually write/read a file. Also I'd pull the code out to something like testWriteFile
            /**
             * Test that we can write a file with libhdfs and then read it back
             */
            int main(void) {
            
          • Style nit: remove extern from the function prototypes in nativeMiniDfs.h
          Show
          Eli Collins added a comment - Looks good, minor comments This function doesn't actually write/read a file. Also I'd pull the code out to something like testWriteFile /** * Test that we can write a file with libhdfs and then read it back */ int main(void) { Style nit: remove extern from the function prototypes in nativeMiniDfs.h
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 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 did not generate any warning messages.

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

          -1 findbugs. The patch appears to introduce 2 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.TestHDFSTrash
          org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//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/12535479/HDFS-3606.001.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 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 did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 2 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestHDFSTrash org.apache.hadoop.hdfs.server.blockmanagement.TestBlocksWithNotEnoughRacks +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2758//console This message is automatically generated.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development