db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig Russell <Craig.Russ...@Sun.COM>
Subject Re: JDO-131--updated maven.xml patch file
Date Mon, 12 Sep 2005 15:50:27 GMT
Hi Karan,

Just a few comments. The patch is in the right area and solves the  
problem. I'd suggest

1. Use a static protected final boolean variable, as Michael proposed.

2. Change the name to jdo.tck.cleanupaftertest (with Michael's  
default of true).

If you send a patch with these changes, Michael can check it in.

Thanks,

Craig

On Sep 11, 2005, at 2:53 PM, karan malhi wrote:

> hi,
>
> Here is the updated maven.xml.patch file. I have added the  
> sysproperty to the doRuntck.iut goal also.
>
> Michael Bouschen wrote:
>
>
>> Hi Karan Singh,,
>>
>>
>>> Hi Michael,
>>>
>>> Do you think we would need the same property for the iut tests?   
>>> The patch which i submitted earlier does not add the sysproperty  
>>> for iut tests in maven.xml.
>>>
>>
>>
>> yes, I think we need the same property for the iut tests, so the  
>> goal doRuntck.iut should add the sysproperty, too.
>>
>> Good catch!
>>
>> Regards Michael
>>
>>
>>>
>>> Michael Bouschen wrote:
>>>
>>>
>>>> Hi Karan Singh,
>>>>
>>>> nice work!
>>>>
>>>> I would like to propose three small changes:
>>>> - All tck property names start with jdo.tck, so how about  
>>>> renaming cleanup.data to jdo.tck.cleanupdata?
>>>> - I propose to specify a default (true) for the new property in  
>>>> JDO_Test. We could also call equalsIgnoreCase in the variable  
>>>> initialization, then it is tested only once:
>>>>    protected static boolean cleanupData =
>>>>        System.getProperty("jdo.tck.cleanupdata",  
>>>> "true").equalsIgnoreCase("true");
>>>> - It looks like your IDE uses tabs for indentation (see the line  
>>>> calling equlasIgnoreCase). Using blanks would have the advantage  
>>>> that it looks the same in all the editors and IDEs.
>>>>
>>>> What do you think?
>>>>
>>>> Regards Michael
>>>>
>>>>
>>>>> Hi,
>>>>>
>>>>> I am submitting the patch files for review for JIRA issue 131.
>>>>>
>>>>> ------------------------------------------------------------------ 
>>>>> ------
>>>>>
>>>>> Index: C:/ApacheJDO/trunk/tck20/test/java/org/apache/jdo/tck/ 
>>>>> JDO_Test.java
>>>>> ================================================================== 
>>>>> =
>>>>> --- C:/ApacheJDO/trunk/tck20/test/java/org/apache/jdo/tck/ 
>>>>> JDO_Test.java    (revision 279926)
>>>>> +++ C:/ApacheJDO/trunk/tck20/test/java/org/apache/jdo/tck/ 
>>>>> JDO_Test.java    (working copy)
>>>>> @@ -137,6 +137,11 @@
>>>>>     /** Name of the file contaninig the properties for the PMF. */
>>>>>     protected static String PMFProperties = System.getProperty 
>>>>> ("PMFProperties");
>>>>>
>>>>> +    /** String indicating whether to clean up data after tests  
>>>>> or not. The value can be either
>>>>> +     * "true" or "false". If "false" then test will not clean  
>>>>> up data from database
>>>>> +     */
>>>>> +    protected static String cleanupData = System.getProperty 
>>>>> ("cleanup.data");
>>>>> +        /** The Properties object for the  
>>>>> PersistenceManagerFactory. */
>>>>>     protected static Properties PMFPropertiesObject;
>>>>>
>>>>> @@ -260,7 +265,9 @@
>>>>>             pmf = null;
>>>>>                 try {
>>>>> -            localTearDown();
>>>>> +            if (cleanupData.equalsIgnoreCase("true")) {
>>>>> +                localTearDown();
>>>>> +            }
>>>>>         }         catch (Throwable t) {
>>>>>             setTearDownThrowable("localTearDown", t);
>>>>>
>>>>> ------------------------------------------------------------------ 
>>>>> ------
>>>>>
>>>>> Index: C:/ApacheJDO/trunk/tck20/maven.xml
>>>>> ================================================================== 
>>>>> =
>>>>> --- C:/ApacheJDO/trunk/tck20/maven.xml    (revision 279926)
>>>>> +++ C:/ApacheJDO/trunk/tck20/maven.xml    (working copy)
>>>>> @@ -361,6 +361,8 @@
>>>>>                          value="${jdo.tck.exclude}"/>
>>>>>             <sysproperty key="jdo.tck.log.directory"
>>>>>                          value="${jdo.tck.log.directory}/$ 
>>>>> {timestamp}"/>
>>>>> +            <sysproperty key="cleanup.data"
>>>>> +                         value="${cleanup.data}"/ 
>>>>> >               <jvmarg line="${database.runtck.sysproperties}"/>
>>>>>             <jvmarg line="${jdo.runtck.sysproperties}"/>
>>>>>             <arg line="${jdo.tck.classes}"/>
>>>>>
>>>>> ------------------------------------------------------------------ 
>>>>> ------
>>>>>
>>>>> Index: C:/ApacheJDO/trunk/tck20/project.properties
>>>>> ================================================================== 
>>>>> =
>>>>> --- C:/ApacheJDO/trunk/tck20/project.properties    (revision  
>>>>> 279926)
>>>>> +++ C:/ApacheJDO/trunk/tck20/project.properties    (working copy)
>>>>> @@ -42,7 +42,8 @@
>>>>> maven.junit.dir = ${jdo.tck.testdir}
>>>>> maven.junit.sysproperties = PMFProperties
>>>>> PMFProperties = jdori.properties
>>>>> -
>>>>> +# Setting this property to false will turn off cleanup of data  
>>>>> from database to inspect database contents after test run
>>>>> +cleanup.data = true
>>>>> # JDO TCK settings
>>>>> jdo.tck.dblist=derby
>>>>> jdo.tck.identitytypes=applicationidentity datastoreidentity
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
> -- 
> Karan Singh
>
> Index: C:/ApacheJDO/trunk/tck20/maven.xml
> ===================================================================
> --- C:/ApacheJDO/trunk/tck20/maven.xml    (revision 279926)
> +++ C:/ApacheJDO/trunk/tck20/maven.xml    (working copy)
> @@ -314,6 +314,8 @@
>                           value="${jdo.tck.exclude}"/>
>              <sysproperty key="jdo.tck.log.directory"
>                           value="${jdo.tck.log.directory}/$ 
> {timestamp}"/>
> +             <sysproperty key="jdo.tck.cleanupData"
> +                         value="${jdo.tck.cleanupData}"/>
>              <jvmarg line="${database.runtck.sysproperties}"/>
>              <jvmarg line="${iut.runtck.sysproperties}"/>
>              <arg line="${jdo.tck.classes}"/>
> @@ -361,6 +363,8 @@
>                           value="${jdo.tck.exclude}"/>
>              <sysproperty key="jdo.tck.log.directory"
>                           value="${jdo.tck.log.directory}/$ 
> {timestamp}"/>
> +            <sysproperty key="jdo.tck.cleanupData"
> +                         value="${jdo.tck.cleanupData}"/>
>              <jvmarg line="${database.runtck.sysproperties}"/>
>              <jvmarg line="${jdo.runtck.sysproperties}"/>
>              <arg line="${jdo.tck.classes}"/>
>

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!


Mime
View raw message