db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel John Debrunner <...@apache.org>
Subject JUnit common methods & assert messages WAS Re: [jira] Commented: (DERBY-827) Performance can be improved by re-using language ResultSets across Activation executions.
Date Sun, 04 Mar 2007 16:51:55 GMT
Dyre.Tjeldvoll@Sun.COM wrote:
> "Knut Anders Hatlen (JIRA)" <jira@apache.org> writes:
> 
>>     [ https://issues.apache.org/jira/browse/DERBY-827?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477237
] 
>>
>> Knut Anders Hatlen commented on DERBY-827:
>> ------------------------------------------
>>
>> Thanks for writing the test, Dyre. I think it is a valuable addition
>> to the test suite. I committed rsfromps.v1 with revision 513679.
>>
>> Some small comments:
>>
>>   - assertRow() and assertResultSet() duplicate functionality that is
>>     already implemented in JDBC.assertRowInResultSet() and
>>     JDBC.assertFullResultSet(). Perhaps they can be reused?
> 
> A couple of things about this. When I started working on this test I
> started by copying lang.GroupByExpressionTest since that seemed like a
> pretty standard lang JUnit test. I observed that it had its own
> methods for verifying that result sets. I found that a bit odd, since
> that seemed something every test has to do, so I checked what was
> available in BaseJDBCTestCase since GroupByExpressionTest inherits
> from it. When I didn't find anything I decided to use those that I had
> copied. 
> 
> I ended up making quite a few modifications to them while writing the
> test because I needed additional functionality to debug the failures I
> saw during development. 
> 
> Now, I absolutely see the value in having such utilites in a location
> where it can be shared by all tests, but do we really need to spread
> these utilities to so many places? Makes it hard for someone who isn't
> a JUnit wizard when you can't find tests that actually follow
> "established best practices"...

I think that's always an issue, finding the best method for the job. I 
don't think putting them in a single class is the best solution. JUnit 
doesn't do that, and neither does the JDK. There are a couple of ways to 
find methods:

  1) Look at the javadoc, Andrew recently made the testing javadoc be 
built nightly:
   http://db.apache.org/derby/javadoc/testing/
     Focus on the org.apache.derbyTesting.junit package

  2) Ask a question on derby-dev, allows other to help out and see the 
solution. I highly recommend this approach.

  3) Look at other tests, if a test seems to have its own method that 
you think should be a common utility, make it a common method, and/or 
ask derby-dev why the test has its own version.

And of course, once you have found the method if you can think of any 
improvement in javadoc, wiki pages etc. that would help the next person 
find the method, then make the improvement.

> Well, I tried using the asserts from JDBC, but why don't they follow
> the standard JUnit assert pattern, (assertX(String message, T
> expected, T returned))? This isn't just a style issue, in this test I
> have relied heavily on passing context information, such as loop
> indices and prepared statement parameters, down to the actual assert
> method so that it gets displayed when something fails (also borrowed
> from GroupByExpressionTest). That info will just be lost if I switch
> to JDBC.assertFullResultSet...

So improve the utility methods. The standard JUnit Assert methods have 
versions that take no message and those that take a message, so there's 
no difference here apart from no-one has scratched the itch to add a 
version with a passed in message. No-one is saying the current Derby 
junit setup is perfect, it's a work in progress, as people find needs 
they implement utility methods.

One comment on the loop indices in assert messages. This style of code 
can be expensive at runtime:
    for (int i = 0; i < count; i++) {
       assertEquals("Testing at iteration " + i, f(i), g(i));
    }

The issue is the message will be created 'count' times even though it is 
never used. This takes up cpu time and garbage collection time.

In the Derby sanity code this style was a problem, causing extended test 
running time, so the encouraged practice (for Derby sanity checks) is
   if (SanityManager.ASSERT)
   {
      if (X != Y)
         SanityManager.THROWASSERT("Value should be " + X + " is " + Y);
   }

rather than
   if (SanityManager.ASSERT)
   {
      SanityManager.ASSERT(X == Y, "Value should be " + X + " is " + Y);
   }

I'm not sure what should be the best practice for JUnit tests should be, 
but based upon past experience when I see a loop with a complex 
iteration specific assert message I worry about execution time.

In some cases the message can be fixed, e.g. this coding style should 
not be encouraged:

   assertEquals("Expected " + X + " to be the equal to " + Y, X, Y);

This is because the JUnit mechanism will print out the value of X and Y, 
so no need to repeat it in a generated message.

Dan.


Mime
View raw message