[
https://issues.apache.org/jira/browse/DERBY-4249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13071403#comment-13071403
]
Kathey Marsden commented on DERBY-4249:
---------------------------------------
I agree that the test should definitely fail if the launched test method fails.
It sounds good to use assertExecJavaCmdAsExpected. It would be good to enhace
that method on failure to in the fail message to include full error output so
we get the stack trace of the failed method.
On the test itself I think it would actually be good to inline
createBasicSetup() and assertDatabase() into testBasicRecovery() method and
also add javadoc comments about the purpose of the test in order to provide
some context of what we are testing here and also reduce clutter if more
fixtures are added.
I think the reason we are seeing the checkpoint is that launchRecoveryInsert is
shutting down the database which it should not. It should just insert
/commit/insert /return. I would not even close the statement and connection
here and as mentioned before just let the errors be thrown in all cases so the
test will fail. Typically the only time in a JUnit test that we need a
try/catch is when it is a negative test and we are expecting an exception and
then there would be a fail before the catch and then an assertSQLState() or
some such to verify we got the right exception. Bottom line is for JUnit the
pass or fail state of the test should be an assertion, not the printed output.
Why does the test need to run without security manager? Security Manager
should be on unless there is a good reason for it to be off.
One nit. The class name at the top of the license header should be the full
class name.
On another note if you want to print output for debug purposes and think it
might be useful to others in the future debugging failures you can just use
println instead of System.out.println and then run with -Dderby.tests.debug=true
Thank you Siddharth for your work on this important test and framework for the
recovery tests. I'll be out for a few weeks starting Thursday. I appreciate
the community continuing to look at the patch revisions while I am out.
> Create a simple store recovery test in JUnit
> --------------------------------------------
>
> Key: DERBY-4249
> URL: https://issues.apache.org/jira/browse/DERBY-4249
> Project: Derby
> Issue Type: Improvement
> Components: Test
> Affects Versions: 10.6.1.0
> Reporter: Kathey Marsden
> Assignee: Siddharth Srivastava
> Priority: Minor
> Attachments: d4249.diff, d4249_1.diff, d4249_2.diff, d4249_3.diff
>
>
> It would be good to be able to start converting the store recovery tests or
> at least be able to write new recovery tests in JUnit. We could start by
> writing a simple recovery test just to establish the framework. The test
> should.
> - Connect, create a table, commit and shutdown the database.
> - fork a jvm, add one row, commit, add another row, exit the jvm.
> - Reconnect with the first jvm and verify that the first row is there and
> the second is not.
> I guess the main thing to decide is how to spawn the second jvm and check
> results. I tend to think the second jvm should actually execute another
> JUnit test, verify the exit code (assuming a failed test has a non-zero exit
> code) and then put the output in the fail assertion if it fails so it shows
> up in the report at the end of the Suite execution. I think we could create
> a test runner that takes a class and a specific test to run instead of the
> whole suite, so we could keep our methods consolidated in a single class for
> the test, but all pure conjecture at this point. I'll have to give it a try,
> but wanted to first see if folks thought this was a reasonable approach.
>
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
