db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Craig Russell (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JDO-394) org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManager.test() and org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManagerForUser.test() don't close PMF correctly
Date Mon, 07 Aug 2006 17:41:14 GMT
    [ http://issues.apache.org/jira/browse/JDO-394?page=comments#action_12426274 ] 
            
Craig Russell commented on JDO-394:
-----------------------------------

Looks great. It's good to clean up these tests. Just a few comments.

1. The pattern:

             if (pmf.getOptimistic() != true) {
-                fail(ASSERTION_FAILED,
-                     "Optimistic set to true, value returned by PMF is " +
-                     pmf.getOptimistic());

hides the value that was returned by the pmf, and gets a new value for the error message.
I'd prefer something like this:

boolean optimistic = pmf.getOptimistic();
             if (optimistic != true) {
-                fail(ASSERTION_FAILED,
-                     "Optimistic set to true, value returned by PMF is " +
-                      optimistic;

2. In src/java/org/apache/jdo/tck/api/persistencemanagerfactory/GetPersistenceManagerFactoryByPropertiesInstance.java
you use the JDO_Test pmf instance but define your own pm and tx instances, which requires
you to use your own finally block to close the pm. Couldn't you use the JDO_Test pm instance
as well? Then the normal tearDown will close the pm.

3. This is really a nit, but I'd prefer the method getConfigurablePMF to be named getUnconfiguredPMF.
From the name, I was expecting perhaps a PMF that was configured with all of the properties
but was still configurable. Instead, it's simply an instance of PMF that had not been configured
at all.

4. In src/java/org/apache/jdo/tck/api/persistencemanagerfactory/GetPersistenceManagerForUser.java:
        username = props.getProperty(CONNECTION_USERNAME_PROP);
        password = props.getProperty(CONNECTION_PASSWORD_PROP);
        props.remove(CONNECTION_USERNAME_PROP);
        props.remove(CONNECTION_PASSWORD_PROP);

This could be simplified, since remove returns the present value in the Map:
        username = props.remove(CONNECTION_USERNAME_PROP);
        password = props.remove(CONNECTION_PASSWORD_PROP);

Craig

> org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManager.test() and org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManagerForUser.test()
don't close PMF correctly
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: JDO-394
>                 URL: http://issues.apache.org/jira/browse/JDO-394
>             Project: JDO
>          Issue Type: Bug
>          Components: tck20
>    Affects Versions: JDO 2 final
>            Reporter: Christian Ernst
>         Assigned To: Michael Bouschen
>            Priority: Minor
>             Fix For: JDO 2 maintenance release 1
>
>         Attachments: JDO-394-2.patch, JDO-394.patch
>
>
> org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManager.test() and 
> org.apache.jdo.tck.api.persistencemanagerfactory.GetPersistenceManagerForUser.test()

> don't close PMF correctly and this can cause other Testcases to fail 
> Following should be added to the finally block of each test()  Method
> if (pmf != null) pmf.close(); 

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