db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Bouschen <mbo.t...@spree.de>
Subject Re: 2nd patch implementing logging proposal
Date Wed, 24 Aug 2005 04:26:31 GMT
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/
>>> -------------------------------------------------------------------
>>>
>>
>>
>>
>
>
>------------------------------------------------------------------------
>
>Index: test/java/org/apache/jdo/tck/util/ConsoleFileOutput.java
>===================================================================
>--- test/java/org/apache/jdo/tck/util/ConsoleFileOutput.java	(revision 0)
>+++ test/java/org/apache/jdo/tck/util/ConsoleFileOutput.java	(revision 0)
>@@ -0,0 +1,80 @@
>+/*
>+ * Copyright 2005 The Apache Software Foundation.
>+ * 
>+ * Licensed under the Apache License, Version 2.0 (the "License");
>+ * you may not use this file except in compliance with the License.
>+ * You may obtain a copy of the License at 
>+ * 
>+ *     http://www.apache.org/licenses/LICENSE-2.0
>+ * 
>+ * Unless required by applicable law or agreed to in writing, software 
>+ * distributed under the License is distributed on an "AS IS" BASIS, 
>+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. 
>+ * See the License for the specific language governing permissions and 
>+ * limitations under the License.
>+ */
>+
>+package org.apache.jdo.tck.util;
>+
>+import java.io.File;
>+import java.io.FileOutputStream;
>+import java.io.IOException;
>+import java.io.OutputStream;
>+import java.io.PrintStream;
>+
>+/**
>+ * Creates an output stream that delegates to
>+ * {@link System#out} and to a file output stream.
>+ * The file name of the file output stream is determined
>+ * by method {@link BatchTestRunner#getFileName()}.
>+ */
>+class ConsoleFileOutput extends OutputStream {
>+
>+    private String fileName;
>+    private PrintStream systemOut = System.out;
>+    private FileOutputStream fileOut;
>+    
>+    ConsoleFileOutput() {
>+        this.fileName = BatchTestRunner.getFileName();
>+        try {
>+            this.fileOut = new FileOutputStream(this.fileName);
>+        } catch (IOException e) {
>+            System.err.println("Cannot create log file "+this.fileName+". "+e);
>+        }
>+    }
>+    
>+    /* 
>+     * @see java.io.OutputStream#write(int)
>+     */
>+    public void write(int b) throws IOException {
>+        this.systemOut.write(b);
>+        this.fileOut.write(b);
>+    }
>+    
>+    /**
>+     * @see java.io.OutputStream#close()
>+     */
>+    public void close()  throws IOException {
>+        this.fileOut.close();
>+        this.systemOut.close();
>+    }
>+
>+    /**
>+     * @see java.io.OutputStream#flush()
>+     */
>+    public void flush()  throws IOException {
>+        this.systemOut.flush();
>+        this.fileOut.flush();
>+    }  
>+    
>+    String getFileName() {
>+        return new File(this.fileName).getName();
>+    }
>+    
>+    String getDirectory() {
>+        String result = new File(this.fileName).getParent();
>+        if (!result.endsWith(File.separator))
>+            result += File.separator;
>+        return result;
>+    }
>+}
>
>Property changes on: test/java/org/apache/jdo/tck/util/ConsoleFileOutput.java
>___________________________________________________________________
>Name: svn:executable
>   + *
>
>Index: test/java/org/apache/jdo/tck/util/BatchResultPrinter.java
>===================================================================
>--- test/java/org/apache/jdo/tck/util/BatchResultPrinter.java	(revision 239377)
>+++ test/java/org/apache/jdo/tck/util/BatchResultPrinter.java	(working copy)
>@@ -16,13 +16,23 @@
> 
> package org.apache.jdo.tck.util;
> 
>+import java.io.File;
>+import java.io.FileInputStream;
>+import java.io.FileNotFoundException;
>+import java.io.FileOutputStream;
>+import java.io.IOException;
>+import java.io.ObjectInputStream;
>+import java.io.ObjectOutputStream;
> import java.io.PrintStream;
>+import java.io.Serializable;
> import java.text.DecimalFormat;
> import java.util.Arrays;
> import java.util.Enumeration;
> import java.util.HashMap;
> import java.util.Map;
> 
>+import javax.jdo.JDOFatalException;
>+
> import junit.framework.AssertionFailedError;
> import junit.framework.Test;
> import junit.framework.TestCase;
>@@ -38,10 +48,22 @@
> public class BatchResultPrinter
>     extends ResultPrinter
> {
>-    /** */
>+    /** The name of the TCK result file. */
>+    private static String resultFileName = "TCK-results.txt";
>+    
>+    /** The stream to delegate the output. */
>+    private ConsoleFileOutput consoleFileOutput;
>+    
>+    /** The time elapsed to run a test suite. */
>     private long runtime;
>     
>     /** */
>+    public BatchResultPrinter(ConsoleFileOutput consoleFileOutput) {
>+        this(new PrintStream(consoleFileOutput));
>+        this.consoleFileOutput = consoleFileOutput;
>+    }
>+        
>+    /** */
>     public BatchResultPrinter(PrintStream writer) {
>         super(writer);
>     }
>@@ -83,23 +105,80 @@
>         
>     /** */
>     protected void printFooter(TestResult result) {
>-        if (result.wasSuccessful()) {
>-            getWriter().print("OK");
>-            getWriter().println (" (" + result.runCount() + " test" + (result.runCount()
== 1 ? "": "s") + ")");
>-                
>+        String message = null;
>+        if (this.consoleFileOutput != null) { 
>+            message = getResultMessage(result, this.consoleFileOutput);
>+            String directory = this.consoleFileOutput.getDirectory();
>+            appendTCKResultMessage(directory, message);
>+            ResultSummary.save(directory, result);
>         } else {
>+            message = getResultMessage(result);
>+        }
>+        
>+        if (!result.wasSuccessful()) {
>             getWriter().println("FAILURES!!!");
>             printErrorSummary(result);
>-            getWriter().println("Tests run: "+result.runCount()+ 
>-                                ",  Failures: "+result.failureCount()+
>-                                ",  Errors: "+result.errorCount()+
>-                                ", Time: "+elapsedTimeAsString(this.runtime)+" seconds.");
>         }
>+        
>+        getWriter().println(message);
>         getWriter().println("Excluded tests: " + System.getProperty("jdo.tck.exclude"));
>     }
>         
>     // helper method
>+    
>+    /**
>+     * Returns the result message for the given result instance and
>+     * the given console file output instance.
>+     * @param result the result instance
>+     * @param consoleFileOutput the console file output instance
>+     * @return the result message
>+     */
>+    private String getResultMessage(TestResult result, ConsoleFileOutput consoleFileOutput)
{
>+        String message = this.consoleFileOutput.getFileName() + ':';
>+        message += System.getProperty("line.separator") + "    ";
>+        message += getResultMessage(result);
>+        return message;
>+    }
>         
>+    /**
>+     * Returns the result message for the given result instance.
>+     * @param result the result instance
>+     * @return the result message
>+     */
>+    private String getResultMessage(TestResult result) {
>+        String message;
>+        if (result.wasSuccessful()) {
>+            message = "OK (" + result.runCount() + " test" + 
>+                      (result.runCount() == 1 ? "": "s") + ")";
>+        } else {
>+            message = "Tests run: "+result.runCount()+ 
>+                      ",  Failures: "+result.failureCount()+
>+                      ",  Errors: "+result.errorCount()+
>+                      ", Time: "+elapsedTimeAsString(this.runtime)+" seconds.";
>+        }
>+        return message;
>+    }
>+        
>+    /**
>+     * Appends the given message to the TCK result file in the given directory. 
>+     * @param directory the directory
>+     * @param message the message
>+     */
>+    private static void appendTCKResultMessage(String directory, String message) {
>+        String fileName = directory + resultFileName;
>+        PrintStream resultStream = null;
>+        try {
>+            resultStream = new PrintStream(
>+                    new FileOutputStream(fileName, true));
>+            resultStream.println(message);
>+        } catch (FileNotFoundException e) {
>+            throw new JDOFatalException("Cannot create file "+fileName, e);
>+        } finally {
>+            if (resultStream != null)
>+                resultStream.close();
>+        }
>+    }
>+
>     /** 
>      * @return Name of the class of the given object without package prefix
>      */
>@@ -194,4 +273,144 @@
>             return BatchResultPrinter.getRootCause(this.t);
>         }
>     }
>+    
>+    /**
>+     * A serializable class used to store test results in a file.
>+     */
>+    public static class ResultSummary implements Serializable {
>+        private static final long serialVersionUID = 1L;
>+        private static String fileNameOfResultSummary = 
>+            "ResultSummary.ser";
>+        
>+        /**
>+         * Deserializes an instance and prints that instance to
>+         * {@link System#out} and the TCK result file.
>+         * Finally deletes the file of the serialized instance. 
>+         * @param args the first element contains the directory 
>+         * where the test result is stored.
>+         */
>+        public static void main(String[] args) {
>+            // print result summary
>+            String directory = args[0] + File.separatorChar;
>+            ResultSummary resultSummary = ResultSummary.load(directory);
>+            appendTCKResultMessage(directory, "-------");
>+            appendTCKResultMessage(directory, "Result: " + 
>+                    resultSummary.toString());
>+            System.out.println("Result: " + resultSummary);
>+            System.out.println("See file '"+ directory + resultFileName +
>+                    "' for details.");
>+            
>+            // delete file
>+            String fileName = args[0] + File.separator + 
>+                fileNameOfResultSummary;
>+            File file = new File(fileName);
>+            file.delete();
>+        }
>+        
>+        /**
>+         * Creates an instance for the given result object and
>+         * serializes that instance to a file in the geven directory.
>+         * @param directory the directory
>+         * @param result the result object
>+         */
>+        public static void save(String directory, TestResult result) {
>+            ResultSummary resultSummary = load(directory);
>+            if (resultSummary == null) {
>+                resultSummary = new ResultSummary();
>+            }
>+            resultSummary.increment(result);
>+            resultSummary.save(directory);
>+        }
>+        
>+        /**
>+         * Returns a deserialized instance stored in the given direcotry.
>+         * @param directory the directory
>+         * @return the deserialized instance
>+         */
>+        private static ResultSummary load(String directory) {
>+            ResultSummary result;
>+            String fileName = directory + fileNameOfResultSummary;
>+            ObjectInputStream ois = null;
>+            try {
>+                try {
>+                    ois = new ObjectInputStream(new FileInputStream(fileName));
>+                    result = (ResultSummary) ois.readObject();
>+                } finally {
>+                    if (ois != null) {
>+                        ois.close();
>+                    }
>+                }
>+            } catch (FileNotFoundException e) {
>+                result = null;
>+            } catch (IOException e) {
>+                throw new JDOFatalException(
>+                        "Cannot deserialize result summary in file "
>+                        +fileName, e);
>+            } catch (ClassNotFoundException e) {
>+                throw new JDOFatalException(
>+                        "Cannot deserialize result summary in file "
>+                        +fileName, e);
>+            }
>+            return result;
>+        }
>+        
>+        /**
>+         * The no argument constructor.
>+         */
>+        public ResultSummary() {}
>+        
>+        private int nrOfTotalConfigurations = 0;
>+        private int nrOfFailedConfigurations = 0;
>+        
>+        /**
>+         * Increments fields of this instance based on the given result object.
>+         * @param result the result object
>+         */
>+        private void increment(TestResult result) {
>+            this.nrOfTotalConfigurations++;
>+            if (!result.wasSuccessful()) {
>+                this.nrOfFailedConfigurations++;
>+            }
>+        }
>+        
>+        /**
>+         * Serializes this instance to a file in the given directory.
>+         * @param directory the directory
>+         */
>+        private void save(String directory) {
>+            String fileName = directory + fileNameOfResultSummary;
>+            ObjectOutputStream oos = null;
>+            try {
>+                try {
>+                    oos = new ObjectOutputStream(new FileOutputStream(fileName));
>+                    oos.writeObject(this);
>+                } finally {
>+                    if (oos != null) {
>+                        oos.close();
>+                    }
>+                }
>+            } catch (FileNotFoundException e) {
>+                throw new JDOFatalException(
>+                        "Cannot create file " + fileName, e);
>+            } catch (IOException e) {
>+                throw new JDOFatalException(
>+                        "Cannot serialize result summary to file "
>+                        + fileName, e);
>+            }
>+        }
>+        
>+        /**
>+         * @see Object#toString()
>+         */
>+        public String toString() {
>+            String result;
>+            if (this.nrOfFailedConfigurations==0) {
>+                result = "All (" + this.nrOfTotalConfigurations + ") configurations passed.";
>+            } else {
>+                result = String.valueOf(this.nrOfFailedConfigurations) + " of " +
>+                         this.nrOfTotalConfigurations + " configurations failed.";
>+            }
>+            return result;
>+        }
>+    }
> }
>Index: test/java/org/apache/jdo/tck/util/BatchTestRunner.java
>===================================================================
>--- test/java/org/apache/jdo/tck/util/BatchTestRunner.java	(revision 239377)
>+++ test/java/org/apache/jdo/tck/util/BatchTestRunner.java	(working copy)
>@@ -17,7 +17,6 @@
> package org.apache.jdo.tck.util;
> 
> import java.io.File;
>-import java.io.FileOutputStream;
> import java.io.IOException;
> import java.io.OutputStream;
> import java.io.PrintStream;
>@@ -130,13 +129,29 @@
>                 try {
>                     // get class instance
>                     Class clazz = Class.forName(className);
>-                    // constructor taking PrintStream arg
>-                    Constructor ctor = clazz.getConstructor(
>-                        new Class[] { PrintStream.class } );
>-                    // create instance
>-                    PrintStream stream = !Boolean.getBoolean("no.log.file") ?
>-                        new PrintStream(new ConsoleFileOutput()) :
>-                        System.out;
>+                    
>+                    Constructor ctor = null;
>+                    OutputStream stream = null;
>+
>+                    // choose constructor taking ConsoleFileOutput arg
>+                    if (!Boolean.getBoolean("no.log.file")) {
>+                        try {
>+                            ctor = clazz.getConstructor(
>+                            new Class[] { ConsoleFileOutput.class } );
>+                            stream = new ConsoleFileOutput();
>+                        }
>+                        catch (NoSuchMethodException ex) {
>+                            ctor = null;
>+                        }
>+                    }
>+                    
>+                    // choose constructor taking PrintStream arg
>+                    if (ctor == null) {
>+                        ctor = clazz.getConstructor(
>+                                new Class[] { PrintStream.class } );
>+                        stream = System.out;
>+                    }
>+                        
>                     return (ResultPrinter)ctor.newInstance(
>                         new Object[] { stream });
>                 }
>@@ -181,53 +196,6 @@
>     }
>     
>     /**
>-     * Creates an output stream that delegates to
>-     * {@link System#out} and to a file output stream.
>-     * The file name of the file output stream is determined
>-     * by method {@link BatchTestRunner#getFileName()}.
>-     */
>-    private static class ConsoleFileOutput extends OutputStream {
>-
>-        private String fileName;
>-        private PrintStream systemOut = System.out;
>-        private FileOutputStream fileOut;
>-        
>-        private ConsoleFileOutput() {
>-            try {
>-                this.fileName = getFileName();
>-                this.fileOut = new FileOutputStream(this.fileName);
>-            } catch (IOException e) {
>-                if (this.fileName!=null)
>-                    System.err.println("Cannot create log file "+this.fileName+". "+e);
>-            }
>-        }
>-        
>-        /* 
>-         * @see java.io.OutputStream#write(int)
>-         */
>-        public void write(int b) throws IOException {
>-            this.systemOut.write(b);
>-            this.fileOut.write(b);
>-        }
>-        
>-        /**
>-         * @see java.io.OutputStream#close()
>-         */
>-        public void close()  throws IOException {
>-            this.fileOut.close();
>-            this.systemOut.close();
>-        }
>-    
>-        /**
>-         * @see java.io.OutputStream#flush()
>-         */
>-        public void flush()  throws IOException {
>-            this.systemOut.flush();
>-            this.fileOut.flush();
>-        }        
>-    }
>-    
>-    /**
>      * Returns a file name which is determined by method
>      * {@link BatchTestRunner#changeFileName(String)}.
>      * The file name has suffix <code>.txt</code>.
>Index: maven.xml
>===================================================================
>--- maven.xml	(revision 239377)
>+++ maven.xml	(working copy)
>@@ -200,6 +200,7 @@
>     <goal name="runtck.iut" prereqs="setProps">
>         <mkdir dir="${jdo.tck.log.directory.database}"/>
>         <attainGoal name="privateRuntck.iut"/>
>+        <attainGoal name="result"/>
>     </goal>
> 
>     <goal name="privateRuntck.iut"
>@@ -236,6 +237,7 @@
>     <goal name="runtck.jdori" prereqs="setProps">
>         <mkdir dir="${jdo.tck.log.directory.database}"/>
>         <attainGoal name="privateRuntck.jdori"/>
>+        <attainGoal name="result"/>
>     </goal>
> 
>     <goal name="privateRuntck.jdori"
>@@ -363,6 +365,14 @@
>         <echo>Finished run with database="${jdo.tck.database}" identitytype="${jdo.tck.identitytype}"
mapping="${jdo.tck.mapping}".</echo>
>     </goal>
> 
>+    <goal name="result">
>+        <java fork="yes" dir="${jdo.tck.testdir}"
>+               classname="org.apache.jdo.tck.util.BatchResultPrinter$ResultSummary">
>+            <classpath refid="jdori.class.path"/>
>+            <arg line="${jdo.tck.log.directory}/${timestamp}"/>
>+        </java>
>+    </goal>
>+
>     <!-- ================== -->
>     <!-- Enhance PC classes -->
>     <!-- ================== -->
>  
>
>------------------------------------------------------------------------
>
>derby-app-alltests.txt:
>    Tests run: 396,  Failures: 17,  Errors: 23, Time: 258,703 seconds.
>derby-app-companyNoRelationships.txt:
>    OK (1 test)
>derby-app-companyEmbedded.txt:
>    OK (1 test)
>derby-app-company1-1Relationships.txt:
>    Tests run: 1,  Failures: 0,  Errors: 1, Time: 10,219 seconds.
>derby-app-company1-MRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,375 seconds.
>derby-app-companyM-MRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,469 seconds.
>derby-app-companyAllRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,985 seconds.
>derby-dsid-alltests.txt:
>    Tests run: 396,  Failures: 16,  Errors: 24, Time: 253,422 seconds.
>derby-dsid-companyNoRelationships.txt:
>    OK (1 test)
>derby-dsid-companyEmbedded.txt:
>    OK (1 test)
>derby-dsid-company1-1Relationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,562 seconds.
>derby-dsid-company1-MRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,25 seconds.
>derby-dsid-companyM-MRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 10,157 seconds.
>derby-dsid-companyAllRelationships.txt:
>    Tests run: 1,  Failures: 1,  Errors: 0, Time: 11 seconds.
>-------
>Result: 10 of 14 configurations failed.
>  
>


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


Mime
View raw message