| Craig Russell (JIRA) 2006-08-07, 1:11 pm |
| [ http://issues.apache.org/jira/brows...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/ GetPersistenceManagerFactoryByProperties
Instance.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/secur...nistrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
|