[ 
http://issues.apache.org/jira/browse/DERBY-1141?page=comments#action_12376272 ] 

John H. Embretsen commented on DERBY-1141:
------------------------------------------

Comments on patch DERBY-1141_20060424.diff:

Myrna, 
Most of it looks good; very good code comments this time :) 
Also, thanks for including my proposed solution for JUnit tests.

That said, I still want to comment on a couple of issues:

1)  

RunTest.java, @@ -291,9 +296,41 @@:

I appreciate the detailed comments here. However, the patch adds:

+            if ((!useprocess) && (isSuiteRun))
+            {
+                boolean started = false;
+                try 
+                {
+                    started = ns.testNetworkServerConnection();
+                }
+                catch (Exception e) {} // ignore
+                if (!started)
+                    ns.start(); // start but don't stop, so not setting 
serverNeedsStopping
+                if (started && !startServer)
+                    ns.stop();
+            }

What if the server has not been started (i.e. started = false), and startServer 
= false? Won't the server be started anyway in this case?
In other words, with this code the following happens (as far as I understand 
it):

1. The network server is started, regardless of the value of the startServer 
variable.
2. If startServer = false (i.e., the harness is instructed not to start the 
server), the server will be stopped if it was already running when executing 
the try block, but it will not be stopped if it was started within the if 
construct following the catch block.

Here is what I suggest to make this work as intended:

+                if (!started && startServer)
+                    ns.start(); // start but don't stop, so not setting 
serverNeedsStopping
+                if (started && !startServer)
+                    ns.stop();


2) 

I tried running the jdbcapi suite (jdk1.4 and 1.5) with useprocess=false, but 
the SUR*.junit tests failed with NullPointerExceptions. I tried applying the 
patch to SVN revision 395093. There may be something wrong in my environment if 
it works for you with this revision or a later one...

Also, when I run the jdbcapi suite with useprocess=false the harness notifies 
me (via the console output) that bestrowidentifier.sql was skipped. However, 
this test is not listed in jdbcapi.skip or jdbcapi_skip.txt. Myrna, do you know 
how to fix this?


3) 

I think the following comment in the *Useprocess.exclude files is invalid in 
this version of the patch:

+# note: junit tests automatically switch to useprocess=true


> test harness usage of useprocess needs improvement
> --------------------------------------------------
>
>          Key: DERBY-1141
>          URL: http://issues.apache.org/jira/browse/DERBY-1141
>      Project: Derby
>         Type: Improvement

>   Components: Test
>     Reporter: Myrna van Lunteren
>     Assignee: Myrna van Lunteren
>  Attachments: DERBY-1141_20060413.diff, DERBY-1141_20060413.stat, 
> DERBY-1141_20060424.diff, DERBY-1141_20060424.stat, 
> JUnit_useprocessFalse_v1.diff
>
> The 'old' functionTests harness has property useprocess, which, when set to 
> false, causes tests within a suite to run from within one jvm, thus creating 
> databases and files in the same directory.
> This functionality is useful for debugging test runs within an IDE.
> Also, it is potentially useful for speeding up testruns.
> However, currently, there are some problems and shortcomings of this 
> functionality.
> - For instance, when running a networkserver test with framework DerbyNet of 
> type .java one sees the message 'Security Manager not installed' show up 
> within the test output, thus causing the test to fail.
> - running derbynet(client)mats suite with useprocess=false doesn't actually 
> make things any faster,  because networkserver gets started fresh for every 
> test. 
>   However, it should be feasible to start networkserver with the first test 
> in a suite, then use that 
>   networkserver  for subsequent tests and shutdown networkserver when the 
> suite finishes.
> - Also, (reported by Mike Matrigali off-list) currently, the following:
> ------------------
> java -Duseprocess=false -Doutputdir=<snippeddirname>/newout  -Dkeepfiles=true 
> org.apache.derbyTesting.functionTests.harness.RunTest lang/closed.java
> ------------------
> makes the test fall out of the harness with: Exception in thread "main"
> and in the .tmp:
> ------------------
> java.security.AccessControlException: access denied (java.io.FilePermission 
> <snippeddirname>\newout read)
>         at 
> java.security.AccessControlContext.checkPermission(AccessControlContext.java:269)
>         at 
> java.security.AccessController.checkPermission(AccessController.java:401)
>         at java.lang.SecurityManager.checkPermission(SecurityManager.java:524)
>         at java.lang.SecurityManager.checkRead(SecurityManager.java:863)
>         at java.io.File.exists(File.java:678)
>         at java.io.Win32FileSystem.canonicalize(Win32FileSystem.java:360)
>         at java.io.File.getCanonicalPath(File.java:513)
>         at 
> org.apache.derbyTesting.functionTests.harness.RunTest.execTestNoProcess(RunTest.java:2370)
>         at 
> org.apache.derbyTesting.functionTests.harness.RunTest.testRun(RunTest.java:443)
>         at 
> org.apache.derbyTesting.functionTests.harness.RunTest.main(RunTest.java:302)
> --------------------------------
>   removing the -Doutputdir gets around that problem, but shouldn't be needed.
> - there is the reference 
> http://www.nabble.com/AccessControlException-when-running-functional-tests-t1321374.html#a3526947
>   I couldn't duplicate this yet, maybe more info to follow.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Reply via email to