Mailing-List: contact jdo-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jdo-dev@db.apache.org
Received-SPF: pass (asf.osuosl.org: local policy)
Message-ID: <430D578C.6030407@spree.de>
Date: Thu, 25 Aug 2005 07:30:52 +0200
From: Michael Bouschen <mbo.tech@spree.de>
Organization: Tech@Spree Engineering
User-Agent: Mozilla Thunderbird 1.0.2 (Windows/20050317)
MIME-Version: 1.0
To: jdo-dev@db.apache.org
Subject: Re: 2nd patch implementing logging proposal
References: <4309DFAB.1030103@spree.de>
 <50767.192.18.42.11.1124758135.squirrel@www.techatspree.com>
 <430B437C.60300@spree.de> <430BF6F7.10506@spree.de>
 <430C4544.4050608@spree.de>
In-Reply-To: <430C4544.4050608@spree.de>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Michael,

thanks for providing the patch. I checked in the changes.

Regards Michael

> Hi Michael,
>
> I incorporated all of your annotations below into the attached patch.
>
> Additionally, there is a change on "derby.properties": The extention 
> of derby log file is ".txt" instead of ".log". I made this change in 
> order to have the same file extention for all log files. If you feel 
> strong on a different extention for log files (e.g. ".log"), let me 
> know. Be aware that this change only takes affect if you copy the 
> properties file to directory "database/derby".
>
> I attached a sample result file for your convenience which I created 
> using this patch.
>
> This is the output of "svn status" in my local workspace:
>
> tck20>svn status
> A      test/java/org/apache/jdo/tck/util/ConsoleFileOutput.java
> M      test/java/org/apache/jdo/tck/util/BatchResultPrinter.java
> M      test/java/org/apache/jdo/tck/util/BatchTestRunner.java
> A      test/java/org/apache/jdo/tck/util/ResultSummary.java
> M      test/conf/derby.properties
> M      maven.xml
>
> Regards,
> Michael
>
>> Hi Michael,
>>
>> thanks. Some remarks:
>>
>> - I think class ResultSummary is good enough being an outer class, 
>> especially because it has its own main method that is called in a 
>> maven goal.
>> - I propose to move the static field resultFileName to ResultSummary 
>> and make it public or package protected.
>> - How about removing the no arg constructor of ResultSummary and move 
>> the instance variable to the top of the class definition.
>> - Method appendTCKResultMessage opens a new FileOutputStream for the 
>> output file. I propose method main of ResultSummary prepares the text 
>> for the output file first and then calls appendTCKResultMessage only 
>> once.
>> - Some of the static fields look like constants (e.g resultFileName, 
>> fileNameOfResultSummary). What do you think of making them final and 
>> change the name to upper case.
>>
>> Thanks for adding the sample summary file. While looking at it I 
>> thought about changing the test result a little bit. I think it is 
>> useful to add the elapsed time in case the test succeeds. So I 
>> propose the following output in case the test succeeds:
>>    OK  Tests run: 1,  Time: 10,219 seconds.
>> I also propose to add '**' in the beginning of the line in case of a 
>> failure:
>>    **  Tests run: 1,  Failures: 0,  Errors: 1, Time: 10,219 seconds.
>> What do you think?
>>
>> Regards Michael
>>
>>> Hi Michael,
>>>
>>> please find the attached patch for review implementing a summary of 
>>> the TCK results for all configurations. In addition the patch 
>>> implements a final result message saying how many configurations 
>>> passed/failed.
>>>
>>> For this purpose, I decided to make static inner class 
>>> "ConsoleFileOutput" an outer class, as this class is now used by 
>>> class "BatchResultPrinter", too.
>>>
>>> A good start for a review is method 
>>> "BatchResultPrinter.printFooter". This method is called once for 
>>> each TCK configuration. There, a final result message is printed to 
>>> a summary file for each configuration. Moreover, that method 
>>> deserializes/serializes a result instance containing the number of 
>>> passed/failed configurations. After all configurations have been 
>>> run, that instance is deserialized for a final message of 
>>> passed/failed configurations. Afterwards, the file storing that 
>>> instance is deleted.
>>>
>>> I attached a result summary file created today as an example.
>>>
>>> Regards,
>>> Michael
>>>
>>>> Hi Michael,
>>>>
>>>> thanks for the patch. I agree with all your changes and checked in the
>>>> patch as you sent it over.
>>>>
>>>> BTW, are you planning to look into the remaining part about adding 
>>>> a new
>>>> file to the logs directory including a summary of the runtck 
>>>> results for
>>>> all the configurations?
>>>>
>>>> Regards Michael
>>>>
>>>>
>>>>> Hi Michael,
>>>>>
>>>>> please find the attached patch implementing the 2nd version of the
>>>>> logging proposal for review. The changes to the first implementation
>>>>> version are:
>>>>>
>>>>> - project.properties
>>>>> -- Property "jdo.tck.util.enhancer.sources" has been removed.
>>>>>
>>>>> - maven.xml
>>>>> -- All log directories are created by maven. Before, only the 
>>>>> database
>>>>> log directory was created by maven.
>>>>> -- The enhancer goal "enhance.prepare" specifies 
>>>>> prereqs="java:compile,
>>>>> copyprops".
>>>>> -- The system property "no.log.file" is not passed to the enhancer VM
>>>>> any more.
>>>>> -- Property "jdo.tck.util.enhancer.sources" is not passed to the java
>>>>> compiler in goal "jdo.tck.util.enhancer.sources" any more.
>>>>>
>>>>> - BatcherTestRunner.java
>>>>> -- The static initiallizerhas been removed.
>>>>> -- An instance of "ConsoleFileOutput" is passed to the constructor 
>>>>> call
>>>>> of class "BatchResultPrinter". Before, "System.out" was passed.
>>>>> -- Log Directories are not created by this class any more.
>>>>> -- Methods "changeFileNameCreateDirectory have been renamed to
>>>>> "changeFileName".
>>>>> -- Methods "getSystemPropertyAsPartialFileName" have been removed.
>>>>> -- Java doc comments have been added.
>>>>>
>>>>> - TCKFileFileAppender
>>>>> -- Java doc comments have been added.
>>>>>
>>>>> 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/
>>>>> -------------------------------------------------------------------
>>>>>
[...]

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