db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shreyas Kaushik <Shreyas.Kaus...@Sun.COM>
Subject Re: jdbcapi/testRelative intermittent diff
Date Thu, 21 Apr 2005 11:23:44 GMT
Hi Kathey,

Please see inline.

~ Shreyas

Kathey Marsden wrote:

>Shreyas Kaushik wrote:
>
>  
>
>>Hi Kathey,
>>
>>Thanks for all the help on making this test as perfect as possible.
>>Surely this would be a step in the righ direction.
>>Attached are the latest diffs that take care of your comments ( I hope
>>;-) ).
>>
>>    
>>
>
>Hi Shreyas,
>
>Well the good news is that the new test has our 5 requirements.
>
>1)   Never print  stack trace for an expected exception
>2)  Print the word PASS or EXPECTED for an expected exception.
>3) Always print the SQLState and Message for any exception. (use
>se.getMessaage() instead of just printing se)
>4) Always print a stack trace for an unexpected exception.
>5) Print the word FAIL for an unexpected exception.  (see caveat below)
>
>As for perfect,  I don't think we are in the neighborhood of perfect
>yet.  My comments would be:
>
>Instead of having this static for your expected exception, it would be
>better to have the methods that  print the exceptions take an SQLState
>parameter, which would make it easier to have more than one negative test.
>
>  static final String EXPECTED_SQL_STATE = "24000";
>  
>
Ok I agree here.

>
>Your test would not print "FAIL"  if it got the wrong exception,
>  
>
I thought the check in the dumpSQLExceptions takes care of this case 
where i check for the expected
SQLState , else I say we got an unexpected exception. Am I missing 
something obvious ?  :-(

>Also, could you tell me why these are no longer local variables?
>
>   static Connection con;
>   static ResultSet rs;
>   static PreparedStatement stmt = null;
>   static PreparedStatement pStmt = null;
>   static Statement stmt1 = null;
>   static String returnValue = null;
>  
>
This I had to change because I had two try-catch block in there.  Hence 
this would result in a compilation error
saying that "rs might not have been initialzed", hence I thought it 
would be better not to have them as local variables.

>I am willing to commit it as  a standalone test, but would not recommend
>it as a basis for a template.  Perhaps if you take on that project it
>would be better for us to identify an existing test that could be made
>into a template.  I am also happy to review any additional improvements
>to testRelative.
>  
>
Ok sure I can do this. But I'm hooked onto multiple things now. Will 
definitely send out a mail when I am ready for this.

>In general I would say you need to take much more care in coding and
>reviewing your own changes. You need to read review comments more
>carefully, get them all integrated the first time and carry that
>knowlege with you to the next change.
>  
>
Thanks, I will do this.

>I am looking forward to your "How To Analyze a Test Failure" document.
>Plese reread our email exchanges and carefully read the
>testing/README.html for information.
>  
>
I have the document ready. What I am having some problem is testing 
whether my document has the right formatting,etc by integrating
it into the Derbyb site and building the site using Forrest. It is a 
painful task doing this and the build process is ultra slow :-( .
Forrest builds everything even for a small change and it takes almost an 
hour , sometimes more to build the site :-(

thanks
Shreyas

>
>Sending        functionTests\master\testRelative.out
>Sending        functionTests\tests\jdbcapi\testRelative.java
>Transmitting file data ..
>Committed revision 164027.
>
>Thanks
>
>Kathey
>
>
>  
>
>>Kathey Marsden wrote:
>>
>>    
>>
>>>Hi   Shreyas,
>>>
>>>I think we are getting closer here, but  I think we should look at what
>>>is going on in testRelative again.
>>>essentially we have
>>>try {
>>>
>>>         positive case 1..
>>>         positive case. 2..
>>>         positive case  3...
>>>                 negative case ...
>>>   } catch (SQLException sqle)
>>>   {
>>>          dumpSQLExceptions(sqle);
>>>
>>>   }  
>>>where dumpSQLException(sqle) now looks like this.
>>>static private void dumpSQLExceptions (SQLException se) {
>>>               System.out.println("PASS -- expected exception");
>>>               while (se != null) {
>>>                     
>>>System.out.println("SQLSTATE("+se.getSQLState()+"): "+se);  // really
>>>should be se.getMessage()
>>>                       se = se.getNextException();
>>>               }
>>>       }
>>>
>>>
>>>So the goals , expanded this time a  little are
>>>1)   Never print  stack trace for an expected exception
>>>2)  Print the word PASS or EXPECTED for an expected exception.
>>>3) Always print the SQLState and Message for any exception. (use
>>>se.getMessaage() instead of just printing se)
>>>4) Always print a stack trace for an unexpected exception.
>>>5) Print the word FAIL for an unexpected exception.
>>>
>>>In our test, yes the last test is a negative case and we do expect an
>>>exception, and you took out the printStackTrace, so as long as things
>>>are running fine the output is good and the test is good.  But let's
>>>imagine that Kathey goes in next week and breaks testRelative for some
>>>reason and  one of the positive test cases failed, what would I see in
>>>my test output?  So if I went and broke this positive case:
>>>
>>>               rs.relative(2);
>>>              System.out.println("isFirst=" + rs.isFirst() + " isLast="
>>>+ rs.isLast() + " isAfterLast=" + rs.isAfterLast());
>>>              returnValue = rs.getString("name");
>>>              System.out.println("Value="+returnValue);
>>>
>>>I would see a diff, but it would say:
>>>      PASS  -- expected exception ......
>>>and I wouldn't see a trace.  So conditions would be satisfied for 1) ,
>>>2) and 3) on our list but not for 4) and 5).
>>>
>>>Really the bottom line is that we can't use the dumpSQLExceptions for
>>>both failures and expected exceptions.
>>>We are really going to have to separate out the negative exceptions.
>>>Our new  test might look something like this.
>>>
>>>try {
>>>         positive case  1...
>>>         postive case   2..
>>>         postive case   3..
>>>         }
>>>catch (SQLException sqle)
>>>   {
>>>         unexpectedException(sqle);
>>>   } 
>>>
>>>try {
>>>      negative case 1 ...
>>>   } catch SQLException(sqle)
>>>   {
>>>       // extra credit make expectedException take an SQLState and
>>>check that the exception is the right one.
>>>      expectedException(sqle)
>>>   }
>>>     try {
>>>   negative case 2
>>>   } catch (SQLException sqle)
>>>   {
>>>      expectedException(sqle)
>>>   }
>>>} 
>>>See examples the exception printing functions  in
>>>functionTests/lang/casting for
>>>   expectedException
>>>   unexpectedException
>>>   gotWrongException
>>> 
>>>
>>>Also take a look at the  statementExceptionExpected  method
>>>lang/procedure.java. Since we need a separate try/catch block for each
>>>negative case, this tests encapsolates all that.  Again I think it would
>>>be nice to have it take a parameter for the expected SQLState that it
>>>would check.
>>>
>>>
>>>For our getting started series (and honestly for everybody)  it would be
>>>terrific to to have  functional test template and then add variations of
>>>these functions into functionTests/util/TestUtil.java for folks to
>>>call.  It would add consistency, I am sure make conversion to a better
>>>harness later easier, and again make life much easier for beginners. 
>>>If you are interested, keep this task in the back of your mind.   Start
>>>by making testRelative as perfect as you possibly can.  Later maybe you
>>>can gut it for your template.
>>>Thanks
>>>
>>>Kathey
>>>
>>>
>>>
>>>
>>>   
>>>
>>>  
>>>
>>> 
>>>
>>>      
>>>
>>------------------------------------------------------------------------
>>
>>Index: /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/testRelative.java
>>===================================================================
>>--- /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/testRelative.java
(revision 161449)
>>+++ /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/testRelative.java
(working copy)
>>@@ -7,18 +7,20 @@
>>import org.apache.derby.tools.JDBCDisplayUtil;
>>
>>public class testRelative {
>>+   
>>+   static final String EXPECTED_SQL_STATE = "24000";
>>+   static Connection con;
>>+   static ResultSet rs;
>>+   static PreparedStatement stmt = null;
>>+   static PreparedStatement pStmt = null;
>>+   static Statement stmt1 = null;
>>+   static String returnValue = null;
>>+
>>   public static void main(String[] args) {
>>        test1(args);        
>>    }
>>    
>>    public static void test1(String []args) {   
>>-                Connection con;
>>-                ResultSet rs;
>>-                PreparedStatement stmt = null;
>>-                PreparedStatement pStmt = null;
>>-                Statement stmt1 = null;
>>-                String returnValue = null;
>>-
>>                System.out.println("Test testRelative starting");
>>
>>                try
>>@@ -71,7 +73,15 @@
>>   			rs.relative(-2);
>>   			returnValue = rs.getString("name");
>>   			System.out.println("Value="+returnValue);
>>+                 } catch(SQLException se) {
>>+		    unexpectedSQLException(se);
>>+                 } catch(Throwable t) {
>>+		    System.out.println("FAIL--unexpected exception: "+t.getMessage());
>>+		    t.printStackTrace(System.out);
>>+                 }
>>
>>+                 try {
>>+
>>   			rs.relative(10);
>>   			System.out.println("isFirst=" + rs.isFirst() + " isLast=" + rs.isLast() + "
isAfterLast=" + rs.isAfterLast());
>>
>>@@ -80,19 +90,36 @@
>>
>> 		} catch(SQLException sqle) {
>> 		   dumpSQLExceptions(sqle);
>>- 		   sqle.printStackTrace();
>> 		} catch(Throwable e) {
>>- 		   System.out.println("FAIL -- unexpected exception: "+e);
>>-                   e.printStackTrace();
>>+ 		   System.out.println("FAIL -- unexpected exception: "+e.getMessage());
>>+                   e.printStackTrace(System.out);
>>
>> 		}
>>      }
>>-      
>>+     
>>+      /**
>>+        * This is to print the expected Exception's details. We are here because
we got an Exception
>>+        * when we expected one, but checking to see that we got the right one.
>>+        **/
>>      static private void dumpSQLExceptions (SQLException se) {
>>-                System.out.println("FAIL -- unexpected exception");
>>+           if( se.getSQLState() != null && (se.getSQLState().equals(EXPECTED_SQL_STATE)))
{ 
>>+                System.out.println("PASS -- expected exception");
>>                while (se != null) {
>>-                        System.out.println("SQLSTATE("+se.getSQLState()+"): "+se);
>>-                        se = se.getNextException();
>>+                    System.out.println("SQLSTATE("+se.getSQLState()+"): "+se.getMessage());
>>+                    se = se.getNextException();
>>                }
>>+            } else {
>>+	        System.out.println("FAIL--Unexpected SQLException: "+se.getMessage());
>>+	        se.printStackTrace(System.out);
>>+	    }
>>        }
>>-}
>>\ No newline at end of file
>>+
>>+     /**
>>+       * We are here because we got an exception when did not expect one.
>>+       * Hence printing the message and stack trace here.
>>+       **/
>>+     static private void unexpectedSQLException(SQLException se) {
>>+	 System.out.println("FAIL -- Unexpected Exception: "+ se.getMessage());
>>+	 se.printStackTrace(System.out);
>>+     }
>>+}
>>Index: /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/master/testRelative.out
>>===================================================================
>>--- /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/master/testRelative.out
(revision 161449)
>>+++ /drivers/derby/trunk/java/testing/org/apache/derbyTesting/functionTests/master/testRelative.out
(working copy)
>>@@ -4,6 +4,5 @@
>>Value=work3
>>Value=work1
>>isFirst=false isLast=false isAfterLast=true
>>-FAIL -- unexpected exception
>>-SQLSTATE(24000): SQL Exception: Invalid cursor state - no current row.
>>-SQL Exception: Invalid cursor state - no current row.
>>+PASS -- expected exception
>>+SQLSTATE(24000): Invalid cursor state - no current row.
>> 
>>
>>    
>>
>
>
>
>  
>

Mime
View raw message