db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Van Couvering (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-919) improve pattern for setting up junit tests
Date Tue, 14 Mar 2006 19:18:39 GMT
    [ http://issues.apache.org/jira/browse/DERBY-919?page=comments#action_12370400 ] 

David Van Couvering commented on DERBY-919:
-------------------------------------------

I think it is great to have base unit test like this, although I agree with Andreas that this
should be renamed.  This class is almost solely about obtaining connections using different
frameworks, and is very JDBC-specific.  There are plenty of unit tests that have no need for
this functionality.

I am not sure how this work integrates with/coincides with the work Andreas did to create
a junit test type which allows you to run 'raw' JUnit tests under the harness.  Can you explain?

Here are my general comments...

Javadoc:
- getConnection() - it would be great to say it connects to the default database, instead
of "some database", which is a little disconcerting as it sounds somewhat non-deterministic.

- You should document what the defaults are, such as "wombat" for the database name and "APP"
for username and password.

- why can't setup() or teardown() automaticlly call resetState() rather than asking the user
to do it?  This seems dangerous and highly error-prone.

- when I try to click on a method name, I get a "404" error, although I can scroll directly
down to view the method details.  I suspect this is probably just a JIRA bug.

BaseDerbyJUnitTest:

- I don't understand why you set a property for the database name before calling obtainConnection()
rather than just passing in the database name to obtainConnection().  It would be good to
at least explain the motivation behind this, as at first read it seems a bit odd.  Scanning
at the code, it looks like you use db(P_DBNAME) in createBaseDatabaseUrl() where you could
just as easily pass the database name in as a parameter

- There are a lot of defaults being setup in a hardcoded fashion in resetState().  It would
be better to have a section of static finals at the top with all the default values so that
someone looking at this code can tell right away what they are.  Actually, looking at Andreas'
TestConfiguration, that is a nice way of doing it . Having it as a separate class also seems
to be useful and more coherent.

- You have a lot of useful debug statements, but your javadoc doesn't really explain how to
turn debug on and off, nor is it clear from the class (e.g. there is not "debug(boolean)"
method).

- Your javadoc should be a little more explicit about how the default framework gets set

- It would be nice to have a helper method that allows subclasses to quickly obtain other
properties set by the harness (e.g. so they don't have to do a privileged block every time
they want to get a system property)

- I like Andreas' typesafe JDBCClient class, it seems a better approach.

Thanks,

David


> improve pattern for setting up junit tests
> ------------------------------------------
>
>          Key: DERBY-919
>          URL: http://issues.apache.org/jira/browse/DERBY-919
>      Project: Derby
>         Type: Sub-task
>   Components: Test
>  Environment: All
>     Reporter: Andreas Korneliussen
>  Attachments: BasicDerbyJUnitTest.html, BasicDerbyJUnitTest.java, BasicDerbyJUnitTestTest.java,
JDBCClient.java, TestConfiguration.java
>
> The current junit tests cannot be run directly from the java.ui.textrunner by i.e using:
> java junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.junitTests.lang.BooleanTest
> .E
> Time: 0.008
> There was 1 error:
> 1) testBoolean(org.apache.derbyTesting.functionTests.tests.junitTests.lang.BooleanTest)java.lang.NullPointerException
>         at org.apache.derbyTesting.functionTests.util.DerbyJUnitTest.faultInDriver(DerbyJUnitTest.java:317)
>         at org.apache.derbyTesting.functionTests.util.DerbyJUnitTest.getConnection(DerbyJUnitTest.java:345)
>         at org.apache.derbyTesting.functionTests.util.DerbyJUnitTest.getConnection(DerbyJUnitTest.java:335)
>         at org.apache.derbyTesting.functionTests.tests.junitTests.lang.BooleanTest.testBoolean(BooleanTest.java:136)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
> FAILURES!!!
> Tests run: 1,  Failures: 0,  Errors: 1
> The reason is that the tests needs to have some fixture being set up before the test
can run, and that this is currently supported by calling a bunch of static methods in the
correct order to initialize some static members of DerbyJUnitTest.
> The proposed alternative is that the added fixture is set up in the suite() method, which
is used by JUnit to get the Test object to be run.

-- 
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


Mime
View raw message