On 2/12/07, Daniel John Debrunner (JIRA) <[EMAIL PROTECTED]> wrote:

   [ 
https://issues.apache.org/jira/browse/DERBY-2299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472525
 ]

The use of assertEquals in the converted test has the arguments the wrong way 
around.

               assertEquals(1956, cursor.getInt(1));
               assertEquals("hello world", cursor.getString(2).trim());

http://www.junit.org/junit/javadoc/3.8.1/junit/framework/Assert.html

static void     assertEquals(int expected, int actual)


This comment confused me. It seems to me that in the above examples
the expected int *is* the first parameter, and the actual *is* the
second, so those are correct....Or am I missing something there?

I did see one instance of assertEquals in the test where the expected
and actual appeared reversed:
   assertEquals(cursor.getCursorName(),"ForCoverageSake");


Then, To reply to Kathey's question:
I wonder whether we should test the different states in the test or
change the >>client SQLState to match embedded.

The documentation clearly states that differences in SQLState are
expected. 
(http://db.apache.org/derby/docs/10.2/adminguide/cadminappsclientdiffs.html).
Changing the SQLState to match embedded may be what we ought to do in
the bigger scheme of things, but I think it goes completely out of the
scope for this JIRA. You could log a separate bug for it. There may
already be a bug existing (I have not searched).

Finally, I have two nits:
- the file was committed(revision 506709) but no cross-reference was
done to the JIRA. I think it would be nice if that's done. I know not
everyone does this all the time, though.
- the new file has a mix of spaces and tabs. While I find that
acceptable in old code I think we should attempt a clean setup in new
code.

But - like I said, those are just nits.

Myrna

Reply via email to