[JUnit] Should assertSQLState throw SQLException?

[Knut Anders Hatlen <Kn...@Sun.COM>]

Currently, BaseJDBCTestCase.assertSQLState() and JDBC.assertSQLState()
basically do an assertEquals() on the SQL state. If the SQL state
doesn't match the expected value, an AssertionFailure is raised, but
the original exception is thrown away.

Should we instead do something like this
  if (!expected.equals(sqle.getSQLState())) throw sqle;
?

This way, we preserve the stack trace and the error message, not only
the SQL state.

-- 
Knut Anders

Re: [JUnit] Should assertSQLState throw SQLException?

[Knut Anders Hatlen <Kn...@Sun.COM>]

Daniel John Debrunner <dj...@apache.org> writes:

> Kristian Waagan wrote:
>
>
>> Any suggestions on how we can achieve what Knut Anders is proposing?
>> I think it is a good proposal to preserve the stack trace, the error
>> message and the SQL state.
>
> I think we want to do something like this (which requires JDK 1.4 or
> higher), I'm looking at implementing this and maybe just lose the error
> in jdk1.3.
>
>
>         } catch (AssertionFailedError e) {
>
>             // Save the SQLException
>             e.initCause(exception);
>
>             throw e;
>         }
>
> around the asserts in assertSQLState() (except for assertion that
> exception is not null)
>
> This will save the stack trace.

Sounds good to me. I don't think it's a big issue that we lose the
error in jdk1.3.

> This could be followed on by code in BaseTestCase.runBase to always
> print the exception, or to save the exception's stack trace in the newly
> added "failed" folder (see BaseTestCase.getFailedFolder()).
>
> I don't think individual tests should be trying to print stack traces,
> if we can centralize it then we will end up with consistent results.
> E.g. well formatted information about SQLExceptions including the
> chaining. Having each test print exceptions will mean endless variety as
> to how they are printed and most missing the chain of exceptions. That's
> what happens today with the existing tests.

I agree. Thank you for all your hard work on the JUnit framework!

-- 
Knut Anders

Re: [JUnit] Should assertSQLState throw SQLException?

[Daniel John Debrunner <dj...@apache.org>]

Kristian Waagan wrote:


> Any suggestions on how we can achieve what Knut Anders is proposing?
> I think it is a good proposal to preserve the stack trace, the error
> message and the SQL state.

I think we want to do something like this (which requires JDK 1.4 or
higher), I'm looking at implementing this and maybe just lose the error
in jdk1.3.


        } catch (AssertionFailedError e) {

            // Save the SQLException
            e.initCause(exception);

            throw e;
        }

around the asserts in assertSQLState() (except for assertion that
exception is not null)

This will save the stack trace.

This could be followed on by code in BaseTestCase.runBase to always
print the exception, or to save the exception's stack trace in the newly
added "failed" folder (see BaseTestCase.getFailedFolder()).

I don't think individual tests should be trying to print stack traces,
if we can centralize it then we will end up with consistent results.
E.g. well formatted information about SQLExceptions including the
chaining. Having each test print exceptions will mean endless variety as
to how they are printed and most missing the chain of exceptions. That's
what happens today with the existing tests.

Dan.

Re: [JUnit] Should assertSQLState throw SQLException?

[Kristian Waagan <Kr...@Sun.COM>]

Rick Hillegas wrote:
> Alternatively, we could print out the stack trace ourselves. The logic 
> to do this could be re-usable by a general-purpose assertEquals() 
> overload which compares two Throwables. Failing by calling 
> assertEquals() seems like the way the JUnit wants to track errors.

I think it is perfectly acceptable to let the test methods throw 
"unexpected" exceptions as they are.

Many of our tests are a little special, because the only type of checked 
exceptions that can be thrown is SQLException. Thus we catch all 
exceptions with one catch-block.

For instance I think this kind of code is okay:

public void testSomething()
         throws XException, YException {
     methodCanThrowXException();
     mayThrowYException();
     methodCanThrowXException();
     try {
         shouldThrowYException();
         fail("An YException should have been thrown");
     } catch (YException ye) {
         assertEquals("myState", ye.getYState());
     }
}

I don't want to clutter my code with try-catches for unrelated 
exceptions, there's nothing I can do anyway. And if the unimaginable 
should happen and XException is thrown, I will have the stack trace (and 
hopefully other tests) to guide further investigations.

In the code above, note that the try-catch block is made as small as 
possible.

As Knut Anders, I also feel some information is missing when the tests 
fail. A five character SQLState does not tell me very much, and often I 
have to consult the messages file to see what it means! As a Derby 
example, let's say the method shouldThrowYException intermittently 
throws an exception with SQL state XSRS7.
Basically, this is what we have:
"expected: XSRS1, actual: XSRS7"

Let's consult the message text of XSRS7 for help:
"Backup caught unexpected exception."

That's all we got, and I don't feel too happy about that.
I can guess that some kind of IO exception caused this, but I want 
Derby/JUnit to tell me so I can be sure. If it can tell me where the 
error happened as well, it would be great!
Last, if the SQL state is not as expected, the test should result in a 
failure, not an error, but still preserve the stack trace.


Any suggestions on how we can achieve what Knut Anders is proposing?
I think it is a good proposal to preserve the stack trace, the error 
message and the SQL state.



-- 
Kristian


> 
> Regards,
> -Rick
> 
> Knut Anders Hatlen wrote:
> 
>> Currently, BaseJDBCTestCase.assertSQLState() and JDBC.assertSQLState()
>> basically do an assertEquals() on the SQL state. If the SQL state
>> doesn't match the expected value, an AssertionFailure is raised, but
>> the original exception is thrown away.
>>
>> Should we instead do something like this
>>  if (!expected.equals(sqle.getSQLState())) throw sqle;
>> ?
>>
>> This way, we preserve the stack trace and the error message, not only
>> the SQL state.
>>
>>  
>>
> 

Re: [JUnit] Should assertSQLState throw SQLException?

[Rick Hillegas <Ri...@Sun.COM>]

Alternatively, we could print out the stack trace ourselves. The logic 
to do this could be re-usable by a general-purpose assertEquals() 
overload which compares two Throwables. Failing by calling 
assertEquals() seems like the way the JUnit wants to track errors.

Regards,
-Rick

Knut Anders Hatlen wrote:

>Currently, BaseJDBCTestCase.assertSQLState() and JDBC.assertSQLState()
>basically do an assertEquals() on the SQL state. If the SQL state
>doesn't match the expected value, an AssertionFailure is raised, but
>the original exception is thrown away.
>
>Should we instead do something like this
>  if (!expected.equals(sqle.getSQLState())) throw sqle;
>?
>
>This way, we preserve the stack trace and the error message, not only
>the SQL state.
>
>  
>