[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12539621 ] Daniel John Debrunner commented on DERBY-3156: -- The test contains a number of static fields, do they all need to be static? Static fields will lead to problems and are not typical in JUnit tests, each test fixture(testXXX method) is an unique instance of a class so either instance fields should be used or even better just pass the information around. E.g. the setUp method will be called for every fixture, so there's no real benefit to storing information in fields since they will not be shared across fixtures. Can you comment the reason the test doesn't run with a security manager. > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Attachments: 3156-diff, 3156-diff.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12540205 ] Knut Anders Hatlen commented on DERBY-3156: --- Hi Vemund, The patch generally looks good. A couple of comments/questions: 1) Do the original tests run with the client driver? It seems to me they don't, so I think it's okay to run them only in embedded mode. 2) I think it would be good to null out the fields when the test has completed (in tearDown()) so that the objects they reference can be garbage collected. 3) Instead of using File.deleteOnExit(), perhaps it's cleaner to delete the files explicitly in tearDown()? Seems that it would also remove the need for the runNo field. 4) I'm wondering if this code + new File(getSystemProperty("derby.system.home") +"foo", + makeStreamFilename("file")).getCanonicalPath()); // erroneous path would be more robust if it was written as new File(new File(getSystemProperty("derby.system.home"), "foo"), ...) As it is now, I think it depends on derby.system.home ending with the path separator character. > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Attachments: 3156-diff, 3156-diff.stat, 3156-diffv2, 3156-diffv2.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12540756 ] Knut Anders Hatlen commented on DERBY-3156: --- Thanks for updating the patch, Vemund. I tried to run the test standalone, but then all test cases failed with errors like this one: 1) testDefault(org.apache.derbyTesting.functionTests.tests.engine.ErrorStreamTest)java.io.FileNotFoundException: /tmp/derbytst/system /method-1.log (No such file or directory) at java.io.FileOutputStream.open(Native Method) at java.io.FileOutputStream.(FileOutputStream.java:179) at java.io.FileOutputStream.(FileOutputStream.java:131) at org.apache.derbyTesting.functionTests.tests.engine.ErrorStreamTest$9.run(ErrorStreamTest.java:502) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.functionTests.tests.engine.ErrorStreamTest.newFileOutputStream(ErrorStreamTest.java:499) at org.apache.derbyTesting.functionTests.tests.engine.ErrorStreamTest.openStreams(ErrorStreamTest.java:360) at org.apache.derbyTesting.functionTests.tests.engine.ErrorStreamTest.setUp(ErrorStreamTest.java:100) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:95) If I first run another test and don't delete the 'system' directory, it works fine. Seems like openStreams() requires that the database directory exists. nits: The method deleteFileOnExit() is now unused and can be deleted. Perhaps the assert methods that throw IOException would have been simpler if they had thrown Exception instead? Then we could skip the try/catch+cast. The PrivilegedActionException will be linked to the IOException, so we will still get the stack trace, and if someone changes the code so that we can get other exceptions than IOException, we get the original exception instead of a ClassCastException. > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Attachments: 3156-diff, 3156-diff.stat, 3156-diffv2, > 3156-diffv2.stat, 3156-diffv3, 3156-diffv3.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12542060 ] Knut Anders Hatlen commented on DERBY-3156: --- Seems like this caused an AccessControlException on Windows. Please take a look at DERBY-3202. > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Attachments: 3156-diff, 3156-diff.stat, 3156-diffv2, > 3156-diffv2.stat, 3156-diffv3, 3156-diffv3.stat, 3156-diffv4, 3156-diffv4.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12544495 ] Knut Anders Hatlen commented on DERBY-3156: --- Could we also remove the canons now? (errorStream.out and logStream.out) > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Fix For: 10.4.0.0 > > Attachments: 3156-diff, 3156-diff.stat, 3156-diffv2, > 3156-diffv2.stat, 3156-diffv3, 3156-diffv3.stat, 3156-diffv4, > 3156-diffv4.stat, 3156-remove-old-tests-diff, 3156-remove-old-tests-diff.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3156) Convert testing of derby error stream to JUnit
[ https://issues.apache.org/jira/browse/DERBY-3156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12544746 ] Knut Anders Hatlen commented on DERBY-3156: --- Removed errorStream.out and logStream.out and committed revision 597356. > Convert testing of derby error stream to JUnit > -- > > Key: DERBY-3156 > URL: https://issues.apache.org/jira/browse/DERBY-3156 > Project: Derby > Issue Type: Test > Components: Test >Reporter: Vemund Østgaard >Assignee: Vemund Østgaard >Priority: Minor > Fix For: 10.4.0.0 > > Attachments: 3156-diff, 3156-diff.stat, 3156-diffv2, > 3156-diffv2.stat, 3156-diffv3, 3156-diffv3.stat, 3156-diffv4, > 3156-diffv4.stat, 3156-remove-old-tests-diff, 3156-remove-old-tests-diff.stat > > > Convert the tests that are related to derby error stream properties and > functionality. > Seems to be these two tests: > lang/logStream.java > lang/errorStream.java -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.