Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: 3.3.0
    • Component/s: tests
    • Labels:
      None

      Description

      In this stage main set (src/java/test) of ZK tests will be converted to TestNG

      1. ZOOKEEPER-397.patch
        98 kB
        Konstantin Boudnik
      2. ZOOKEEPER-397.patch
        98 kB
        Konstantin Boudnik
      3. ZOOKEEPER-397.patch
        101 kB
        Patrick Hunt
      4. ZOOKEEPER-397.patch
        136 kB
        Patrick Hunt
      5. ZOOKEEPER-397.patch
        125 kB
        Konstantin Boudnik
      6. ZOOKEEPER-397.patch
        133 kB
        Konstantin Boudnik
      7. ZOOKEEPER-397.patch
        139 kB
        Patrick Hunt
      8. ZOOKEEPER-397.patch
        154 kB
        Konstantin Boudnik
      9. testng-5.9-jdk15.jar
        833 kB
        Konstantin Boudnik

        Activity

        Hide
        Konstantin Boudnik added a comment -

        Initial version of the patch for mainline tests conversion

        Show
        Konstantin Boudnik added a comment - Initial version of the patch for mainline tests conversion
        Hide
        Konstantin Boudnik added a comment -

        The following issues are taken care of by this patch:

        • all test cases of mainline suite are converted into native TestNG format
        • new target is added into build.xml to execute mainline tests under TestNG control rather than JUnit (it is now the default behavior for test-core-java)
        • a couple new classes are added to serve TestNG reporting purposes org.apache.zookeeper.MyInvocationReporter and MyOutputInterceptor. The first is intended to unify the logging of test execution invocation (instead of having a separate LOG.info for every test case, this class is added once); the second one prints out the information about executed test cases and the the result of the execution. It comes in this form:
          [testng] org.apache.zookeeper.test.WatcherFuncTest.testGetDataSync: Passed (in 12ms)
          Generally, I'd suggest to move the classes somewhere at the level above zookeeper for they will be serving more than one project in the future.
        • A number of test classes are fixed to work properly in the TestNG paradigm where test classes aren't inhered from some common TestCase thus their instances aren't normally getting thrown after each test method execution.

        Unsolved issues:
        1) org.apache.zookeeper.test.WatcherTest:testWatchAutoResetWithPending
        fails with the following diagnostics
        Did not connect
        org.apache.zookeeper.test.ClientBase$CountdownWatcher.waitForConnected(ClientBase.java:95)
        at org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending(WatcherTest.java:174)

        when is executed along with all other tests. An attempt to increase some timeouts was made, but it didn't help a lot.

        2) All standard output from the testcases along with the output of ConsoleAppender is printed to console now. I see two way of fixing it:

        • changing log4j configuration for the tests
        • creating, if possible, a workaround at the TestNG level to intercept stdout if needed
          The latter solution might be relative difficult to achieve.

        I would appreciate to get a review for this patch.

        Show
        Konstantin Boudnik added a comment - The following issues are taken care of by this patch: all test cases of mainline suite are converted into native TestNG format new target is added into build.xml to execute mainline tests under TestNG control rather than JUnit (it is now the default behavior for test-core-java) a couple new classes are added to serve TestNG reporting purposes org.apache.zookeeper.MyInvocationReporter and MyOutputInterceptor. The first is intended to unify the logging of test execution invocation (instead of having a separate LOG.info for every test case, this class is added once); the second one prints out the information about executed test cases and the the result of the execution. It comes in this form: [testng] org.apache.zookeeper.test.WatcherFuncTest.testGetDataSync: Passed (in 12ms) Generally, I'd suggest to move the classes somewhere at the level above zookeeper for they will be serving more than one project in the future. A number of test classes are fixed to work properly in the TestNG paradigm where test classes aren't inhered from some common TestCase thus their instances aren't normally getting thrown after each test method execution. Unsolved issues: 1) org.apache.zookeeper.test.WatcherTest:testWatchAutoResetWithPending fails with the following diagnostics Did not connect org.apache.zookeeper.test.ClientBase$CountdownWatcher.waitForConnected(ClientBase.java:95) at org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending(WatcherTest.java:174) when is executed along with all other tests. An attempt to increase some timeouts was made, but it didn't help a lot. 2) All standard output from the testcases along with the output of ConsoleAppender is printed to console now. I see two way of fixing it: changing log4j configuration for the tests creating, if possible, a workaround at the TestNG level to intercept stdout if needed The latter solution might be relative difficult to achieve. I would appreciate to get a review for this patch.
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 144 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-vesta.apache.org/71/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/71/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/71/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/12407926/ZOOKEEPER-397.patch against trunk revision 774081. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 144 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-vesta.apache.org/71/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/71/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/71/console This message is automatically generated.
        Hide
        Konstantin Boudnik added a comment -

        Missed TestNG jar file

        Show
        Konstantin Boudnik added a comment - Missed TestNG jar file
        Hide
        Konstantin Boudnik added a comment -

        Always falling test is excluded for now

        Show
        Konstantin Boudnik added a comment - Always falling test is excluded for now
        Hide
        Patrick Hunt added a comment -

        I made a change to the patch to append log4j messages from the code under test to a file in the output directory.

        This substantially reduces the output to stdout, but there are still a few issues that need to be resolved, cancelling
        for now:

        1) I'm consistently seeing 6 failures, all of the FLE/LE/HierQu tests, but acl tests are failing as well

        2) all tests running under a single VM may be a problem for us. Our server is really designed to shutdown
        via exiting the vm, not necessarily cleaning up every detail. Having this expectation in the tests may be
        unrealistic, at least from the perspective of easily converting to testng.

        The updated patch also included a small set of changes to output interceptor.

        I'm thinking that it might be interesting to look at having ant fork each test class, running testng as the forked
        process. This target can run all the classes, and subsequently check if any failed and return failure if any one
        test fails. (this is essentially what junit task does in ant with forkmode=pertest). I'm surprised testng does not
        support this. (ant "apply" task can do the looping over test classes btw)

        Show
        Patrick Hunt added a comment - I made a change to the patch to append log4j messages from the code under test to a file in the output directory. This substantially reduces the output to stdout, but there are still a few issues that need to be resolved, cancelling for now: 1) I'm consistently seeing 6 failures, all of the FLE/LE/HierQu tests, but acl tests are failing as well 2) all tests running under a single VM may be a problem for us. Our server is really designed to shutdown via exiting the vm, not necessarily cleaning up every detail. Having this expectation in the tests may be unrealistic, at least from the perspective of easily converting to testng. The updated patch also included a small set of changes to output interceptor. I'm thinking that it might be interesting to look at having ant fork each test class, running testng as the forked process. This target can run all the classes, and subsequently check if any failed and return failure if any one test fails. (this is essentially what junit task does in ant with forkmode=pertest). I'm surprised testng does not support this. (ant "apply" task can do the looping over test classes btw)
        Hide
        Patrick Hunt added a comment -

        This updated patch fixes a few test but is still not quite there. Main thing is that I now
        assign a different port each time a test server (setup usually) is created. This should
        help to eliminate the "address in use" issues.

        Show
        Patrick Hunt added a comment - This updated patch fixes a few test but is still not quite there. Main thing is that I now assign a different port each time a test server (setup usually) is created. This should help to eliminate the "address in use" issues.
        Hide
        Konstantin Boudnik added a comment -

        Reporters are updated to have nicely formatted output.
        A couple of missed @Before and @After methods were returned back to the code

        Show
        Konstantin Boudnik added a comment - Reporters are updated to have nicely formatted output. A couple of missed @Before and @After methods were returned back to the code
        Hide
        Konstantin Boudnik added a comment -

        A couple of files were missed from the last version

        Show
        Konstantin Boudnik added a comment - A couple of files were missed from the last version
        Hide
        Patrick Hunt added a comment -

        This updated patch gets all the tests running on my machine except for FLETest. I'm going to ask
        Flavio to take a look.

        Otw all the main tests are running under testng though.

        Show
        Patrick Hunt added a comment - This updated patch gets all the tests running on my machine except for FLETest. I'm going to ask Flavio to take a look. Otw all the main tests are running under testng though.
        Hide
        Flavio Junqueira added a comment -

        Tests pass for me:

          [testng] PASSED org.apache.zookeeper.test.FLENewEpochTest.testLENewEpoch Passed ( 660 ms)
           [testng] PASSED org.apache.zookeeper.test.FLETest.testLE Passed ( 1438 ms)
           [testng] PASSED org.apache.zookeeper.test.HierarchicalQuorumTest.testHierarchicalQuorum Passed ( 599 ms)
        
        Show
        Flavio Junqueira added a comment - Tests pass for me: [testng] PASSED org.apache.zookeeper.test.FLENewEpochTest.testLENewEpoch Passed ( 660 ms) [testng] PASSED org.apache.zookeeper.test.FLETest.testLE Passed ( 1438 ms) [testng] PASSED org.apache.zookeeper.test.HierarchicalQuorumTest.testHierarchicalQuorum Passed ( 599 ms)
        Hide
        Konstantin Boudnik added a comment - - edited

        I can confirm that most of the tests are passing on Mac OS, even the FLETest.
        However, when I've un-commented the one we've decided to exclude earlier (org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending) it has failed the same way as it was failing before.

        Other than that I don't see any issues!

        Show
        Konstantin Boudnik added a comment - - edited I can confirm that most of the tests are passing on Mac OS, even the FLETest. However, when I've un-commented the one we've decided to exclude earlier (org.apache.zookeeper.test.WatcherTest.testWatchAutoResetWithPending) it has failed the same way as it was failing before. Other than that I don't see any issues!
        Hide
        Patrick Hunt added a comment -

        not a blocker for 3.2, moving to 3.3

        Show
        Patrick Hunt added a comment - not a blocker for 3.2, moving to 3.3
        Hide
        Konstantin Boudnik added a comment -

        Fixing the status to "Patch Available"

        Show
        Konstantin Boudnik added a comment - Fixing the status to "Patch Available"
        Hide
        Hadoop QA added a comment -

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

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/119/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/12408109/ZOOKEEPER-397.patch against trunk revision 785741. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 145 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/119/console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        patch doesn't apply

        Show
        Benjamin Reed added a comment - patch doesn't apply
        Hide
        Konstantin Boudnik added a comment -

        This version of the patch solves all the conflicts caused by the long period of inactivity in this particular JIRA.

        Not all of the tests are passing under TestNG now, because some new tests were introduced, which set some environment and fail to clean it properly afterward.

        However, per our conversation with Patrick I do submit this version of the patch for it has features having some value for the project.

        Show
        Konstantin Boudnik added a comment - This version of the patch solves all the conflicts caused by the long period of inactivity in this particular JIRA. Not all of the tests are passing under TestNG now, because some new tests were introduced, which set some environment and fail to clean it properly afterward. However, per our conversation with Patrick I do submit this version of the patch for it has features having some value for the project.

          People

          • Assignee:
            Konstantin Boudnik
            Reporter:
            Konstantin Boudnik
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development