List:Commits« Previous MessageNext Message »
From:Luís Soares Date:February 19 2010 1:04pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (aelkin:3346) Bug#45576
View as plain text  
Hi Andrei,

On Fri, 2010-02-19 at 14:35 +0200, Andrei Elkin wrote:
> Luís, hello.
> 
> > SUGGESTIONS
> > -----------
> >  
> >   S1. What about replacing the beginning of the test with:
> >
> >       -- source include/master-slave-reset.inc
> >
> >       Something like:
> >
> > === modified file 'mysql-test/suite/rpl/t/rpl_row_create_table.test'
> > --- mysql-test/suite/rpl/t/rpl_row_create_table.test	2009-11-27 13:34:39
> > +0000
> > +++ mysql-test/suite/rpl/t/rpl_row_create_table.test	2010-02-19 09:39:28
> > +0000
> > @@ -136,14 +136,8 @@
> >  # BUG#22864 (Rollback following CREATE ... SELECT discards 'CREATE
> >  # table' from log):
> >  --echo ================ BUG#22864 ================
> > -connection slave;
> > -STOP SLAVE;
> > -RESET SLAVE;
> > -connection master;
> > -RESET MASTER;
> > -connection slave;
> > -START SLAVE;
> > -connection master;
> > +-- source include/master-slave-reset.inc
> > +-- connection master
> >  SET AUTOCOMMIT=0;
> >  CREATE TABLE t1 (a INT);
> >  INSERT INTO t1 VALUES (1),(2),(3);
> >
> >
> >       This way we avoid the potential risk of having tables that
> >       were created in previous tests messing with this one (after
> >       all this test checks for tables that were not created on
> >       the slave).
> 
> Good. Changed the test this way:
> http://lists.mysql.com/commits/100834

Looks good. Except that I would keep consistency among 
mysqltest commands in the same test region, eg:

+
+-- source include/master-slave-reset.inc
+
 connection master;

I would do it like:

+source include/master-slave-reset.inc;
connection master;

Anyway... your patch does the trick!

. o O (Hmm... Are there coding guidelines for tests?)

> >
> >   S2. What about replacing the SELECT I_S with SHOW CREATE TABLE
> >       for all four tables? This way we keep the test behavior as
> >       close as we can from original.
> 
> Well, that type of info does not really relate to the host of 
> that part of test - bug@22864 fixes.
> And generally I agree with you, still this w/a can bring in 
> some non-deterministic items into the result file.
> 
> 
> <andrei> luis, there can be something in the table def like auto-inc,
>          barracuda :-), ... 
> (we actually can't name it what more non-deterministic can slip in
>  show-create-table in a while).
> 
> So we're better stay of any extra:s now (as well as in the past that part
> of engine-querying should have been disqualified by reviewing).
> 

OK. Agreed.

> Thanks!

You're welcome!

Luís

Thread
bzr commit into mysql-5.1-bugteam branch (aelkin:3346) Bug#45576Andrei Elkin19 Feb
Re: bzr commit into mysql-5.1-bugteam branch (aelkin:3346) Bug#45576Luís Soares19 Feb