Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Derby Development" <derby-dev@db.apache.org>
Received-SPF: neutral (hermes.apache.org: local policy)
Message-ID: <42678752.3010507@sbcglobal.net>
Date: Thu, 21 Apr 2005 03:58:26 -0700
From: Kathey Marsden <kmarsdenderby@sbcglobal.net>
User-Agent: Mozilla Thunderbird 0.7.3 (Windows/20040803)
MIME-Version: 1.0
To: Derby Development <derby-dev@db.apache.org>
Subject: Re: jdbcapi/testRelative intermittent diff
References: <425C13F4.10107@sbcglobal.net> <425F8710.7030600@Sun.com>
 <425FF8A8.4090506@sbcglobal.net> <42632DC8.4000907@sun.com>
 <d9619e4a050417213114a333f9@mail.gmail.com> <42633AB4.8010209@sun.com>
 <42641DD7.2070909@sbcglobal.net> <42649A92.2070808@Sun.com>
In-Reply-To: <42649A92.2070808@Sun.com>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit


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


Your test would not print "FAIL"  if it got the wrong exception,

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;

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.

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.

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.


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