List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:February 4 2011 3:00pm
Subject:Re: bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584
View as plain text  
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

Thread
bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584Luis Soares4 Feb
  • Re: bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584Sven Sandberg4 Feb