Derby
  1. Derby
  2. DERBY-3838

Convert derbynet/DerbyNetAutoStart to JUnit

    Details

    1. DERBY-3838.diff
      0.7 kB
      Myrna van Lunteren
    2. DERBY-3838.diff
      13 kB
      Myrna van Lunteren
    3. DERBY-3838_2.diff
      14 kB
      Myrna van Lunteren
    4. DERBY-3838_3.stat
      1 kB
      Myrna van Lunteren
    5. DERBY-3838_3.diff
      47 kB
      Myrna van Lunteren

      Issue Links

        Activity

        Hide
        Yun Lee added a comment -

        Hi, Erlend! Will you continue on with the issue? If you are not free with it, I would like to be assigned. Thanks for reply!

        Show
        Yun Lee added a comment - Hi, Erlend! Will you continue on with the issue? If you are not free with it, I would like to be assigned. Thanks for reply!
        Hide
        Kathey Marsden added a comment -

        Yun, I think it is fine to grab this issue. We have not see Erlend for quite a while.

        Show
        Kathey Marsden added a comment - Yun, I think it is fine to grab this issue. We have not see Erlend for quite a while.
        Hide
        Yun Lee added a comment -

        I have to focus on my other issues, so unassigned myself.

        Show
        Yun Lee added a comment - I have to focus on my other issues, so unassigned myself.
        Hide
        Kathey Marsden added a comment -

        Tiago are you still working on this issue? If not, if you have done any work on it, please post a patch.

        Show
        Kathey Marsden added a comment - Tiago are you still working on this issue? If not, if you have done any work on it, please post a patch.
        Hide
        Myrna van Lunteren added a comment -

        Attaching a patch which is a first step in converting this test - it only handles the following test cases:

        • derby.drda.startNetworkServer set to false
        • derby.drda.startNetworkServer set to true but derby.drda.portNumber not set
        • derby.drda.startNetworkServer set to true and derby.drda.portNumber set

        The next test case will need to examine derby.log

        Show
        Myrna van Lunteren added a comment - Attaching a patch which is a first step in converting this test - it only handles the following test cases: derby.drda.startNetworkServer set to false derby.drda.startNetworkServer set to true but derby.drda.portNumber not set derby.drda.startNetworkServer set to true and derby.drda.portNumber set The next test case will need to examine derby.log
        Hide
        Knut Anders Hatlen added a comment -

        Did you forget "svn add" before creating the patch? The one I downloaded was almost empty...

        Show
        Knut Anders Hatlen added a comment - Did you forget "svn add" before creating the patch? The one I downloaded was almost empty...
        Hide
        Myrna van Lunteren added a comment -

        Methinks I did forget svn add.
        Here's a new version of the patch (same file name) after svn add.

        Show
        Myrna van Lunteren added a comment - Methinks I did forget svn add. Here's a new version of the patch (same file name) after svn add.
        Hide
        Dag H. Wanvik added a comment -

        Thanks, Myrna! Some comments:

        • unused import: org.apache.derbyTesting.junit.JDBC
        • line 142: getBasePort is static, so better to use TestConfiguration.getBasePort ?
        • TODO: missing: port equals -1. Should this work or not?
        • xtestStartNetworkServerTrueNoPort: rename, the initial x is often used to comment out test cases, so confusing.
        • removeSystemProperty("derby.drda.startNetworkServer") in setUp necessary? If so, shouldn't some other tests have cleaned up? Suggest pushing and popping setting in test on setUp/tearDown
        • TODO: testStartNetworkServerFalse: remove debugging output testStartNetworkServerFalse: incorrect indent in much of method
        • pingForServerUp: if we expect server not to come up, return before 60 seconds, I think, to save test time. It shouldn't matter much for coverage. Maybe 15s?
        • xtestStartNetworkServerTrueNoPort: bad indent line 2
        • TODO; startNetworkServerTrueHelper remove debug printing
        • "(emb)derbynet.DerbyNetAutoStartTest.testStartNetworkServerLogMessageOnDoubleBoot xtestchecklog
          the port is 1532" failed for me.

        [Error/failure logged at Wed Mar 20 16:33:53 EST 2013]
        junit.framework.AssertionFailedError: Timed out waiting for network server to start (localhost:1527)
        at junit.framework.Assert.fail(Assert.java:47)
        at org.apache.derbyTesting.junit.NetworkServerTestSetup.waitForServerStart(NetworkServerTestSetup.java:565)
        at org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest.testStartNetworkServerLogMessageOnDoubleBoot(DerbyNetAutoStartTest.java:196)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:601)
        at junit.framework.TestCase.runTest(TestCase.java:164)
        at junit.framework.TestCase.runBare(TestCase.java:130)
        at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117)
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424)
        at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441)
        at junit.framework.TestResult$1.protect(TestResult.java:106)
        at junit.framework.TestResult.runProtected(TestResult.java:124)
        at junit.framework.TestResult.run(TestResult.java:109)
        at junit.framework.TestCase.run(TestCase.java:120)
        at junit.framework.TestSuite.runTest(TestSuite.java:230)
        at junit.framework.TestSuite.run(TestSuite.java:225)
        at junit.framework.TestSuite.runTest(TestSuite.java:230)
        at junit.framework.TestSuite.run(TestSuite.java:225)
        at junit.framework.TestSuite.runTest(TestSuite.java:230)
        at junit.framework.TestSuite.run(TestSuite.java:225)
        at junit.textui.TestRunner.doRun(TestRunner.java:121)
        at junit.textui.TestRunner.start(TestRunner.java:185)
        at junit.textui.TestRunner.main(TestRunner.java:143)

        Note: are we trying to access 1527? Test says its trying to reach 1532?

        • Another failure: 2) testStartNetworkServerFalse(org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest)junit.framework.AssertionFailedError
          at org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest.testStartNetworkServerFalse(DerbyNetAutoStartTest.java:96)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424)
          at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441)

        Could it be that the test fixtures are run in "wrong" order? Java 1.7 runs them in arbitrary order...

        • Some lines > 80 long.
        Show
        Dag H. Wanvik added a comment - Thanks, Myrna! Some comments: unused import: org.apache.derbyTesting.junit.JDBC line 142: getBasePort is static, so better to use TestConfiguration.getBasePort ? TODO: missing: port equals -1. Should this work or not? xtestStartNetworkServerTrueNoPort: rename, the initial x is often used to comment out test cases, so confusing. removeSystemProperty("derby.drda.startNetworkServer") in setUp necessary? If so, shouldn't some other tests have cleaned up? Suggest pushing and popping setting in test on setUp/tearDown TODO: testStartNetworkServerFalse: remove debugging output testStartNetworkServerFalse: incorrect indent in much of method pingForServerUp: if we expect server not to come up, return before 60 seconds, I think, to save test time. It shouldn't matter much for coverage. Maybe 15s? xtestStartNetworkServerTrueNoPort: bad indent line 2 TODO; startNetworkServerTrueHelper remove debug printing "(emb)derbynet.DerbyNetAutoStartTest.testStartNetworkServerLogMessageOnDoubleBoot xtestchecklog the port is 1532" failed for me. [Error/failure logged at Wed Mar 20 16:33:53 EST 2013] junit.framework.AssertionFailedError: Timed out waiting for network server to start (localhost:1527) at junit.framework.Assert.fail(Assert.java:47) at org.apache.derbyTesting.junit.NetworkServerTestSetup.waitForServerStart(NetworkServerTestSetup.java:565) at org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest.testStartNetworkServerLogMessageOnDoubleBoot(DerbyNetAutoStartTest.java:196) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:601) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.textui.TestRunner.doRun(TestRunner.java:121) at junit.textui.TestRunner.start(TestRunner.java:185) at junit.textui.TestRunner.main(TestRunner.java:143) Note: are we trying to access 1527? Test says its trying to reach 1532? Another failure: 2) testStartNetworkServerFalse(org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest)junit.framework.AssertionFailedError at org.apache.derbyTesting.functionTests.tests.derbynet.DerbyNetAutoStartTest.testStartNetworkServerFalse(DerbyNetAutoStartTest.java:96) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:117) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBareOverridable(BaseJDBCTestCase.java:424) at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:441) Could it be that the test fixtures are run in "wrong" order? Java 1.7 runs them in arbitrary order... Some lines > 80 long.
        Hide
        Myrna van Lunteren added a comment -

        Hi Dag, thanks for reviewing the patch!

        I actually wanted to upload a cleaner version of the test but uploaded a work-in-progress version...
        Hence the non-working testStartNetworkServerLogMessageOnDoubleBoot, and left-over 'TODO's re removing debugging lines...

        I took care of a number of your comments already, but will try to finish the remaining issues before posting a new patch...

        Show
        Myrna van Lunteren added a comment - Hi Dag, thanks for reviewing the patch! I actually wanted to upload a cleaner version of the test but uploaded a work-in-progress version... Hence the non-working testStartNetworkServerLogMessageOnDoubleBoot, and left-over 'TODO's re removing debugging lines... I took care of a number of your comments already, but will try to finish the remaining issues before posting a new patch...
        Hide
        Dag H. Wanvik added a comment -

        Hi Myrna, yes I realized that the patch wasn't finished; just thought it easier to mention all that I saw Thanks for converting this test; the old one doesn't run on my Windows box I notice (instability I presume...)

        Show
        Dag H. Wanvik added a comment - Hi Myrna, yes I realized that the patch wasn't finished; just thought it easier to mention all that I saw Thanks for converting this test; the old one doesn't run on my Windows box I notice (instability I presume...)
        Hide
        Myrna van Lunteren added a comment -

        Attaching another intermediate patch - this one is pretty much finished.

        I did not add it into a suite yet (nor have I run the test in a suite).

        I need to do that, and remove the original and its canons from the derbyclientmats.
        I also think this test needs an exception clause against running with CDC profile.
        I also think maybe we need to have this forced in English locale, as we're checking on the contents of derby.log with an expected English string.

        I think I addressed many of Dag's comments.
        However, I tried the test with ibm 1.7 and it did not run the fixtures in a different sequence, so I wonder how this version runs with jdk 1.7. I can probably find one later.

        Also, re the port number - I've used getNextAvailablePort, which ups the number every time it is used. I don't know exactly why - even if I just run this test - the junit framework thinks it's up to number 1532 - up 5 from 1527 - I assume under the covers we're cranking up the number...
        I added 2 numbers to the maximum in TestConfiguration, although if the test runs with -Dderby.tests.basePort, we're only using getNextAvailablePort once.

        Patch ready for review.

        Show
        Myrna van Lunteren added a comment - Attaching another intermediate patch - this one is pretty much finished. I did not add it into a suite yet (nor have I run the test in a suite). I need to do that, and remove the original and its canons from the derbyclientmats. I also think this test needs an exception clause against running with CDC profile. I also think maybe we need to have this forced in English locale, as we're checking on the contents of derby.log with an expected English string. I think I addressed many of Dag's comments. However, I tried the test with ibm 1.7 and it did not run the fixtures in a different sequence, so I wonder how this version runs with jdk 1.7. I can probably find one later. Also, re the port number - I've used getNextAvailablePort, which ups the number every time it is used. I don't know exactly why - even if I just run this test - the junit framework thinks it's up to number 1532 - up 5 from 1527 - I assume under the covers we're cranking up the number... I added 2 numbers to the maximum in TestConfiguration, although if the test runs with -Dderby.tests.basePort, we're only using getNextAvailablePort once. Patch ready for review.
        Hide
        Dag H. Wanvik added a comment -

        Thanks, Myrna! For me the test took over 4 minutes to finish: in line
        ca 250 I see this code:

        // Check the server is still up
        assertTrue(NetworkServerTestSetup.pingForServerUp(ns, null, false));

        Should it be "true" here for the argument "expectServerUp" (since we expect the server to still be up)? If I substitute with "true", the test runs quickly... It seems all pingForServerUp does if the argument is "false" is sit around for 4 minutes observing the server is (still) up and then returns true...

        Show
        Dag H. Wanvik added a comment - Thanks, Myrna! For me the test took over 4 minutes to finish: in line ca 250 I see this code: // Check the server is still up assertTrue(NetworkServerTestSetup.pingForServerUp(ns, null, false)); Should it be "true" here for the argument "expectServerUp" (since we expect the server to still be up)? If I substitute with "true", the test runs quickly... It seems all pingForServerUp does if the argument is "false" is sit around for 4 minutes observing the server is (still) up and then returns true...
        Hide
        Dag H. Wanvik added a comment -

        Perhaps you use "false" here to make sure the server doesn't go down within a "reasonable" time after the previous operation, .i.e. just using "true" to check if it is still up may not be sufficient since it could still go down due to the shutdowns asynchronous nature after a while (if something doesnt work the way we expect it to). Even so, I think waiting for 4 minutes here is excessive.

        Show
        Dag H. Wanvik added a comment - Perhaps you use "false" here to make sure the server doesn't go down within a "reasonable" time after the previous operation, .i.e. just using "true" to check if it is still up may not be sufficient since it could still go down due to the shutdowns asynchronous nature after a while (if something doesnt work the way we expect it to). Even so, I think waiting for 4 minutes here is excessive.
        Hide
        Dag H. Wanvik added a comment -

        Ps! The Javadoc for pingForServerUp could use some improvement now.

        Show
        Dag H. Wanvik added a comment - Ps! The Javadoc for pingForServerUp could use some improvement now.
        Hide
        Myrna van Lunteren added a comment -

        Thanks for the review Dag.
        You're right, re the expected server up - it should be expecting true...

        However, I don't think that is the section causing the 4 mins elapsed time.
        Instead, if I eliminate the final check in the fixture testStartNetworkServerLogMessageOnDualStart:
        ns.shutdown();
        assertFalse(NetworkServerTestSetup.pingForServerUp(ns, null, false));
        then the fixture takes about 2 seconds instead of 2.5 mins on my system.
        I don't think that check (to see if the server was really down) is very useful, and certainly not worth that much time. I'll eliminate it.
        I'll run full tests.

        Show
        Myrna van Lunteren added a comment - Thanks for the review Dag. You're right, re the expected server up - it should be expecting true... However, I don't think that is the section causing the 4 mins elapsed time. Instead, if I eliminate the final check in the fixture testStartNetworkServerLogMessageOnDualStart: ns.shutdown(); assertFalse(NetworkServerTestSetup.pingForServerUp(ns, null, false)); then the fixture takes about 2 seconds instead of 2.5 mins on my system. I don't think that check (to see if the server was really down) is very useful, and certainly not worth that much time. I'll eliminate it. I'll run full tests.
        Hide
        Myrna van Lunteren added a comment -

        Attaching another patch.
        This one has the indicated change to check for server up with expected 'true', and does not do the final unnecessary check in the fixture testStartNetworkServerLogMessageOnDualStart().

        Also added a force to the English locale for that fixture and matching steps in teardown to undo.

        Also added a check so this test doesn't run with CDC/ME.

        And removed the old test and master and all references.

        suites.All and derbyall passed for me like this, I will commit shortly.

        Show
        Myrna van Lunteren added a comment - Attaching another patch. This one has the indicated change to check for server up with expected 'true', and does not do the final unnecessary check in the fixture testStartNetworkServerLogMessageOnDualStart(). Also added a force to the English locale for that fixture and matching steps in teardown to undo. Also added a check so this test doesn't run with CDC/ME. And removed the old test and master and all references. suites.All and derbyall passed for me like this, I will commit shortly.
        Hide
        Myrna van Lunteren added a comment -

        committed patch _3 with revision 146415 (http://svn.apache.org/viewvc?view=revision&revision=1461415).

        Show
        Myrna van Lunteren added a comment - committed patch _3 with revision 146415 ( http://svn.apache.org/viewvc?view=revision&revision=1461415 ).
        Hide
        Myrna van Lunteren added a comment -

        The converted test ran fine in the various nightly runs.
        Backported the change to 10.9 with revision 1462304 (http://svn.apache.org/viewvc?view=revision&revision=1462304)

        Show
        Myrna van Lunteren added a comment - The converted test ran fine in the various nightly runs. Backported the change to 10.9 with revision 1462304 ( http://svn.apache.org/viewvc?view=revision&revision=1462304 )
        Hide
        Dag H. Wanvik added a comment -

        Thanks for porting this, Myrna! The test now runs quickly for me too.

        Show
        Dag H. Wanvik added a comment - Thanks for porting this, Myrna! The test now runs quickly for me too.
        Hide
        Myrna van Lunteren added a comment -

        I intend to backport to 10.8, but I am not yet used to having a 10.10 branch, so forgot that step.
        I'll backport to 10.10.
        Also, although the test did get skipped with weme6.2 on trunk, after the backport to 10.9, I see the test failing with weme6.2 there, so I need to make sure it gets skipped properly.

        Show
        Myrna van Lunteren added a comment - I intend to backport to 10.8, but I am not yet used to having a 10.10 branch, so forgot that step. I'll backport to 10.10. Also, although the test did get skipped with weme6.2 on trunk, after the backport to 10.9, I see the test failing with weme6.2 there, so I need to make sure it gets skipped properly.
        Hide
        Myrna van Lunteren added a comment -

        I also saw a 10.9 build failure from Jenkins but I don't see how that could be caused by my check-in.
        Also see that there are test failures in the runs at Oracle with 10.9 and jdk 8 on one platform with 10.9 after my check-in, but the same failures happened after the previous check-in, so I'm ignoring those too.

        Show
        Myrna van Lunteren added a comment - I also saw a 10.9 build failure from Jenkins but I don't see how that could be caused by my check-in. Also see that there are test failures in the runs at Oracle with 10.9 and jdk 8 on one platform with 10.9 after my check-in, but the same failures happened after the previous check-in, so I'm ignoring those too.
        Hide
        Myrna van Lunteren added a comment -

        merged the change to the 10.10 branch and committed with revision 1462658 (http://svn.apache.org/viewvc?view=revision&revision=r1462658).

        Show
        Myrna van Lunteren added a comment - merged the change to the 10.10 branch and committed with revision 1462658 ( http://svn.apache.org/viewvc?view=revision&revision=r1462658 ).
        Hide
        Myrna van Lunteren added a comment -

        moved the check to ensure this gets skipped with JSR169/CDC into the suite method in the 10.9 branch, committed with revision 1462677 (http://svn.apache.org/viewvc?view=revision&revision=1462677).

        Show
        Myrna van Lunteren added a comment - moved the check to ensure this gets skipped with JSR169/CDC into the suite method in the 10.9 branch, committed with revision 1462677 ( http://svn.apache.org/viewvc?view=revision&revision=1462677 ).
        Hide
        Myrna van Lunteren added a comment -

        backported revision 1462304 and 1462677 from 10.9 into 10.8 with revision 1462681 http://svn.apache.org/viewvc?view=revision&revision=1462681.

        Show
        Myrna van Lunteren added a comment - backported revision 1462304 and 1462677 from 10.9 into 10.8 with revision 1462681 http://svn.apache.org/viewvc?view=revision&revision=1462681 .
        Hide
        Myrna van Lunteren added a comment -

        Noticed a derbynetautostart.properties file in the java/testing/org/apache/derbyTesting/functionTests/suites directory, which only had instructions to skip with jfoundation. I found no trace of a derbynetautostart.runall, and the derbynetautostart.properties file didn't include any other suite, so this file was never doing anything in particular. Further, we're now not supporting j2ME anymore and the test has been converted to junit, so I removed the file from trunk without doing any testing.
        Also removed the reference from java/testing/README.htm.
        committed with revision http://svn.apache.org/viewvc?view=revision&revision=1480446.

        Show
        Myrna van Lunteren added a comment - Noticed a derbynetautostart.properties file in the java/testing/org/apache/derbyTesting/functionTests/suites directory, which only had instructions to skip with jfoundation. I found no trace of a derbynetautostart.runall, and the derbynetautostart.properties file didn't include any other suite, so this file was never doing anything in particular. Further, we're now not supporting j2ME anymore and the test has been converted to junit, so I removed the file from trunk without doing any testing. Also removed the reference from java/testing/README.htm. committed with revision http://svn.apache.org/viewvc?view=revision&revision=1480446 .

          People

          • Assignee:
            Myrna van Lunteren
            Reporter:
            Erlend Birkenes
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development