DerbyJUnitTest has many cases where the SQLException thrown by Derby
(through JDBC) is ignored. To me, this seems very bad practice for test
code.

For example:

        //
        // Swallow uninteresting exceptions when disposing of jdbc objects.
        //
        protected       static  void    close( ResultSet rs )
        {
                try {
                        if ( rs != null ) { rs.close(); }
                }
                catch (SQLException e) {}
        }       

If ResultSet.close is throwing an exception when being closed then to my
mind that's a bug, but we would never see it through a Junit test.

And for dropping schema objects:

        protected       static  void    dropSchemaObject( Connection conn, 
String genus,
String objectName )
        {
                PreparedStatement       ps = null;
                
                try {
                        ps = prepare( conn, "drop " + genus + " " + objectName 
);

                        ps.execute();
                }
                catch (SQLException e) {}

                close( ps );
        }

If this code is intended to handle the object already being dropped then
an explict SQLState check would be preferred to this blanket ignoring of
 exceptions. Again, there is the potential for valid bugs to be hidden
by this ignoring of exceptions.

I think it would be good to significantly clean up this utility class
before it gets used by too many tests. Very few of the methods have
useful Javadoc indicating their intended purpose, I'm especially
confused by the calls that convert returned values to Object, there seem
to be two approaches, convert using JDBC Type id and convert using
expected object type, not sure when I would use one or the other. And
related to that, SMALLINT is typically converted to a Short whereas JDBC
 uses a Integer. Not sure if the difference is intentional or not.

Dan.

Reply via email to