Apache JDO Project

Author

JDO-58

Michael Watzek

2005-06-06, 7:49 am

Hi,

there are 5 tests (AfterCloseGetPMThrowsException,
AfterCloseSetMethodsThrowException, Close,
CloseFailsIfTransactionActive,
CloseWithoutPermissionThrowsSecurityExce
ption) that call "getPMF()" and
"pmf.close()" in their "testXXX" methods , but they do not nullify field
"pmf". All of those tests fail in "localTearDown": "localTearDown" calls
"getPMF()" which returns field "pmf" if it is not null. For this reason,
"getPMF()" returnes a closed PMF in those tests.

The proposal for a fix is to add a check before "localTearDown" is called:

if (pmf!=null && pmf.isClosed())
pmf = null;

Regards,
Michael
--
-------------------------------------------------------------------
Michael Watzek Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de Buelowstr. 66
Tel.: ++49/30/235 520 36 10783 Berlin - Germany
Fax.: ++49/30/217 520 12 http://www.spree.de/
-------------------------------------------------------------------

Craig Russell

2005-06-06, 7:49 am

Michelle Caisse

2005-06-06, 7:49 am

Alternatively, getPMF() could check to see if pmf is closed or null and
return a new pmf in either case.

-- Michelle

Michael Watzek wrote:

> Hi,
>
> there are 5 tests (AfterCloseGetPMThrowsException,
> AfterCloseSetMethodsThrowException, Close,
> CloseFailsIfTransactionActive,
> CloseWithoutPermissionThrowsSecurityExce
ption) that call "getPMF()"
> and "pmf.close()" in their "testXXX" methods , but they do not nullify
> field "pmf". All of those tests fail in "localTearDown":
> "localTearDown" calls "getPMF()" which returns field "pmf" if it is
> not null. For this reason, "getPMF()" returnes a closed PMF in those
> tests.
>
> The proposal for a fix is to add a check before "localTearDown" is
> called:
>
> if (pmf!=null && pmf.isClosed())
> pmf = null;
>
> Regards,
> Michael

Craig Russell

2005-06-06, 7:49 am

Michael Bouschen

2005-06-06, 7:50 am

Hi Michelle,

Michael W. and I discussed this as an alternative. The issue is that
method getPMF might be called by the test method with the expectation to
return "the" pmf instance. E.g. a test class might want to check whether
the pmf is closed and calls getPMF().isClosed(). But getPMF returns a
new pmf instance which is open and the test fails.

We thought we better do not change the behavior of getPMF, but fix the
cleanup code in tearDown.

Regards Michael

> Alternatively, getPMF() could check to see if pmf is closed or null and
> return a new pmf in either case.
>
> -- Michelle
>
> Michael Watzek wrote:
>
>
>

--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin

Michelle Caisse

2005-06-06, 7:50 am

Hi, Michael,

Okay, sounds good to me.

-- Michelle

Michael Bouschen wrote:

> Hi Michelle,
>
> Michael W. and I discussed this as an alternative. The issue is that
> method getPMF might be called by the test method with the expectation
> to return "the" pmf instance. E.g. a test class might want to check
> whether the pmf is closed and calls getPMF().isClosed(). But getPMF
> returns a new pmf instance which is open and the test fails.
>
> We thought we better do not change the behavior of getPMF, but fix the
> cleanup code in tearDown.
>
> Regards Michael
>
>
>

Michael Watzek

2005-06-06, 7:50 am

Hi Craig,

> Hi Michelle,
>
> We should look at the cases for pmf being null. The tearDown method
> relies on the ability to get access to a PM in order to do cleanup. It
> doesn't make sense to go through the expensive process of getting a PMF
> if there is no tearDown work to do.
>
> So perhaps we need to check first for any instances or classes to be
> removed before we get a PMF in tearDown...
I agree!

> And we should probably disallow pmf == null in cases where there is work
> to do.
I'm not sure. Do you mean that the test result is ERROR for test cases
having "pmf == null in cases where there is work to do"? Let me know if
you feel strong on this.

By then I'll prepare a patch that includes your issue above and the
proposal below.

Regards,
Michael

>
> Craig
>
> On May 31, 2005, at 11:06 AM, Michelle Caisse wrote:
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!

--
-------------------------------------------------------------------
Michael Watzek Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de Buelowstr. 66
Tel.: ++49/30/235 520 36 10783 Berlin - Germany
Fax.: ++49/30/217 520 12 http://www.spree.de/
-------------------------------------------------------------------

Craig Russell

2005-06-06, 7:50 am

Michael Watzek

2005-06-06, 7:50 am

Hi Craig,

>
>
> I think so. Since this is a new feature, any test case that uses
> tearDown methods should leave the PMF open. Can you think of a case
> where this is not true?
No. My question is: Do we want to *force* test cases to leave the PMF
open when they add tear down instances and/or classes?

Regards,
Michael

Craig Russell

2005-06-06, 7:50 am

Michael Watzek

2005-06-06, 7:50 am

Hi Craig,

> Hi Michael,
>
> On Jun 1, 2005, at 9:30 AM, Michael Watzek wrote:
>
>
>
> Yes. Unless we can think of cases where the test method wants instances
> removed but needs to close the PMF for some reason. I think in these
> cases we can let the test method clean itself.
Ok. What do you think of adding a check in tearDown (before the other
check):

if ((pmf == null || pmf.isClosed()) &&
(this.tearDownInstances.size() > 0 || this.tearDownClasses.size() > 0))
throw new JDOFatalException ("PMF must not be nullified or closed
when tear down instances and /or classes have been added.");

Michael
--
-------------------------------------------------------------------
Michael Watzek Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de Buelowstr. 66
Tel.: ++49/30/235 520 36 10783 Berlin - Germany
Fax.: ++49/30/217 520 12 http://www.spree.de/
-------------------------------------------------------------------

Craig Russell

2005-06-06, 7:50 am