Hi Luís,
Nice work, both the server patch and the test patch are good. Great that
you take the time to improve the tests.
I would suggest simplifications in two places in rpl_show_errors.test,
and there's a typo in a comment in rpl_stop_middle_group.test. See
comments inline.
On 02/04/2011 03:20 PM, Luis Soares wrote:
[...]
> === modified file 'mysql-test/extra/rpl_tests/rpl_stop_middle_group.test'
> --- a/mysql-test/extra/rpl_tests/rpl_stop_middle_group.test 2010-12-19 17:22:30
> +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_stop_middle_group.test 2011-02-04 14:20:29
> +0000
[...]
> @@ -121,20 +127,22 @@ connection slave;
[...]
> +--let $assert_text= There max value for field 'a' is 2
> +--let $assert_cond= [SELECT MAX(a) AS Val FROM tm, Val, 1] = 2
> +--source include/assert.inc
> +
> +--let $assert_text= There max value for field 'a' is 1
> +--let $assert_cond= [SELECT MAX(a) AS Val FROM ti, Val, 1] = 1
> +--source include/assert.inc
s/There/The/
(two last asserts)
> === modified file 'mysql-test/suite/rpl/t/rpl_show_errors.test'
> --- a/mysql-test/suite/rpl/t/rpl_show_errors.test 2010-12-19 17:22:30 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_show_errors.test 2011-02-04 14:20:29 +0000
[...]
> @@ -22,27 +25,26 @@ DROP TABLE t1;
> -- connection slave
> # action: now wait for the slave to stop because it cannot
> # remove a table that does not exist
> --- source include/wait_for_slave_sql_to_stop.inc
> +-- let $slave_sql_errno=1051
> +-- source include/wait_for_slave_sql_error.inc
>
> --- echo # assertion: timestamp should be filled
> --- let $errmsg0= query_get_value("SHOW SLAVE STATUS", Last_SQL_Error, 1)
> --- let $errts0= `SELECT SUBSTRING("$errmsg0" FROM 1 FOR 15)`
> -if (`SELECT (NOT STRCMP("$errts0", "") OR NOT ("$errts0" REGEXP
> "[0-9][0-9][0-9][0-9] [0-9][0-9]:[0-9][0-9]:[0-9][0-9]"))`)
> -{
> - -- echo Timestamp does not match the expected one: expected '#### ##:##:##', got:
> '$errts0'
> - -- die
> -}
> +--let $ts_regexp= [0-9][0-9][0-9][0-9] [0-9][0-9]:[0-9][0-9]:[0-9][0-9]
> +--let $errts0= query_get_value("SHOW SLAVE STATUS", $field, 1)
> +--let $match_regexp= `SELECT ("$errts0" REGEXP "$ts_regexp")`
> +--let $not_empty= `SELECT STRCMP("$errts0", "")<> 0`
> +
> +--let $assert_cond= [ SELECT $not_empty AS Not_Empty, Not_Empty, 1] = [ SELECT
> $match_regexp AS RE_Match, RE_Match, 1]
I think the above can be simplified to:
--let $assert_cond= [SHOW SLAVE STATUS, $field, 1] LIKE "$ts_regex"
(no need to check if the timestamp is '' after we have checked that it
matches the format)
> +--let $assert_text= $field is not null and matches the expected format
> +--source include/assert.inc
>
> # action: stop the slave (this should preserve the error)
> -- source include/stop_slave.inc
> --- let $errmsg1= query_get_value("SHOW SLAVE STATUS", Last_SQL_Error, 1)
> --- echo # assertion: show that error is preserved after stop slave as well as the
> timestamp
> -if (`SELECT (STRCMP("$errmsg0", "$errmsg1"))`)
> -{
> - -- let $errts1= `SELECT SUBSTRING("$errmsg1" FROM 1 FOR 15)`
> - -- echo timestamp { got: "$errts1", expected: "$errts0" }, message { got:
> "$errmsg1", expected: "$errmsg0" }
> - -- die
> -}
> +
> +--let $errts1= query_get_value("SHOW SLAVE STATUS", $field, 1)
> +--let $match_ts= `SELECT ("$errts0" REGEXP "$errts1")`
> +--let $assert_cond= [ SELECT $match_ts AS TS_Match, TS_Match, 1] = 1
I think the above can be simplified to:
--let $assert_cond= $errts0 = [SHOW SLAVE STATUS, $field, 1]
[...]
/Sven
