Derby
  1. Derby
  2. DERBY-3834

Convert derbynet/runtimeinfo to JUnit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.1.1
    • Fix Version/s: 10.5.3.2, 10.6.1.0
    • Component/s: Test
    • Labels:
      None
    1. DERBY-3834-1.patch
      0.7 kB
      Tiago R. Espinha
    2. DERBY-3834.patch
      51 kB
      Tiago R. Espinha
    3. DERBY-3834.patch
      50 kB
      Tiago R. Espinha
    4. DERBY-3834.patch
      50 kB
      Tiago R. Espinha
    5. DERBY-3834.patch
      49 kB
      Tiago R. Espinha
    6. DERBY-3834.patch
      46 kB
      Tiago R. Espinha
    7. DERBY-3834.patch
      41 kB
      Tiago R. Espinha
    8. DERBY-3834.stat
      0.6 kB
      Tiago R. Espinha
    9. DERBY-3834.patch
      41 kB
      Tiago R. Espinha
    10. Derby-3834_1.diff
      42 kB
      Erlend Birkenes

      Issue Links

        Activity

        Hide
        Myrna van Lunteren added a comment -

        Committed the backport, including 1 revision for DERBY-4307, with revision 1064388.
        I had to make one modification; for some reason, the calls to TestConfiguration.getCurrent().getPort() resulted in -1 when the strings static global assignments. I moved them as final strings into the methods where they were used, and that works.

        Show
        Myrna van Lunteren added a comment - Committed the backport, including 1 revision for DERBY-4307 , with revision 1064388. I had to make one modification; for some reason, the calls to TestConfiguration.getCurrent().getPort() resulted in -1 when the strings static global assignments. I moved them as final strings into the methods where they were used, and that works.
        Hide
        Myrna van Lunteren added a comment -

        reopen to record that I'm backporting this to 10.5

        Show
        Myrna van Lunteren added a comment - reopen to record that I'm backporting this to 10.5
        Hide
        Tiago R. Espinha added a comment -

        The patches were committed and the regressions are running fine. Closing the issue.

        Show
        Tiago R. Espinha added a comment - The patches were committed and the regressions are running fine. Closing the issue.
        Hide
        Tiago R. Espinha added a comment -

        Attaching a small patch to fix the Javadoc failure.

        Show
        Tiago R. Espinha added a comment - Attaching a small patch to fix the Javadoc failure.
        Hide
        Kathey Marsden added a comment -

        It looks like this introduced a javadoc warning:
        Failed with following errors. Check details in docs.txt
        [javadoc] C:\nightlies\main\src\opensource\java\testing\org\apache\derbyTesting\functionTests\tests\derbynet\RuntimeInfoTest.java:78: warning - @return tag has no arguments.
        [javadoc] 1 warning
        Build: OK

        Show
        Kathey Marsden added a comment - It looks like this introduced a javadoc warning: Failed with following errors. Check details in docs.txt [javadoc] C:\nightlies\main\src\opensource\java\testing\org\apache\derbyTesting\functionTests\tests\derbynet\RuntimeInfoTest.java:78: warning - @return tag has no arguments. [javadoc] 1 warning Build: OK
        Hide
        Kathey Marsden added a comment -

        Committed revision 792001

        Show
        Kathey Marsden added a comment - Committed revision 792001
        Hide
        Tiago R. Espinha added a comment -

        The suites.All suite ran with no failures. I think the patch is ready for commit.

        Show
        Tiago R. Espinha added a comment - The suites.All suite ran with no failures. I think the patch is ready for commit.
        Hide
        Tiago R. Espinha added a comment -

        You are right Kathey, that seems to be the offending method. I'm submitting a patch that looks just for the initial part of the string, which should be common cross-locales.

        Running regressions as well.

        Show
        Tiago R. Espinha added a comment - You are right Kathey, that seems to be the offending method. I'm submitting a patch that looks just for the initial part of the string, which should be common cross-locales. Running regressions as well.
        Hide
        Kathey Marsden added a comment -

        I think the problem is related to the vetPing() method of NetworkServerTestSetup looking for an English string?
        If I change vetPing to just look for "DRDA_NoIO.S:"
        then it seems to run ok.

        Show
        Kathey Marsden added a comment - I think the problem is related to the vetPing() method of NetworkServerTestSetup looking for an English string? If I change vetPing to just look for "DRDA_NoIO.S:" then it seems to run ok.
        Hide
        Tiago R. Espinha added a comment -

        Hello Kathey,

        I'm not sure if that is it. I would think that we have this same situation in other tests, no? They should be independent...

        I find it weird that it only happens under Linux... makes me think that there is some underlying difference on how the JVM is handling stuff around. Maybe it also has to do with the default locale... or with file permissions somehow. I will have to look at it later today.

        Show
        Tiago R. Espinha added a comment - Hello Kathey, I'm not sure if that is it. I would think that we have this same situation in other tests, no? They should be independent... I find it weird that it only happens under Linux... makes me think that there is some underlying difference on how the JVM is handling stuff around. Maybe it also has to do with the default locale... or with file permissions somehow. I will have to look at it later today.
        Hide
        Kathey Marsden added a comment -

        On my linux machine with and without basePort set running the test on its own I am getting a Connection refused message to the console in German and then a timeout failure for the test.
        It is at least consistent so I should be able to debug tomorrow. Could it be related to having two clientServerDecorator instances in a single test?

        Show
        Kathey Marsden added a comment - On my linux machine with and without basePort set running the test on its own I am getting a Connection refused message to the console in German and then a timeout failure for the test. It is at least consistent so I should be able to debug tomorrow. Could it be related to having two clientServerDecorator instances in a single test?
        Hide
        Kathey Marsden added a comment -

        Hi Tiago I ran suites.All with your latest patch and got no failures on Windows XP with derby.tests.basePort=11123. I will do a run on linux with the same setting and if I see no issues will go ahead and commit.

        Show
        Kathey Marsden added a comment - Hi Tiago I ran suites.All with your latest patch and got no failures on Windows XP with derby.tests.basePort=11123. I will do a run on linux with the same setting and if I see no issues will go ahead and commit.
        Hide
        Tiago R. Espinha added a comment -

        I got failures again:
        -------------------8<---------------
        Time: 6,031.741
        There were 2 failures:
        1) org.apache.derbyTesting.functionTests.tests.derbynet.RuntimeInfoTestjunit.framework.AssertionFailedError: Timed out waiting for network server to start
        at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:200)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:20)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        2) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by:
        java.sql.SQLNonTransientConnectionException: The DDM object 0x1232 is not supported. The connection has been terminated.
        at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70)
        at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358)
        at org.apache.derby.client.am.Connection.createStatement(Connection.java:368)
        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.update(StressMultiTest.java:558)
        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:402)
        at java.lang.Thread.run(Thread.java:619)
        Caused by: org.apache.derby.client.am.SqlException: The DDM object 0x1232 is not supported. The connection has been terminated.
        at org.apache.derby.client.am.SqlException.copyAsUnchainedSQLException(SqlException.java:505)
        at org.apache.derby.client.am.Sqlca.chainDeferredExceptionsToAgentOrAsConnectionWarnings(Sqlca.java:342)
        at org.apache.derby.client.am.Sqlca.getJDBCMessage(Sqlca.java:304)
        at org.apache.derby.client.am.SqlException.getMessage(SqlException.java:402)
        at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358)
        at org.apache.derby.client.am.Statement.executeUpdate(Statement.java:509)
        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.update(StressMultiTest.java:560)
        ... 2 more

        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331)
        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70)
        at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425)
        at java.lang.Thread.run(Thread.java:619)

        FAILURES!!!
        Tests run: 11312, Failures: 2, Errors: 0
        -----------------------8<----------------------
        I can't seem to find the reason for these though...

        Show
        Tiago R. Espinha added a comment - I got failures again: ------------------- 8< --------------- Time: 6,031.741 There were 2 failures: 1) org.apache.derbyTesting.functionTests.tests.derbynet.RuntimeInfoTestjunit.framework.AssertionFailedError: Timed out waiting for network server to start at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:200) at junit.extensions.TestSetup$1.protect(TestSetup.java:20) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 2) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by: java.sql.SQLNonTransientConnectionException: The DDM object 0x1232 is not supported. The connection has been terminated. at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70) at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358) at org.apache.derby.client.am.Connection.createStatement(Connection.java:368) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.update(StressMultiTest.java:558) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:402) at java.lang.Thread.run(Thread.java:619) Caused by: org.apache.derby.client.am.SqlException: The DDM object 0x1232 is not supported. The connection has been terminated. at org.apache.derby.client.am.SqlException.copyAsUnchainedSQLException(SqlException.java:505) at org.apache.derby.client.am.Sqlca.chainDeferredExceptionsToAgentOrAsConnectionWarnings(Sqlca.java:342) at org.apache.derby.client.am.Sqlca.getJDBCMessage(Sqlca.java:304) at org.apache.derby.client.am.SqlException.getMessage(SqlException.java:402) at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358) at org.apache.derby.client.am.Statement.executeUpdate(Statement.java:509) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.update(StressMultiTest.java:560) ... 2 more at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425) at java.lang.Thread.run(Thread.java:619) FAILURES!!! Tests run: 11312, Failures: 2, Errors: 0 ----------------------- 8< ---------------------- I can't seem to find the reason for these though...
        Hide
        Tiago R. Espinha added a comment -

        I'm still rather unsure about the status of this patch. Last night I got some failures once more - namely failures with beans, which seems to happen every now and then. The failures also had an error message in German, which might raise a red flag. The weird part is that I can consistently run the derbynet suite without any failures...

        I'm running suites.All tonight once more to see how it pans out.

        Show
        Tiago R. Espinha added a comment - I'm still rather unsure about the status of this patch. Last night I got some failures once more - namely failures with beans, which seems to happen every now and then. The failures also had an error message in German, which might raise a red flag. The weird part is that I can consistently run the derbynet suite without any failures... I'm running suites.All tonight once more to see how it pans out.
        Hide
        Tiago R. Espinha added a comment -

        The said bug with the port is now fixed in DERBY-4217. Still, upon running suites.All I got:
        ---------8<-----------------------
        Time: 5,827.406
        There was 1 failure:
        1) org.apache.derbyTesting.functionTests.tests.derbynet.RuntimeInfoTestjunit.framework.AssertionFailedError: Timed out waiting for network server to start
        at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:200)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:20)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)
        at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
        at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
        at junit.extensions.TestSetup.run(TestSetup.java:25)

        FAILURES!!!
        Tests run: 11312, Failures: 1, Errors: 0
        ---------------------8<---------------------------------

        It seems a rather innocent failure and after running the derbynet suite again twice the failure did not pop again. It needs to be said that the suites.All was ran simultaneously with derbyall. Still, I think we can chalk it up to delays in the server response due to the high load. Nonetheless, I will be running suites.All again, this time alone.

        Test runs permitting, the patch seems to be ready for commit.

        Show
        Tiago R. Espinha added a comment - The said bug with the port is now fixed in DERBY-4217 . Still, upon running suites.All I got: --------- 8< ----------------------- Time: 5,827.406 There was 1 failure: 1) org.apache.derbyTesting.functionTests.tests.derbynet.RuntimeInfoTestjunit.framework.AssertionFailedError: Timed out waiting for network server to start at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:200) at junit.extensions.TestSetup$1.protect(TestSetup.java:20) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) FAILURES!!! Tests run: 11312, Failures: 1, Errors: 0 --------------------- 8< --------------------------------- It seems a rather innocent failure and after running the derbynet suite again twice the failure did not pop again. It needs to be said that the suites.All was ran simultaneously with derbyall. Still, I think we can chalk it up to delays in the server response due to the high load. Nonetheless, I will be running suites.All again, this time alone. Test runs permitting, the patch seems to be ready for commit.
        Hide
        Tiago R. Espinha added a comment -

        We want to hold this patch for now. My suites.All run crashed with some bug related to DERBY-4217 - it seems that when using a basePort with 5 digits (i.e. higher than 9999), the SysinfoTest will crash. It might be unrelated with this issue in the end but I'm still investigating why this is happening.

        Show
        Tiago R. Espinha added a comment - We want to hold this patch for now. My suites.All run crashed with some bug related to DERBY-4217 - it seems that when using a basePort with 5 digits (i.e. higher than 9999), the SysinfoTest will crash. It might be unrelated with this issue in the end but I'm still investigating why this is happening.
        Hide
        Tiago R. Espinha added a comment -

        All the patches down to Erlend's had a cut & paste error on the policy file names. Fixed that. The tests will be ran with the fixed patch.

        Show
        Tiago R. Espinha added a comment - All the patches down to Erlend's had a cut & paste error on the policy file names. Fixed that. The tests will be ran with the fixed patch.
        Hide
        Tiago R. Espinha added a comment -

        Forgot to add the Apache license header to the new file.

        I'll be running suites.All and derbyall with this patch tonight.

        Show
        Tiago R. Espinha added a comment - Forgot to add the Apache license header to the new file. I'll be running suites.All and derbyall with this patch tonight.
        Hide
        Tiago R. Espinha added a comment -

        Attaching a new patch to this issue.

        After having discussed this with Kathey on IRC, we agreed on an alternative solution for this issue. Instead of separating the locale test from the actual RuntimeInfo test, I created a LocaleTestSetup decorator that allows the test to run with different localizations.

        The solution I found to manage the expected outputs was to use an HashMaps inside HashMaps. This way we're able to select the set of expected outputs for a certain locale easily, and then it is also easy to pick the correct output message within the results for that locale. It might not be the best solution performance-wise, but this way it is rather easy to add new locales to be tested, should we want to.

        Still, some input and alternative ideas are appreciated.

        Show
        Tiago R. Espinha added a comment - Attaching a new patch to this issue. After having discussed this with Kathey on IRC, we agreed on an alternative solution for this issue. Instead of separating the locale test from the actual RuntimeInfo test, I created a LocaleTestSetup decorator that allows the test to run with different localizations. The solution I found to manage the expected outputs was to use an HashMaps inside HashMaps. This way we're able to select the set of expected outputs for a certain locale easily, and then it is also easy to pick the correct output message within the results for that locale. It might not be the best solution performance-wise, but this way it is rather easy to add new locales to be tested, should we want to. Still, some input and alternative ideas are appreciated.
        Hide
        Kathey Marsden added a comment -

        SysinfoLocaleTest seems to be an embedded tests a and tests derby.ui.locale which is not relevant here. Do you think it will adapt itself well to the network server runtimeinfo testing?

        I guess I am still leaning toward a locale specific runtimeinfo test. I suppose catching an exception and resetting the locale would work too, but seems a little messier.

        Show
        Kathey Marsden added a comment - SysinfoLocaleTest seems to be an embedded tests a and tests derby.ui.locale which is not relevant here. Do you think it will adapt itself well to the network server runtimeinfo testing? I guess I am still leaning toward a locale specific runtimeinfo test. I suppose catching an exception and resetting the locale would work too, but seems a little messier.
        Hide
        Tiago R. Espinha added a comment -

        I just noticed that we have a SysinfoLocaleTest that tests just the localized output of Sysinfo. Would it be a viable option to have a single LocaleTest where all these outputs are tested?

        Show
        Tiago R. Espinha added a comment - I just noticed that we have a SysinfoLocaleTest that tests just the localized output of Sysinfo. Would it be a viable option to have a single LocaleTest where all these outputs are tested?
        Hide
        Tiago R. Espinha added a comment -

        Hello Kathey,

        Well, considering I'm manipulating the server that the test spawns automatically, it should never stay running. I'm not spawning an independent server just to accommodate the locale testing; instead, I just stop the server that the test automatically starts, change the locale and start the server again.

        The odd case would be if this fixture fails, but a possible solution is to encapsulate this method in a try..catch statement. Since all the fixtures are actually invoked manually because the order in which they are ran matters, I can just catch the exception that this method may throw and set the locale back to normal.

        I'm just laying this on the table because it seems to me a bit of overkill to create a whole new test with just one fixture that tests the locale.

        Show
        Tiago R. Espinha added a comment - Hello Kathey, Well, considering I'm manipulating the server that the test spawns automatically, it should never stay running. I'm not spawning an independent server just to accommodate the locale testing; instead, I just stop the server that the test automatically starts, change the locale and start the server again. The odd case would be if this fixture fails, but a possible solution is to encapsulate this method in a try..catch statement. Since all the fixtures are actually invoked manually because the order in which they are ran matters, I can just catch the exception that this method may throw and set the locale back to normal. I'm just laying this on the table because it seems to me a bit of overkill to create a whole new test with just one fixture that tests the locale.
        Hide
        Kathey Marsden added a comment -

        Hi Tiago,

        I am concerned that if this tests fails we will be left with a network server running or leave the tests set to the German locale, causing subsequent tests to fail.

        I think it would be better to make a second test
        LocaleRuntimeInfoTest or some such and then use:
        public NetworkServerTestSetup
        (
        Test test,
        String[] systemProperties,
        String[] startupArgs,
        boolean serverShouldComeUp
        )

        to spawn the server with the correct locale by setting system properties. It will make sure everything is shutdown and cleaned up regardless of whether the test fails.

        Show
        Kathey Marsden added a comment - Hi Tiago, I am concerned that if this tests fails we will be left with a network server running or leave the tests set to the German locale, causing subsequent tests to fail. I think it would be better to make a second test LocaleRuntimeInfoTest or some such and then use: public NetworkServerTestSetup ( Test test, String[] systemProperties, String[] startupArgs, boolean serverShouldComeUp ) to spawn the server with the correct locale by setting system properties. It will make sure everything is shutdown and cleaned up regardless of whether the test fails.
        Hide
        Tiago R. Espinha added a comment -

        Attaching a new patch file.

        This patch removes the test from the exclude and the runall files and it also addresses the locale issue. In the old test, it seems that the locale wasn't even being properly tested. In this patch however, in the x_testRuntimeInfoLocale() fixture I changed the locale to German, restarted the server and then the fixture is processed normally (accounting for German output). Just before the fixture ends, I reset the locale back to what it was originally and once more restart the server so that the last fixture is able to complete normally.

        There's one detail I'd like to get some input on. I added a method to this test called waitForServerShutdown(), which, as the name suggests, holds on the execution until the server shuts down. This has the same purpose as NetworkServerTestSetup.waitForServerStart(); but in this case we also want to wait for it to shutdown. Would it be acceptable to create the waitForServerShutdown() method also in the NetworkServerTestSetup? It might be useful for other tests and it's pretty standard, it just waits for the server to shutdown. Plus, if other tests require it in the future, they won't have to re-implement it locally.

        Finally, I also added the comment on the singleUseDatabase usage, like Kathey suggested.

        Show
        Tiago R. Espinha added a comment - Attaching a new patch file. This patch removes the test from the exclude and the runall files and it also addresses the locale issue. In the old test, it seems that the locale wasn't even being properly tested. In this patch however, in the x_testRuntimeInfoLocale() fixture I changed the locale to German, restarted the server and then the fixture is processed normally (accounting for German output). Just before the fixture ends, I reset the locale back to what it was originally and once more restart the server so that the last fixture is able to complete normally. There's one detail I'd like to get some input on. I added a method to this test called waitForServerShutdown(), which, as the name suggests, holds on the execution until the server shuts down. This has the same purpose as NetworkServerTestSetup.waitForServerStart(); but in this case we also want to wait for it to shutdown. Would it be acceptable to create the waitForServerShutdown() method also in the NetworkServerTestSetup? It might be useful for other tests and it's pretty standard, it just waits for the server to shutdown. Plus, if other tests require it in the future, they won't have to re-implement it locally. Finally, I also added the comment on the singleUseDatabase usage, like Kathey suggested.
        Hide
        Tiago R. Espinha added a comment -

        I have been discussing the locale bit with Kathey and we were able to draw some conclusions, but there's still a pending issue.

        The original test apparently did no verification at all on the locale front. Despite setting the user.country and user.language properties, the output it expects is actually in English, so it doesn't seem like it was expected for it to work properly.

        Still, we agreed it would be a nice addition to the test, having it check the localized output. The problem arises with the fact that the output of runtimeinfo is dependent on the locale of the server and not of the client (unlike ping, for example). So for this to work, that one single fixture has to shutdown the server and spawn another instance with the changed locale.

        Does anyone have ideas or comments on this?

        It is also noteworthy that the test seems to pick up the localization properties if I define them with -D on the command line.

        Show
        Tiago R. Espinha added a comment - I have been discussing the locale bit with Kathey and we were able to draw some conclusions, but there's still a pending issue. The original test apparently did no verification at all on the locale front. Despite setting the user.country and user.language properties, the output it expects is actually in English, so it doesn't seem like it was expected for it to work properly. Still, we agreed it would be a nice addition to the test, having it check the localized output. The problem arises with the fact that the output of runtimeinfo is dependent on the locale of the server and not of the client (unlike ping, for example). So for this to work, that one single fixture has to shutdown the server and spawn another instance with the changed locale. Does anyone have ideas or comments on this? It is also noteworthy that the test seems to pick up the localization properties if I define them with -D on the command line.
        Hide
        Kathey Marsden added a comment -

        Thanks Tiago for reviving this test. I ran the test with IBM 1.6 and it passed and confirmed that it is skipped with weme.

        You should remove references to the old test in functionTests/suites/
        DerbyNetClientRemote.exclude
        DerbyNetClientUseprocess.exclude
        DerbyNetRemote.exclude
        DerbyNetUseprocess.exclude
        derbynetmats.runall
        j9derbynetmats.runall

        and run derbyall to make sure all references to the old test are removed.

        In the test I think it would be good to comment that a separate database was necessary to ensure consistent output above the call to singleUseDatabaseDecorator.

        I think x_testRuntimeInfo and x_testRuntimeInfoMethod and related constants could be more descriptively named, like x_testRuntimeInfoWithActiveConn and x_testRuntimeInfoAfterConnClose.

        I am a bit confused by x_testRuntimeInfoLocale, even in the original test. I would have expected it to test non-English output, but instead runs with:
        "-Duser.language=err", "-Duser.country=DE",
        which makes it revert to English I guess. I think it would be better if it tested the localization with user.language=de and then just check the output for some German string.

        Show
        Kathey Marsden added a comment - Thanks Tiago for reviving this test. I ran the test with IBM 1.6 and it passed and confirmed that it is skipped with weme. You should remove references to the old test in functionTests/suites/ DerbyNetClientRemote.exclude DerbyNetClientUseprocess.exclude DerbyNetRemote.exclude DerbyNetUseprocess.exclude derbynetmats.runall j9derbynetmats.runall and run derbyall to make sure all references to the old test are removed. In the test I think it would be good to comment that a separate database was necessary to ensure consistent output above the call to singleUseDatabaseDecorator. I think x_testRuntimeInfo and x_testRuntimeInfoMethod and related constants could be more descriptively named, like x_testRuntimeInfoWithActiveConn and x_testRuntimeInfoAfterConnClose. I am a bit confused by x_testRuntimeInfoLocale, even in the original test. I would have expected it to test non-English output, but instead runs with: "-Duser.language=err", "-Duser.country=DE", which makes it revert to English I guess. I think it would be better if it tested the localization with user.language=de and then just check the output for some German string.
        Hide
        Tiago R. Espinha added a comment -

        Ran suites.All and I got a 100% pass.

        Show
        Tiago R. Espinha added a comment - Ran suites.All and I got a 100% pass.
        Hide
        Tiago R. Espinha added a comment -

        Attaching a new patch that compiles without errors against JDK1.4.2.

        It also accounts for the customizable port part (DERBY-4217), which the previous patch didn't.

        Show
        Tiago R. Espinha added a comment - Attaching a new patch that compiles without errors against JDK1.4.2. It also accounts for the customizable port part ( DERBY-4217 ), which the previous patch didn't.
        Hide
        Tiago R. Espinha added a comment -

        Attaching a patch to the issue.

        This patch has Myrna's remarks in consideration. It no longer uses the ExecProcUtil but execJavaCmd instead. It also uses the println() in BaseTestCase which will only print when the tests are being ran on verbose mode.

        This patch also fixes the issue where only the second run would pass. Since it is impossible to predict the order by which the prepared statements will appear under x_testRuntimeInfoLocale(), I worked around it by parsing just parts of the output.

        Also, the methods are invoked by a single fixture testRunTests(), since the order by which they are ran does matter and we can't rely on JUnit to run them in order.

        Show
        Tiago R. Espinha added a comment - Attaching a patch to the issue. This patch has Myrna's remarks in consideration. It no longer uses the ExecProcUtil but execJavaCmd instead. It also uses the println() in BaseTestCase which will only print when the tests are being ran on verbose mode. This patch also fixes the issue where only the second run would pass. Since it is impossible to predict the order by which the prepared statements will appear under x_testRuntimeInfoLocale(), I worked around it by parsing just parts of the output. Also, the methods are invoked by a single fixture testRunTests(), since the order by which they are ran does matter and we can't rely on JUnit to run them in order.
        Hide
        Tiago R. Espinha added a comment -

        I'll be taking over this issue as part of DERBY-4090

        Show
        Tiago R. Espinha added a comment - I'll be taking over this issue as part of DERBY-4090
        Hide
        Myrna van Lunteren added a comment -

        I ran the test, but the first time, it fails for me.
        If I run it again without cleaning up the dir, it passes.
        Maybe some of the details regarding sessions and statements are different when you first create a database...

        So maybe you can wrap the test in a singleUseDecorator and adjust the expected output to what it would be when there are no pre-existing databases.

        Other comments:

        • I'd like it better if you could avoid using ExecProcUtil.execCmdDumpResults...
        • There is a property, derby.tests.debug, which can be set if someone wants to debug a test. I suggest you remove the boolean 'print' and instead, in the method 'print()' use org.apache.derbyTesting.junit.println' rather than System.out.println. That way, no change of the code is needed to debug the test.
        Show
        Myrna van Lunteren added a comment - I ran the test, but the first time, it fails for me. If I run it again without cleaning up the dir, it passes. Maybe some of the details regarding sessions and statements are different when you first create a database... So maybe you can wrap the test in a singleUseDecorator and adjust the expected output to what it would be when there are no pre-existing databases. Other comments: I'd like it better if you could avoid using ExecProcUtil.execCmdDumpResults... There is a property, derby.tests.debug, which can be set if someone wants to debug a test. I suggest you remove the boolean 'print' and instead, in the method 'print()' use org.apache.derbyTesting.junit.println' rather than System.out.println. That way, no change of the code is needed to debug the test.
        Hide
        Erlend Birkenes added a comment -

        Please review.

        Added the new test to suites.All and remove from derbyall and deleted old files.

        -Erlend

        Show
        Erlend Birkenes added a comment - Please review. Added the new test to suites.All and remove from derbyall and deleted old files. -Erlend

          People

          • Assignee:
            Tiago R. Espinha
            Reporter:
            Erlend Birkenes
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development