ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-579

zkpython needs more test coverage for ACL code paths

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.1
    • Fix Version/s: 3.3.0
    • Component/s: contrib-bindings
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      zkpython's tests don't do a good enough job of exercising the ACL code paths. A few new tests that confirm that setACL and friends are working correctly are needed.

      1. zookeeper-579.patch
        6 kB
        Henry Robinson
      2. zookeeper-579.patch
        7 kB
        Mahadev konar

        Activity

        Patrick Hunt made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #715 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/715/)
        . zkpython needs more test coverage for ACL code paths (henry via mahadev)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #715 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/715/ ) . zkpython needs more test coverage for ACL code paths (henry via mahadev)
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Mahadev konar added a comment -

        I just committed this. thanks henry!

        Show
        Mahadev konar added a comment - I just committed this. thanks henry!
        Mahadev konar made changes -
        Attachment zookeeper-579.patch [ 12437966 ]
        Hide
        Mahadev konar added a comment -

        forgot to attach a new file.

        Show
        Mahadev konar added a comment - forgot to attach a new file.
        Mahadev konar made changes -
        Attachment zookeeper-579.patch [ 12437965 ]
        Mahadev konar made changes -
        Attachment zookeeper-579.patch [ 12437965 ]
        Hide
        Mahadev konar added a comment -

        i tried the patch. It works against 2.6 fine. So, I changed the README to reflect that we have just tested against 2.6 and greater (excluding 3.*).

        Earlier it said that it works against >= 2.3. Other than that the patch looks good. I will go ahead and commit it.

        Show
        Mahadev konar added a comment - i tried the patch. It works against 2.6 fine. So, I changed the README to reflect that we have just tested against 2.6 and greater (excluding 3.*). Earlier it said that it works against >= 2.3. Other than that the patch looks good. I will go ahead and commit it.
        Hide
        Patrick Hunt added a comment -

        Mahadev can you take a look at this, are you still experiencing issues in test(s)?

        Show
        Patrick Hunt added a comment - Mahadev can you take a look at this, are you still experiencing issues in test(s)?
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12434938/zookeeper-579.patch
        against trunk revision 911716.

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

        +1 tests included. The patch appears to include 2 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/66/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/66/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/66/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/12434938/zookeeper-579.patch against trunk revision 911716. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 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/66/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/66/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/66/console This message is automatically generated.
        Hide
        Henry Robinson added a comment -

        It is a problem that the build is considered successful even though tests fail... I should make a new JIRA to fix that.

        What version of Python are you using? Both of those tests pass fine for me on Python 2.6, but it's possible that this is a version-related error.

        Show
        Henry Robinson added a comment - It is a problem that the build is considered successful even though tests fail... I should make a new JIRA to fix that. What version of Python are you using? Both of those tests pass fine for me on Python 2.6, but it's possible that this is a version-related error.
        Mahadev konar made changes -
        Fix Version/s 3.3.0 [ 12313976 ]
        Hide
        Mahadev konar added a comment -

        marking it for 3.3

        Show
        Mahadev konar added a comment - marking it for 3.3
        Hide
        Mahadev konar added a comment -

        I get this while running ant test in src/contrib/zkpython:

             [exec] ======================================================================
             [exec] ERROR: test_multiple_watchers (__main__.CallbackTest)
             [exec] ----------------------------------------------------------------------
             [exec] Traceback (most recent call last):
             [exec]   File "src/test/callback_test.py", line 130, in test_multiple_watchers
             [exec]     self.assertTrue(self.watcher1 and self.watcher2, "One or more watchers failed to fire")
             [exec] AttributeError: 'CallbackTest' object has no attribute 'assertTrue'
             [exec] 
             [exec] ----------------------------------------------------------------------
             [exec] Ran 4 tests in 0.167s
             [exec] 
             [exec] FAILED (errors=1)
             [exec] Running src/test/clientid_test.py
             [exec] .
             [exec] ----------------------------------------------------------------------
             [exec] Ran 1 test in 0.011s
             [exec] 
             [exec] OK
             [exec] Running src/test/connection_test.py
             [exec]   File "src/test/connection_test.py", line 118
             [exec]     self.assertEqual(True, all( zookeeper.close(h) == zookeeper.OK for h in handles ))
             [exec]                                                                      ^
             [exec] SyntaxError: invalid syntax
             [exec] Running src/test/create_test.py
             [exec] ....
             [exec] ----------------------------------------------------------------------
             [exec] Ran 4 tests in 0.072s
             [exec] 
             [exec] OK
             [exec] Running src/test/delete_test.py
             [exec] ..
             [exec] ----------------------------------------------------------------------
             [exec] Ran 2 tests in 0.046s
             [exec] 
             [exec] OK
             [exec] Running src/test/exists_test.py
             [exec] ...
             [exec] ----------------------------------------------------------------------
             [exec] Ran 3 tests in 0.062s
             [exec] 
             [exec] OK
             [exec] Running src/test/get_set_test.py
             [exec] ......
             [exec] ----------------------------------------------------------------------
             [exec] Ran 6 tests in 4.812s
             [exec] 
             [exec] OK
        
        test-stop:
        
        test:
        
        BUILD SUCCESSFUL
        Total time: 13 seconds
        

        I see some errors in the output which look a little suspicious, though ant declares a successful build. Am I missing something?

        Show
        Mahadev konar added a comment - I get this while running ant test in src/contrib/zkpython: [exec] ====================================================================== [exec] ERROR: test_multiple_watchers (__main__.CallbackTest) [exec] ---------------------------------------------------------------------- [exec] Traceback (most recent call last): [exec] File "src/test/callback_test.py", line 130, in test_multiple_watchers [exec] self.assertTrue(self.watcher1 and self.watcher2, "One or more watchers failed to fire") [exec] AttributeError: 'CallbackTest' object has no attribute 'assertTrue' [exec] [exec] ---------------------------------------------------------------------- [exec] Ran 4 tests in 0.167s [exec] [exec] FAILED (errors=1) [exec] Running src/test/clientid_test.py [exec] . [exec] ---------------------------------------------------------------------- [exec] Ran 1 test in 0.011s [exec] [exec] OK [exec] Running src/test/connection_test.py [exec] File "src/test/connection_test.py", line 118 [exec] self.assertEqual(True, all( zookeeper.close(h) == zookeeper.OK for h in handles )) [exec] ^ [exec] SyntaxError: invalid syntax [exec] Running src/test/create_test.py [exec] .... [exec] ---------------------------------------------------------------------- [exec] Ran 4 tests in 0.072s [exec] [exec] OK [exec] Running src/test/delete_test.py [exec] .. [exec] ---------------------------------------------------------------------- [exec] Ran 2 tests in 0.046s [exec] [exec] OK [exec] Running src/test/exists_test.py [exec] ... [exec] ---------------------------------------------------------------------- [exec] Ran 3 tests in 0.062s [exec] [exec] OK [exec] Running src/test/get_set_test.py [exec] ...... [exec] ---------------------------------------------------------------------- [exec] Ran 6 tests in 4.812s [exec] [exec] OK test-stop: test: BUILD SUCCESSFUL Total time: 13 seconds I see some errors in the output which look a little suspicious, though ant declares a successful build. Am I missing something?
        Hide
        Mahadev konar added a comment -

        +1 the patch looks good. ll run zkpython tests and post the results here.

        henry, you might want to start adding comments to the zookeeper.c file so that its easier for other developers to work on it in case someone wants to pick it up.

        Show
        Mahadev konar added a comment - +1 the patch looks good. ll run zkpython tests and post the results here. henry, you might want to start adding comments to the zookeeper.c file so that its easier for other developers to work on it in case someone wants to pick it up.
        Henry Robinson made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Henry Robinson made changes -
        Field Original Value New Value
        Attachment zookeeper-579.patch [ 12434938 ]
        Hide
        Henry Robinson added a comment -

        Patch adds test for aget_acl, aset_acl, set_acl and get_acl.

        Also adds tests in the C code for ACL objects that are roughly correctly formed, throwing InvalidACL where they are not found. This is to prevent an unpleasant bug where passing another object, say a callable, could silently succeed until a strange error condition would present.

        Show
        Henry Robinson added a comment - Patch adds test for aget_acl, aset_acl, set_acl and get_acl. Also adds tests in the C code for ACL objects that are roughly correctly formed, throwing InvalidACL where they are not found. This is to prevent an unpleasant bug where passing another object, say a callable, could silently succeed until a strange error condition would present.
        Henry Robinson created issue -

          People

          • Assignee:
            Henry Robinson
            Reporter:
            Henry Robinson
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development