List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:February 7 2011 12:13pm
Subject:Re: bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584
View as plain text  
Hi Luís,

Thanks for fixing this. Just one remaining comment on the assert 
condition, please see below.

Luis Soares wrote:
[...]
> === 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 15:38:59 +0000
[...]
> @@ -22,27 +27,21 @@ 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 $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]

This condition will be true either if $not_empty and $match_regexp are 
both 1, or if both are 0; the latter is not what we want. Since we check 
that the field matches the regex, we don't need an additional check to 
verify that it's non-null (because NULL does not match the regex). Also, 
please note that the full $assert_cond is evaluated in a SELECT 
statement, so there is no need to wrap things in [SELECT ...] 
sub-statements. So think the last three lines can be simplified to:

--let $assert_cond= `SELECT ("$errts0" REGEXP "$ts_regexp")`

> +--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
[...]
> @@ -80,26 +80,19 @@ START SLAVE;
>  -- source include/wait_for_slave_param.inc
>  -- echo *** must be having the replicate-same-server-id IO thread error ***
>  
> --- echo # assertion: assert that error has been reported as well as a timestamp
> --- let $errmsg0= query_get_value("SHOW SLAVE STATUS", Last_IO_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 $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]

Same as above: the last three lines can be simplified to:

--let $assert_cond= `SELECT ("$errts0" REGEXP "$ts_regexp")`

> +--let $assert_text= $field is not null and matches the expected format
> +--source include/assert.inc
>  
>  # action: stop the slave
>  -- source include/stop_slave.inc
>  
> --- let $errmsg1= query_get_value("SHOW SLAVE STATUS", Last_IO_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 $assert_cond= "$errts0" = "[SHOW SLAVE STATUS, $field, 1]"
> +--let $assert_text= $field matches the one reported before stopping slave threads
> +--source include/assert.inc
>  
>  # action: restore correct settings
>  -- replace_result $MASTER_MYPORT MASTER_PORT
> 
[...]

/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 Sandberg7 Feb
    • Re: bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584Luís Soares7 Feb
Re: bzr commit into mysql-trunk branch (luis.soares:3595) Bug#58584Luís Soares7 Feb