Derby
  1. Derby
  2. DERBY-3829

Convert derbynet/sysinfo and derbynet/sysinfo_with_properties to JUnit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.1.1
    • Fix Version/s: 10.6.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      I'm guessing these two can be combined into one file

      1. Derby-3829_1.diff
        54 kB
        Erlend Birkenes
      2. Derby-3829_2.diff
        67 kB
        Erlend Birkenes
      3. DERBY-3829_m3.diff
        69 kB
        Myrna van Lunteren
      4. DERBY-3829_m5.diff
        71 kB
        Myrna van Lunteren
      5. DERBY-3829_m6.diff
        2 kB
        Myrna van Lunteren

        Issue Links

          Activity

          Hide
          Myrna van Lunteren added a comment -

          With revision 930052 I backported the conversion to 10.4, as a step to addressing DERBY-4536.

          Show
          Myrna van Lunteren added a comment - With revision 930052 I backported the conversion to 10.4, as a step to addressing DERBY-4536 .
          Hide
          Myrna van Lunteren added a comment -

          Resolving; final comments addressed; assigning to Erlend as he did most of the work; marking fixed in 10.6, but most of this did go into 10.5.

          Show
          Myrna van Lunteren added a comment - Resolving; final comments addressed; assigning to Erlend as he did most of the work; marking fixed in 10.6, but most of this did go into 10.5.
          Hide
          Myrna van Lunteren added a comment -

          Regression tests passed; committed to trunk (10.6) with revision 886857.

          Show
          Myrna van Lunteren added a comment - Regression tests passed; committed to trunk (10.6) with revision 886857.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Myrna. The changes look good. +1 to commit if the regression tests pass.

          Show
          Knut Anders Hatlen added a comment - Thanks, Myrna. The changes look good. +1 to commit if the regression tests pass.
          Hide
          Myrna van Lunteren added a comment -

          attaching a follow-up patch which I think takes care of Knut's comments.
          Will run regression tests.

          Show
          Myrna van Lunteren added a comment - attaching a follow-up patch which I think takes care of Knut's comments. Will run regression tests.
          Hide
          Kathey Marsden added a comment -

          Unassigning Erlend as I don't think he is still working on this issue. Erlend if you want to pick it back up, please reassign yourself.

          Show
          Kathey Marsden added a comment - Unassigning Erlend as I don't think he is still working on this issue. Erlend if you want to pick it back up, please reassign yourself.
          Hide
          Myrna van Lunteren added a comment -

          Thx for the review!
          I went ahead and committed my latest variation on the original patch with revision 688103.
          Leaving open so Knut's comments can be addressed in a follow-up patch.

          Show
          Myrna van Lunteren added a comment - Thx for the review! I went ahead and committed my latest variation on the original patch with revision 688103. Leaving open so Knut's comments can be addressed in a follow-up patch.
          Hide
          Knut Anders Hatlen added a comment -

          The changes to assertExecJavaCmdAsExpected() seem to have been incorporated correctly. Thanks!

          One minor comment (not important enough to hold the commit) is that I think the three new methods in BaseTestCase could be static. That would allow them to be used in more contexts than if they're not static.

          Another minor issue (copied from the existing code, not introduced here) is that the catch block in execJavaCmd() casts pe.getException() to SecurityException, but since SecurityException is a RuntimeException, it's never going to be wrapped in a PrivilegedActionException. (Some may say that it's better if the test code doesn't unwrap the PrivilegedActionExceptions, because that would simplify the code and the underlying cause would be reported by JUnit anyway.)

          Show
          Knut Anders Hatlen added a comment - The changes to assertExecJavaCmdAsExpected() seem to have been incorporated correctly. Thanks! One minor comment (not important enough to hold the commit) is that I think the three new methods in BaseTestCase could be static. That would allow them to be used in more contexts than if they're not static. Another minor issue (copied from the existing code, not introduced here) is that the catch block in execJavaCmd() casts pe.getException() to SecurityException, but since SecurityException is a RuntimeException, it's never going to be wrapped in a PrivilegedActionException. (Some may say that it's better if the test code doesn't unwrap the PrivilegedActionExceptions, because that would simplify the code and the underlying cause would be reported by JUnit anyway.)
          Hide
          Myrna van Lunteren added a comment -

          Attaching a new patch (DERBY-3829_m5.diff) which attempts to address the permissions problem I saw on windows.
          Turns out sysinfo needs read access to org.apache.derby.info.DBMS.properties. If we're using jars, that's in derby.jar, and I guess permission is already granted (I didn't double check that, it seemed logical).
          I struggled for a while trying to grant java.io.FilePermission read to $derbyTesting.codeclasses, but that one uses the actual OS's file separator and the FilePermission needs the forward slash - or so it seemed empirically.
          So the latest patch adds a property to just the SysinfoTest.policy and wraps that property into a SystemProperty Setup around the test's networkserver setup if we're using classes.

          I think there are some other tests that just return an empty suite if we're running with classes for similar reasons, but I thought it worthwhile to make it work as we're already using a decidated policy file.

          I svn updated before building this patch and tried to incorporate the recent changes to BaseTestCase.assertExecJavaCmdOK, but I wasn't quite sure that all intended changes were in or not, so a review will be appreciated.

          I ran suites.All with jars on linux (no problems), and with classes on windows...On windows I ran twice and got some strange what looked like cleanup-related errors the first time around, especially in the upgrade tests...(warnings like, table already exist etc), but they didn't show up a second time...

          All in all, if there are no further suggestions, I'd like to commit this latest variation on Erlend's work...

          Show
          Myrna van Lunteren added a comment - Attaching a new patch ( DERBY-3829 _m5.diff) which attempts to address the permissions problem I saw on windows. Turns out sysinfo needs read access to org.apache.derby.info.DBMS.properties. If we're using jars, that's in derby.jar, and I guess permission is already granted (I didn't double check that, it seemed logical). I struggled for a while trying to grant java.io.FilePermission read to $derbyTesting.codeclasses, but that one uses the actual OS's file separator and the FilePermission needs the forward slash - or so it seemed empirically. So the latest patch adds a property to just the SysinfoTest.policy and wraps that property into a SystemProperty Setup around the test's networkserver setup if we're using classes. I think there are some other tests that just return an empty suite if we're running with classes for similar reasons, but I thought it worthwhile to make it work as we're already using a decidated policy file. I svn updated before building this patch and tried to incorporate the recent changes to BaseTestCase.assertExecJavaCmdOK, but I wasn't quite sure that all intended changes were in or not, so a review will be appreciated. I ran suites.All with jars on linux (no problems), and with classes on windows...On windows I ran twice and got some strange what looked like cleanup-related errors the first time around, especially in the upgrade tests...(warnings like, table already exist etc), but they didn't show up a second time... All in all, if there are no further suggestions, I'd like to commit this latest variation on Erlend's work...
          Hide
          Knut Anders Hatlen added a comment -

          Didn't notice that the problems with assertExecJavaCmdAsExpected() were discussed here until after I checked in a fix for it under DERBY-3841.

          FWIW, I think using a char[1] buffer instead of a char[1024] buffer isn't the best way to handle this in readProcessOutput(). Probably better to keep the larger buffer, store the return value from Reader.read(char[],int,int) in a variable, and use the String(char[],int,int) constructor to specify which part of the char array to use.

          Show
          Knut Anders Hatlen added a comment - Didn't notice that the problems with assertExecJavaCmdAsExpected() were discussed here until after I checked in a fix for it under DERBY-3841 . FWIW, I think using a char [1] buffer instead of a char [1024] buffer isn't the best way to handle this in readProcessOutput(). Probably better to keep the larger buffer, store the return value from Reader.read(char[],int,int) in a variable, and use the String(char[],int,int) constructor to specify which part of the char array to use.
          Hide
          Myrna van Lunteren added a comment -

          It now looks to me like the error only occurs when I run with classes, not with jars...

          Show
          Myrna van Lunteren added a comment - It now looks to me like the error only occurs when I run with classes, not with jars...
          Hide
          Erlend Birkenes added a comment -

          I got that error once too. Only one time and I haven't been able to reproduce it. It seems like it might be a bug in the server or something.
          Does it happen every time?

          Show
          Erlend Birkenes added a comment - I got that error once too. Only one time and I haven't been able to reproduce it. It seems like it might be a bug in the server or something. Does it happen every time?
          Hide
          Erlend Birkenes added a comment -

          Thanks Myrna, thats great! I never thought of the difference in newline characters.

          I too get the permissions errors I think, at least I saw some when I printed out the whole output from sysinfo, but those lines are removed so they don't affect the test.
          I will try to figure out what causes it though, as it can be confusing and messy that they are present in the log.

          I'm working on DERBY-3834 right now and alot of your modifications are appropriate there as well, so I'll put them in before I post it.

          Show
          Erlend Birkenes added a comment - Thanks Myrna, thats great! I never thought of the difference in newline characters. I too get the permissions errors I think, at least I saw some when I printed out the whole output from sysinfo, but those lines are removed so they don't affect the test. I will try to figure out what causes it though, as it can be confusing and messy that they are present in the log. I'm working on DERBY-3834 right now and alot of your modifications are appropriate there as well, so I'll put them in before I post it.
          Hide
          Myrna van Lunteren added a comment -

          I attached my variations on patch Derby-3829_2.diff, and ran tests - derbynet._Suite and individual tests, and suites.All on linux, and it all went swimmingly...
          But then I ran suites.All on wnidows and got 2 failures in the new sysinfo test (on top of some replication failures):

          1) testSysinfoMethod(org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest)java.lang.Exception: DRDA_InvalidReplyTooShort.S:Invalid reply from network server: Insufficient data.
          at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessageWork(NetworkServerControlImpl.java:3179)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessage(NetworkServerControlImpl.java:1835)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.fillReplyBuffer(NetworkServerControlImpl.java:2771)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.readStringReply(NetworkServerControlImpl.java:2828)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.sysinfo(NetworkServerControlImpl.java:1282)
          at org.apache.derby.drda.NetworkServerControl.getSysinfo(NetworkServerControl.java:469)
          at org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest.testSysinfoMethod(SysinfoTest.java:208)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:104)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:23)
          I am trying to figure out what's up with this...

          Show
          Myrna van Lunteren added a comment - I attached my variations on patch Derby-3829_2.diff, and ran tests - derbynet._Suite and individual tests, and suites.All on linux, and it all went swimmingly... But then I ran suites.All on wnidows and got 2 failures in the new sysinfo test (on top of some replication failures): 1) testSysinfoMethod(org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest)java.lang.Exception: DRDA_InvalidReplyTooShort.S:Invalid reply from network server: Insufficient data. at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessageWork(NetworkServerControlImpl.java:3179) at org.apache.derby.impl.drda.NetworkServerControlImpl.consolePropertyMessage(NetworkServerControlImpl.java:1835) at org.apache.derby.impl.drda.NetworkServerControlImpl.fillReplyBuffer(NetworkServerControlImpl.java:2771) at org.apache.derby.impl.drda.NetworkServerControlImpl.readStringReply(NetworkServerControlImpl.java:2828) at org.apache.derby.impl.drda.NetworkServerControlImpl.sysinfo(NetworkServerControlImpl.java:1282) at org.apache.derby.drda.NetworkServerControl.getSysinfo(NetworkServerControl.java:469) at org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest.testSysinfoMethod(SysinfoTest.java:208) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:104) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:23) I am trying to figure out what's up with this...
          Hide
          Myrna van Lunteren added a comment -

          I finally figured out that the problem was with the sed() method and the comparisons;
          on windows, the end-of-line character is not \n, but \n\r, at least for most lines (empirically, it seemed, the 'Version...' line has \n. But I could have been dreaming that up).
          I added a replaceAll line to swap all \n\r to \n and then my problems went away.
          I assume the permissions problem I saw was a result of left over policy file settings.

          other modifications over previous patch:

          • the new class had somehow gotten a mix of tabs and spaces. I turned it into all spaces.
          • network server tests can't run with JSR169 - nor when derbynet is not available.
            added those two conditions to the suite() method (returns empty suite if not ok).
          • the code in org.apache.derbyTesting.functionTests.harness should not be used anymore, this
            includes references to the jvm class, and setting of j9_13 jvm etc.
            I took that section out.
          • junit keeps state of all global variables until the entire suite (e.g. suites.All) is done.
            So, using global variables is a drain on resources.
            If it can't be avoided making a variable global, then it should get set to null in a teardown() method.
            I took care of this
          • the print variable was no longer needed but was accidentally left behind
          • the makePolicyName() was doing a System.out.println. I see this is done in some other tests (e.g.
            ServerPropertiesTest, which I converted), but we might as well make it fail, I think.
          Show
          Myrna van Lunteren added a comment - I finally figured out that the problem was with the sed() method and the comparisons; on windows, the end-of-line character is not \n, but \n\r, at least for most lines (empirically, it seemed, the 'Version...' line has \n. But I could have been dreaming that up). I added a replaceAll line to swap all \n\r to \n and then my problems went away. I assume the permissions problem I saw was a result of left over policy file settings. other modifications over previous patch: the new class had somehow gotten a mix of tabs and spaces. I turned it into all spaces. network server tests can't run with JSR169 - nor when derbynet is not available. added those two conditions to the suite() method (returns empty suite if not ok). the code in org.apache.derbyTesting.functionTests.harness should not be used anymore, this includes references to the jvm class, and setting of j9_13 jvm etc. I took that section out. junit keeps state of all global variables until the entire suite (e.g. suites.All) is done. So, using global variables is a drain on resources. If it can't be avoided making a variable global, then it should get set to null in a teardown() method. I took care of this the print variable was no longer needed but was accidentally left behind the makePolicyName() was doing a System.out.println. I see this is done in some other tests (e.g. ServerPropertiesTest, which I converted), but we might as well make it fail, I think.
          Hide
          Myrna van Lunteren added a comment -

          I'm looking at the new patch, and on the whole, I like it; however, I still get a similar failure as before...
          I did verify the failure happens only on my windows XP system.
          I'm trying to figure out what the problem is...

          Show
          Myrna van Lunteren added a comment - I'm looking at the new patch, and on the whole, I like it; however, I still get a similar failure as before... I did verify the failure happens only on my windows XP system. I'm trying to figure out what the problem is...
          Hide
          Erlend Birkenes added a comment - - edited

          In this patch i changed BaseTestCase.assertExecJavaCmdAsExpected and added two new methods named execJavaCmd (which executes the command and returns a Process) and readProcessOutput which reads the output from a Process and returns it as a String. Two methods was necessary to allow assertExecJavaCmdAsExpected to check the exit code of the process.

          the new method BaseTestCase.execJavaCmd is exactly the same as Utilities.execJavaCmd, and that method was only used in one test, derbynet/ServerPropertiesTest, so I changed it to use the new BaseTestCase.execJavaCmd instead and removed Utilities.execJavaCmd.

          derbynet/runtimeinfo still uses ExecProcUtil.execCmdDumpResults but when DERBY-3834 is resolved I will remove ExecProcUtil as there is only that one method in there.

          I also added a singleUseDatabaseDecorator, because of the problem in DERBY-3834, just in case.

          I didn't do anything about the permissions-problem Myrna was seeing, because I don't have that problem here and need more information about it.

          I ran the test from classes and jars and ran derbynet/_Suite. Also ran all the tests that uses assertExecJavaCmdAsExpected to confirm that it still works the same way.

          Show
          Erlend Birkenes added a comment - - edited In this patch i changed BaseTestCase.assertExecJavaCmdAsExpected and added two new methods named execJavaCmd (which executes the command and returns a Process) and readProcessOutput which reads the output from a Process and returns it as a String. Two methods was necessary to allow assertExecJavaCmdAsExpected to check the exit code of the process. the new method BaseTestCase.execJavaCmd is exactly the same as Utilities.execJavaCmd, and that method was only used in one test, derbynet/ServerPropertiesTest, so I changed it to use the new BaseTestCase.execJavaCmd instead and removed Utilities.execJavaCmd. derbynet/runtimeinfo still uses ExecProcUtil.execCmdDumpResults but when DERBY-3834 is resolved I will remove ExecProcUtil as there is only that one method in there. I also added a singleUseDatabaseDecorator, because of the problem in DERBY-3834 , just in case. I didn't do anything about the permissions-problem Myrna was seeing, because I don't have that problem here and need more information about it. I ran the test from classes and jars and ran derbynet/_Suite. Also ran all the tests that uses assertExecJavaCmdAsExpected to confirm that it still works the same way.
          Hide
          Erlend Birkenes added a comment -

          Never mind, I think I fixed it. If I change it to char[1] and remove the .trim() from output = output + new String(ca).trim() it works fine.
          The other tests which use this still seems to work too.

          Show
          Erlend Birkenes added a comment - Never mind, I think I fixed it. If I change it to char [1] and remove the .trim() from output = output + new String(ca).trim() it works fine. The other tests which use this still seems to work too.
          Hide
          Erlend Birkenes added a comment -

          There is still something wrong with the way assertExecJavaCmdAsExpected captures output.

          I get this output:

          --------- Derby Network Server Information --------
          Version: CSS10050/10.5.0.0 alpha - (1) Build: 1 DRDA Product Id: CSS10050 ** SANE BUILD **
          – listing properties –
          derby.drda.traceDirectory=/home/erlend/workspace/Derby/system
          derby.drda.maxThreads=0
          derby.drda.sslMode=off
          derby.drda.keepAlive=true
          derby.drda.minThreads=0
          derby.drda.portNumber=1527
          derby.drda.logConnections=false
          derby.drda.timeSlice=0
          derby.drda.startNetworkServer=false
          derby.drda.host=127.0.0.1
          derby.drda.traceAll=false
          ------------------ Java Information ------------------
          Java Version: 1.5.0_15
          Java Vendor: Sun Microsystems Inc.
          Java home: /usr/lib/jvm/java-1.5.0-sun-1.5.0.15/jre
          Java classpath: /home/erlend/workspace/Derby/classes:/home/erlend/Download/eclipse/plugins/org.junit_3.8.2.v200706111738/junit.jar:/home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/79/1/.cp/:/home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/7/1/.cp/
          OS name: Linux
          OS architecture: i386
          OS version:2.6.24-19-generic
          Java user name: erlend
          Java user home: /home/erlend
          Java user dir: /home/erlend/workspace/Derby
          java.specification.name: Java Platform API Specification
          java.specification.version: 1.5
          --------- Derby Information --------
          JRE - JDBC: J2SE 5.0 - JDBC 3.0
          [/home/erlend/workspace/Derby/classes] 10.5.0.0 alpha - (1)
          [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/plugins/org.junit_3.8.2.v200706111738/junit.jar read)] <null>
          [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/79/1/.cp read)] <null>
          [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/7/1/.cp read)] <null>
          ------------------------------------------------------
          ----------------- Locale Information -----------------
          Current Locale : [English/United States [en_US]]
          Found support for locale: [cs]
          version: 10.5.0.0 alpha- (1)
          Found support for locale: [de_DE]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [es]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [fr]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [hu]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [it]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [ja_JP]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [ko_KR]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [pl]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [pt_BR]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [ru]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [zh_CN]
          version: 10.5.0.0 alpha - (1)
          Found support for locale: [zh_TW]
          version: 10.5.0.0 alpha - (1)
          ------------------------------------------------------

          -------------------------
          ----------------- Locale Information -----------------
          Current Locale : [English/United States [en_US]]
          Found support for locale: [cs]
          version: 10.5.0.0 alpha

          The last 5 lines should not be there.. i tried playing with the char[] length and it changes the output, but I can't get it to go away. And if it did I suspect it could still be wrong with different output.

          Show
          Erlend Birkenes added a comment - There is still something wrong with the way assertExecJavaCmdAsExpected captures output. I get this output: --------- Derby Network Server Information -------- Version: CSS10050/10.5.0.0 alpha - (1) Build: 1 DRDA Product Id: CSS10050 ** SANE BUILD ** – listing properties – derby.drda.traceDirectory=/home/erlend/workspace/Derby/system derby.drda.maxThreads=0 derby.drda.sslMode=off derby.drda.keepAlive=true derby.drda.minThreads=0 derby.drda.portNumber=1527 derby.drda.logConnections=false derby.drda.timeSlice=0 derby.drda.startNetworkServer=false derby.drda.host=127.0.0.1 derby.drda.traceAll=false ------------------ Java Information ------------------ Java Version: 1.5.0_15 Java Vendor: Sun Microsystems Inc. Java home: /usr/lib/jvm/java-1.5.0-sun-1.5.0.15/jre Java classpath: /home/erlend/workspace/Derby/classes:/home/erlend/Download/eclipse/plugins/org.junit_3.8.2.v200706111738/junit.jar:/home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/79/1/.cp/:/home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/7/1/.cp/ OS name: Linux OS architecture: i386 OS version:2.6.24-19-generic Java user name: erlend Java user home: /home/erlend Java user dir: /home/erlend/workspace/Derby java.specification.name: Java Platform API Specification java.specification.version: 1.5 --------- Derby Information -------- JRE - JDBC: J2SE 5.0 - JDBC 3.0 [/home/erlend/workspace/Derby/classes] 10.5.0.0 alpha - (1) [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/plugins/org.junit_3.8.2.v200706111738/junit.jar read)] <null> [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/79/1/.cp read)] <null> [Java Security Exception: access denied (java.io.FilePermission /home/erlend/Download/eclipse/configuration/org.eclipse.osgi/bundles/7/1/.cp read)] <null> ------------------------------------------------------ ----------------- Locale Information ----------------- Current Locale : [English/United States [en_US] ] Found support for locale: [cs] version: 10.5.0.0 alpha- (1) Found support for locale: [de_DE] version: 10.5.0.0 alpha - (1) Found support for locale: [es] version: 10.5.0.0 alpha - (1) Found support for locale: [fr] version: 10.5.0.0 alpha - (1) Found support for locale: [hu] version: 10.5.0.0 alpha - (1) Found support for locale: [it] version: 10.5.0.0 alpha - (1) Found support for locale: [ja_JP] version: 10.5.0.0 alpha - (1) Found support for locale: [ko_KR] version: 10.5.0.0 alpha - (1) Found support for locale: [pl] version: 10.5.0.0 alpha - (1) Found support for locale: [pt_BR] version: 10.5.0.0 alpha - (1) Found support for locale: [ru] version: 10.5.0.0 alpha - (1) Found support for locale: [zh_CN] version: 10.5.0.0 alpha - (1) Found support for locale: [zh_TW] version: 10.5.0.0 alpha - (1) ------------------------------------------------------ ------------------------- ----------------- Locale Information ----------------- Current Locale : [English/United States [en_US] ] Found support for locale: [cs] version: 10.5.0.0 alpha The last 5 lines should not be there.. i tried playing with the char[] length and it changes the output, but I can't get it to go away. And if it did I suspect it could still be wrong with different output.
          Hide
          Myrna van Lunteren added a comment -

          Some of the troubles I had before the assertExecJavaCmdAsExpected got its current shape are documented in DERBY-3088...

          Show
          Myrna van Lunteren added a comment - Some of the troubles I had before the assertExecJavaCmdAsExpected got its current shape are documented in DERBY-3088 ...
          Hide
          Myrna van Lunteren added a comment -

          Hm...
          I can do some further experimenting with the permissions...and/or send you the output I get. Maybe I can modify the exec command so it starts the sysinfo call without security manager..
          But it's sort of late in the day here though and I need to do other stuff, I'll try to look into this further tomorrow...

          I think if you can make a variation of assertExecJavaCmdAsExpected to return the full output that would be good. I'm fairly certain I tried that at some point and there was some reason why that didn't work well...Can't remember the details.
          (The buffer would get closed before it got to the calling method? Or it would never release and hang the test/networkserver? Or keep the pointer open & thus leak memory? Something like that.)
          I'll see if I can find notes/remarks...(they're probably buried somewhere in a comment).

          But if we could do it, I'd prefer that over using execCmdDumpResults. We probably should remove that method, if it's not used anywhere anymore, to prevent future confusion...

          Show
          Myrna van Lunteren added a comment - Hm... I can do some further experimenting with the permissions...and/or send you the output I get. Maybe I can modify the exec command so it starts the sysinfo call without security manager.. But it's sort of late in the day here though and I need to do other stuff, I'll try to look into this further tomorrow... I think if you can make a variation of assertExecJavaCmdAsExpected to return the full output that would be good. I'm fairly certain I tried that at some point and there was some reason why that didn't work well...Can't remember the details. (The buffer would get closed before it got to the calling method? Or it would never release and hang the test/networkserver? Or keep the pointer open & thus leak memory? Something like that.) I'll see if I can find notes/remarks...(they're probably buried somewhere in a comment). But if we could do it, I'd prefer that over using execCmdDumpResults. We probably should remove that method, if it's not used anywhere anymore, to prevent future confusion...
          Hide
          Erlend Birkenes added a comment -

          Hmm, I don't get the permissions errors or any other errors here, seems something is different on windows. I'll try to figure out what..

          I'll change ExecProcUtil.execCmdDumpResults to something else. I could have just used BaseTestCase.assertExecJavaCmdAsExpected if it could deal with the parts of the output that are variable..

          Is it a good idea to extract the command-running part of assertExecJavaCmdAsExpected into a new method that just runs the command and returns the output? assertExecJavaCmdAsExpected would call the new method so it would stay the same, but the output would be available for manipulation when that's needed. That would be useful for DERBY-3824 as well. The exact same thing is done there, so it would at least save some duplicate code. However, this is the last two places where execCmdDumpResults is used, so maybe it's not worth messing with it..

          Show
          Erlend Birkenes added a comment - Hmm, I don't get the permissions errors or any other errors here, seems something is different on windows. I'll try to figure out what.. I'll change ExecProcUtil.execCmdDumpResults to something else. I could have just used BaseTestCase.assertExecJavaCmdAsExpected if it could deal with the parts of the output that are variable.. Is it a good idea to extract the command-running part of assertExecJavaCmdAsExpected into a new method that just runs the command and returns the output? assertExecJavaCmdAsExpected would call the new method so it would stay the same, but the output would be available for manipulation when that's needed. That would be useful for DERBY-3824 as well. The exact same thing is done there, so it would at least save some duplicate code. However, this is the last two places where execCmdDumpResults is used, so maybe it's not worth messing with it..
          Hide
          Myrna van Lunteren added a comment -

          I don't know, but I do get a permissions problem reading from my classes dir...From derby.log:
          2008-08-14 21:31:21.109 GMT : access denied (java.io.FilePermission <...my classes dir...> read)
          java.security.AccessControlException: access denied (java.io.FilePermission <...my classes dir...> read)
          at java.security.AccessControlContext.checkPermission(AccessControlContext.java:264)
          at java.security.AccessController.checkPermission(AccessController.java:427)
          at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
          at java.lang.SecurityManager.checkRead(SecurityManager.java:871)
          at java.io.File.exists(File.java:700)
          at java.io.Win32FileSystem.canonicalize(Win32FileSystem.java:401)
          at java.io.File.getCanonicalPath(File.java:531)
          at org.apache.derby.impl.tools.sysinfo.Main.formatURL(Main.java:1260)
          at org.apache.derby.impl.tools.sysinfo.Main.loadZipFromResource(Main.java:877)
          at org.apache.derby.impl.tools.sysinfo.Main.getAllInfo(Main.java:781)
          at org.apache.derby.impl.tools.sysinfo.Main.reportDerby(Main.java:241)
          at org.apache.derby.impl.tools.sysinfo.Main.getMainInfo(Main.java:141)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.getCLSSysInfo(NetworkServerControlImpl.java:2092)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.sendSysInfo(NetworkServerControlImpl.java:1980)
          at org.apache.derby.impl.drda.NetworkServerControlImpl.processCommands(NetworkServerControlImpl.java:1659)
          at org.apache.derby.impl.drda.DRDAConnThread.sessionInitialState(DRDAConnThread.java:645)
          at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:279)

          Show
          Myrna van Lunteren added a comment - I don't know, but I do get a permissions problem reading from my classes dir...From derby.log: 2008-08-14 21:31:21.109 GMT : access denied (java.io.FilePermission <...my classes dir...> read) java.security.AccessControlException: access denied (java.io.FilePermission <...my classes dir...> read) at java.security.AccessControlContext.checkPermission(AccessControlContext.java:264) at java.security.AccessController.checkPermission(AccessController.java:427) at java.lang.SecurityManager.checkPermission(SecurityManager.java:532) at java.lang.SecurityManager.checkRead(SecurityManager.java:871) at java.io.File.exists(File.java:700) at java.io.Win32FileSystem.canonicalize(Win32FileSystem.java:401) at java.io.File.getCanonicalPath(File.java:531) at org.apache.derby.impl.tools.sysinfo.Main.formatURL(Main.java:1260) at org.apache.derby.impl.tools.sysinfo.Main.loadZipFromResource(Main.java:877) at org.apache.derby.impl.tools.sysinfo.Main.getAllInfo(Main.java:781) at org.apache.derby.impl.tools.sysinfo.Main.reportDerby(Main.java:241) at org.apache.derby.impl.tools.sysinfo.Main.getMainInfo(Main.java:141) at org.apache.derby.impl.drda.NetworkServerControlImpl.getCLSSysInfo(NetworkServerControlImpl.java:2092) at org.apache.derby.impl.drda.NetworkServerControlImpl.sendSysInfo(NetworkServerControlImpl.java:1980) at org.apache.derby.impl.drda.NetworkServerControlImpl.processCommands(NetworkServerControlImpl.java:1659) at org.apache.derby.impl.drda.DRDAConnThread.sessionInitialState(DRDAConnThread.java:645) at org.apache.derby.impl.drda.DRDAConnThread.run(DRDAConnThread.java:279)
          Hide
          Kathey Marsden added a comment -

          The comments regarding BufferedOutputStream may indeed be an issue, but I don't think it is the cause of the error. That error looks like a protocol error that shouldn't happen at all. Do we get a protocol error if permissions are not right?

          Show
          Kathey Marsden added a comment - The comments regarding BufferedOutputStream may indeed be an issue, but I don't think it is the cause of the error. That error looks like a protocol error that shouldn't happen at all. Do we get a protocol error if permissions are not right?
          Hide
          Myrna van Lunteren added a comment -

          Looks mostly ok, however I got 5 failures when I ran the test (with classes), and I think the main cause is this:
          testSysinfoMethod(org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest)java.lang.Exception: DRDA_InvalidReplyTooShort.S:Invalid reply from network server: Insufficient data.

          I see you've used BufferedOutputStream and ExecProcUtil.execCmdDumpResults(SysInfoLocaleCmd,vCmd,bos).

          The ExecProcUtil is really part of the "old" test harness...

          Furthermore, what comes through the buffer may not always be the same, or the full output. That's why we used an InputStreamReader with the BaseTestCase.assertExecJavaCmdAsExpected.

          Show
          Myrna van Lunteren added a comment - Looks mostly ok, however I got 5 failures when I ran the test (with classes), and I think the main cause is this: testSysinfoMethod(org.apache.derbyTesting.functionTests.tests.derbynet.SysinfoTest)java.lang.Exception: DRDA_InvalidReplyTooShort.S:Invalid reply from network server: Insufficient data. I see you've used BufferedOutputStream and ExecProcUtil.execCmdDumpResults(SysInfoLocaleCmd,vCmd,bos). The ExecProcUtil is really part of the "old" test harness... Furthermore, what comes through the buffer may not always be the same, or the full output. That's why we used an InputStreamReader with the BaseTestCase.assertExecJavaCmdAsExpected.
          Hide
          Erlend Birkenes added a comment -

          Here is a complete patch. Please review and commit if it's ok.

          I put both the old tests into one new one called derbynet/SysinfoTest. I thought about adding them to NetworkServerControlApiTest but I figured it would make NetworkServerControlApiTest very messy.

          A few things:
          The old test tested localized sysinfo, but the output from the server is not actually localized. It's exactly the same as en_US output. I included the localized test, but it tests against the same string as the en_US one. If the output is changed in the future a separate string can easily be added for localized.
          Is that ok, or should I try to find a way to test the localization?

          Also, when the tests run from jars, all the locale info is not included in the output at all so I just removed it from all the tests to make it simpler. Hope that's ok too.

          I added the new test to derbynet/_Suite and removed the old one and all its files (i think..), i ran the _Suite both from classes and jars.

          Ok, I hope I remembered everything!

          -Erlend

          Show
          Erlend Birkenes added a comment - Here is a complete patch. Please review and commit if it's ok. I put both the old tests into one new one called derbynet/SysinfoTest. I thought about adding them to NetworkServerControlApiTest but I figured it would make NetworkServerControlApiTest very messy. A few things: The old test tested localized sysinfo, but the output from the server is not actually localized. It's exactly the same as en_US output. I included the localized test, but it tests against the same string as the en_US one. If the output is changed in the future a separate string can easily be added for localized. Is that ok, or should I try to find a way to test the localization? Also, when the tests run from jars, all the locale info is not included in the output at all so I just removed it from all the tests to make it simpler. Hope that's ok too. I added the new test to derbynet/_Suite and removed the old one and all its files (i think..), i ran the _Suite both from classes and jars. Ok, I hope I remembered everything! -Erlend
          Hide
          Erlend Birkenes added a comment -

          Good idea, I'll try that.

          Show
          Erlend Birkenes added a comment - Good idea, I'll try that.
          Hide
          Kathey Marsden added a comment -

          or you might be able to add them to NetworkServerControlApiTest

          Show
          Kathey Marsden added a comment - or you might be able to add them to NetworkServerControlApiTest

            People

            • Assignee:
              Erlend Birkenes
              Reporter:
              Erlend Birkenes
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development