Hi, Monty! On May 04, Michael Widenius wrote:

At lp:maria/5.5

------------------------------------------------------------ revno: 3393 revision-id: monty@askmonty.org-20120504133709-w1l2t1vhvpqupu3v parent: monty@askmonty.org-20120503130041-fb9myku0uh1qybty committer: Michael Widenius <monty@askmonty.org> branch nick: maria-5.5 timestamp: Fri 2012-05-04 16:37:09 +0300 message: Added --continue-on-error to mysqltest and mysql-test-run This will contune the test case even if there was an error. This makes it easier to run a test that contains many sub tests against one engine.

First, you forgot to test the new feature. see mysqltest.test for examples

=== modified file 'client/mysqltest.cc' --- a/client/mysqltest.cc 2012-02-23 06:50:11 +0000 +++ b/client/mysqltest.cc 2012-05-04 13:37:09 +0000 @@ -1746,9 +1791,15 @@ static int diff_check(const char *diff_n my_snprintf(buf, sizeof(buf), "%s -v", diff_name);

if (!(res_file= popen(buf, "r"))) - die("popen(\"%s\", \"r\") failed", buf); + { + report_or_die("popen(\"%s\", \"r\") failed", buf); + return; + }

I doubt it makes sense to continue in this case

- /* if diff is not present, nothing will be in stdout to increment have_diff */ + /* + if diff is not present, nothing will be in stdout to increment + have_diff + */ if (fgets(buf, sizeof(buf), res_file)) have_diff= 1;

@@ -2069,9 +2120,13 @@ void check_result()

switch (compare_files(log_file.file_name(), result_file_name)) { case RESULT_OK: - break; /* ok */ + if (!error_count) + break; /* ok */ + mess= "Got errors while running test";

I wonder whether you need it. As the error goes into the result, it'll inevitably end up in the .reject file. one can get RESULT_OK only if the same error is present in the .result file. But in this case you won't increment error_count, because an error in the result file means --error directive.

+ /* Fallthrough */ case RESULT_LENGTH_MISMATCH: - mess= "Result length mismatch\n"; + if (!mess) + mess= "Result length mismatch\n"; /* Fallthrough */ case RESULT_CONTENT_MISMATCH: { @@ -3240,8 +3319,10 @@ void do_exec(struct st_command *command) log_msg("exec of '%s' failed, error: %d, status: %d, errno: %d", ds_cmd.str, error, status, errno); dynstr_free(&ds_cmd); - die("command \"%s\" failed\n\nOutput from before failure:\n%s\n", - command->first_argument, ds_res.str); + if (!opt_continue_on_error) + die("command \"%s\" failed\n\nOutput from before failure:\n%s\n", + command->first_argument, ds_res.str);

why not report_or_die ?

+ return; }

DBUG_PRINT("info", @@ -3941,12 +4030,12 @@ void read_until_delimiter(DYNAMIC_STRING No characters except \n are allowed on the same line as the command */ - die("Trailing characters found after command"); + report_or_die("Trailing characters found after command");

I think --continue-on-error should not affect syntax errors in the test script itself. They should always abort the execution. in general, you've documented the new option as "This is mostly useful when testing a storage engine and one wants to see how much of a multi-test file it can execute." which is also what BigFish wanted it to do. So, most (if not all) mysqltest failures not related to a storage engine should not be ignored.

}

if (feof(cur_file->file)) - die("End of file encountered before '%s' delimiter was found", - ds_delimiter->str); + report_or_die("End of file encountered before '%s' delimiter was found", + ds_delimiter->str);

if (match_delimiter(c, ds_delimiter->str, ds_delimiter->length)) { === modified file 'mysql-test/mysql-test-run.pl' --- a/mysql-test/mysql-test-run.pl 2012-05-03 13:00:41 +0000 +++ b/mysql-test/mysql-test-run.pl 2012-05-04 13:37:09 +0000 @@ -5805,6 +5807,11 @@ sub start_mysqltest ($) { mtr_add_arg($args, "--max-connections=%d", $opt_max_connections); }

+ if ($opt_continue_on_error) + { + mtr_add_arg($args, "--continue-on-error");

I think this should enable --force too. or, perhaps, --force should enable this and one does not need a separate --continue-on-error in mtr.

+ } + if ( $opt_embedded_server ) {

Regards, Sergei