Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3.0
    • Fix Version/s: 3.3.0
    • Component/s: tests
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Initial attempt to create ZooKeeper tests based on the example code on the website.
      Current plan is to test features used in examples using ZooKeeper calls directly. Another approach would be to make more usable abstractions such as those in src/recipes and test those.

      1. ZOOKEEPER-543.patch
        2 kB
        Steven Cheng
      2. ZOOKEEPER-543.patch
        5 kB
        Steven Cheng
      3. ZOOKEEPER-543.patch
        3 kB
        Steven Cheng
      4. ZOOKEEPER-543.patch
        5 kB
        Steven Cheng

        Activity

        Hide
        Mahadev konar added a comment -

        I just committed this. thanks steven!

        Show
        Mahadev konar added a comment - I just committed this. thanks steven!
        Hide
        Mahadev konar added a comment -

        +1 a nice to have test case!

        Show
        Mahadev konar added a comment - +1 a nice to have test case!
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12438085/ZOOKEEPER-543.patch
        against trunk revision 919706.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/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/12438085/ZOOKEEPER-543.patch against trunk revision 919706. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/79/console This message is automatically generated.
        Hide
        Steven Cheng added a comment -

        Extra import was left in there for some reason.

        Show
        Steven Cheng added a comment - Extra import was left in there for some reason.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12431894/ZOOKEEPER-543.patch
        against trunk revision 919640.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        -1 core tests. The patch failed core unit tests.

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

        Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/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/12431894/ZOOKEEPER-543.patch against trunk revision 919640. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/133/console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12431894/ZOOKEEPER-543.patch
        against trunk revision 912052.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

        -1 core tests. The patch failed core unit tests.

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

        Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/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/12431894/ZOOKEEPER-543.patch against trunk revision 912052. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 8 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/118/console This message is automatically generated.
        Hide
        Patrick Hunt added a comment -

        Steven, only reason I can think of is that the mocks are not setup to handle sequential?

        Show
        Patrick Hunt added a comment - Steven, only reason I can think of is that the mocks are not setup to handle sequential?
        Hide
        Steven Cheng added a comment -

        C and Java version of testSequentialNodeData.

        I found that almost all the tests in TestClient.cc need THREADED and this test also would not work without THREADED even though there's no reason it shouldn't.

        Is there a reason for this?

        Show
        Steven Cheng added a comment - C and Java version of testSequentialNodeData. I found that almost all the tests in TestClient.cc need THREADED and this test also would not work without THREADED even though there's no reason it shouldn't. Is there a reason for this?
        Hide
        Steven Cheng added a comment -

        Can do, shouldn't take too long I think.

        Show
        Steven Cheng added a comment - Can do, shouldn't take too long I think.
        Hide
        Patrick Hunt added a comment -

        Steven that's great. Do you want to take a shot at adding similar verification to the c tests as well? Or
        should we just consider this one as is? (meaning java only)

        Show
        Patrick Hunt added a comment - Steven that's great. Do you want to take a shot at adding similar verification to the c tests as well? Or should we just consider this one as is? (meaning java only)
        Hide
        Steven Cheng added a comment -

        I took a look at ClientTest and testMultipleWatcherObjs seems to cover most of what the lock test does. There is a fair bit of overlap with testSequentialNodeNames and the queue test, but the queue test tests the data.

        I moved the queue test into ClientTest and changed it to make it more similar to testSequentialNodeNames in this patch.

        Show
        Steven Cheng added a comment - I took a look at ClientTest and testMultipleWatcherObjs seems to cover most of what the lock test does. There is a fair bit of overlap with testSequentialNodeNames and the queue test, but the queue test tests the data. I moved the queue test into ClientTest and changed it to make it more similar to testSequentialNodeNames in this patch.
        Hide
        Patrick Hunt added a comment -

        I don't mind adding this, but aren't we testing this already in our tests? If not, wouldn't it be better to add these to existing/new test classes
        that are more geared to the specific functionality being tested, rather than to some example which may change over time? Or is the
        idea to point ppl to this "exampletest" class and therefore use it as a teaching aid, rather than just a test? Also, putting into say clienttest
        class would enable testing under both standalone & quorum (for example). I guess my point is, why specifically "exampletest", rather than
        just a test? Perhaps I'm just confused by the naming, again, I think the idea of adding more tests to cover things we don't already test
        is a good idea so don't want to dis-wade you in general.

        Show
        Patrick Hunt added a comment - I don't mind adding this, but aren't we testing this already in our tests? If not, wouldn't it be better to add these to existing/new test classes that are more geared to the specific functionality being tested, rather than to some example which may change over time? Or is the idea to point ppl to this "exampletest" class and therefore use it as a teaching aid, rather than just a test? Also, putting into say clienttest class would enable testing under both standalone & quorum (for example). I guess my point is, why specifically "exampletest", rather than just a test? Perhaps I'm just confused by the naming, again, I think the idea of adding more tests to cover things we don't already test is a good idea so don't want to dis-wade you in general.
        Hide
        Steven Cheng added a comment -

        Hi Mahadev, my original idea was to have small tests that tested the basic features needed to implement the examples without bringing in a full API.

        testQueue tests that creating nodes works properly, in a similar way that a queue would use them.

        testLock tests that create and delete work properly in a way that a lock would use them.

        At this point I think that the queue and lock recipes do a good job of covering these cases, so these tests are probably not needed.

        Show
        Steven Cheng added a comment - Hi Mahadev, my original idea was to have small tests that tested the basic features needed to implement the examples without bringing in a full API. testQueue tests that creating nodes works properly, in a similar way that a queue would use them. testLock tests that create and delete work properly in a way that a lock would use them. At this point I think that the queue and lock recipes do a good job of covering these cases, so these tests are probably not needed.
        Hide
        Mahadev konar added a comment -

        waiting for steven to update the jira.

        Show
        Mahadev konar added a comment - waiting for steven to update the jira.
        Hide
        Mahadev konar added a comment -

        steven,
        i am a little confused here. Is this the code that is testing something? Looks like you have a test for producer consumer classes? Dont we need to create an api (maybe copy from the examples in the docs) first and then test them? This could very well go into src/java/examples but am not sure what your code is trying to test. Can you be a little more specific?

        Show
        Mahadev konar added a comment - steven, i am a little confused here. Is this the code that is testing something? Looks like you have a test for producer consumer classes? Dont we need to create an api (maybe copy from the examples in the docs) first and then test them? This could very well go into src/java/examples but am not sure what your code is trying to test. Can you be a little more specific?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12421475/ZOOKEEPER-543.patch
        against trunk revision 899383.

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

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        -1 release audit. The applied patch generated 208 release audit warnings (more than the trunk's current 207 warnings).

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/testReport/
        Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/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/12421475/ZOOKEEPER-543.patch against trunk revision 899383. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. -1 release audit. The applied patch generated 208 release audit warnings (more than the trunk's current 207 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/104/console This message is automatically generated.
        Hide
        Steven Cheng added a comment -

        There's a bug in the node name processing, it tries to remove the full path from the string before parsing the id. The test still passes because of the 0 padding on the names. I'll fix this.

        Show
        Steven Cheng added a comment - There's a bug in the node name processing, it tries to remove the full path from the string before parsing the id. The test still passes because of the 0 padding on the names. I'll fix this.

          People

          • Assignee:
            Steven Cheng
            Reporter:
            Steven Cheng
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development