Re: bk commit into 5.0 tree (msvensson:1.2523) BUG#31741

[Magnus Svensson]

tor 2007-10-25 klockan 21:45 +0200 skrev Guilhem Bichot:
> Hello Magnus,
> 
> On Mon, Oct 22, 2007 at 10:56:29AM +0200, msvensson@stripped wrote:
> > Below is the list of changes that have just been committed into a local
> > 5.0 repository of msvensson. When msvensson does a push these changes will
> > be propagated to the main repository and, within 24 hours after the
> > push, to the public repository.
> > For information on how to access the public repository
> > see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> > 
> > ChangeSet@stripped, 2007-10-22 10:56:26+02:00, msvensson@stripped +10 -0
> >   Bug#31741 mysqltest - deprecate "system" command
> 
> As agreed, there are many more tests in 5.1 than in 5.0 which use
> "system"; some are hidden in extra/, others in suite/, a grep -r will
> find them all :) and I can review the 5.1 patch.

yepp, it's good that you did these first so we can agree on a way of
fixing them.

> 
> > --- a/mysql-test/t/mysqltest.test	2007-08-13 15:46:10 +02:00
> > +++ b/mysql-test/t/mysqltest.test	2007-10-22 10:56:23 +02:00
> > @@ -942,21 +942,8 @@ echo $d;
> >  # ----------------------------------------------------------------------------
> >  # Test system
> >  # ----------------------------------------------------------------------------
> > -#system ls > /dev/null;
> > -system echo "hej" > /dev/null;
> > -#--system ls > /dev/null
> > ---system echo "hej" > /dev/null;
> > -
> > ---error 1
> > ---exec echo "system;" | $MYSQL_TEST 2>&1
> > ---error 1
> > ---exec echo "system $NONEXISTSINFVAREABLI;" | $MYSQL_TEST 2>&1
> > ---error 1
> > ---exec echo "system false;" | $MYSQL_TEST 2>&1
> > -
> > ---disable_abort_on_error
> > -system NonExistsinfComamdn 2> /dev/null;
> > ---enable_abort_on_error
> > +# This command is deprecated and will always return an error message
> > +# system echo "hej" > /dev/null;
> 
> why leave the "system" line, if it's commented out?
> Is there a way to test that we get "deprecated" and mysqltest dies?
> If not, I propose to remove the two # lines.

Yes, I can  add a test to make sure it is deprecated.

> 
> > diff -Nrup a/mysql-test/t/ndb_autodiscover.test
> b/mysql-test/t/ndb_autodiscover.test
> > --- a/mysql-test/t/ndb_autodiscover.test	2006-01-24 08:30:48 +01:00
> > +++ b/mysql-test/t/ndb_autodiscover.test	2007-10-22 10:56:23 +02:00
> 
> > @@ -85,7 +85,10 @@ show status like 'handler_discover%';
> >  flush tables;
> >  
> >  # Modify the frm file on disk
> > -system echo "blaj" >> $MYSQLTEST_VARDIR/master-data/test/t2.frm ;
> > +remove_file $MYSQLTEST_VARDIR/master-data/test/t2.frm;
> > +write_file $MYSQLTEST_VARDIR/master-data/test/t2.frm;
> > +blaj
> > +EOF
> 
> the system used >> so appended to the file.
> The new commands are rather equivalent to > (remove before appending),
> Probably you can just remove the remove_file.

That is correct! I'll take out the "remove_file" and use "append_file"
instead of "write_file"

> 
> > @@ -200,7 +203,7 @@ insert into t4 values (1, "Automatic");
> >  select * from t4;
> >  
> >  # Remove the table from NDB
> > -system exec $NDB_TOOLS_DIR/ndb_drop_table --no-defaults -d test t4 >>
> $NDB_TOOLS_OUTPUT ; 
> > +--exec $NDB_TOOLS_DIR/ndb_drop_table --no-defaults -d test t4 >>
> $NDB_TOOLS_OUTPUT ; 
> 
> "remove_file" has no --, "--exec" has...
> I don't care, just a note.

As you probably know a -- command is always terminated by newline("\n").
Otherwise a command is terminated by current delimiter, default ";"

That means the "--exec <command>;" will actually execute the command
including the semicolon. Which is kind of a bug. I'll change it to use
no -- at all.

There is actually a gotcha in using "--<mysqltest command>". Since it's
interpreted as a "comment in command", we can't detect if it's
misspelled. Oouch, we just think it's a comment. But, a line will be
added to the <testname>.warning  file. I have some idea to totally
disallow -- as the start of a comment. Some people argue that since it's
a legal SQL comment - we should allow it. But I would rather dissallow
it as a _comment_ and only treat it as a "command ended by newline".

What do you think? Remeber this is not SQL, it's mysqltest language. ;)

Best regards
Magnus



-- 
Magnus Svensson
Senior Software Engineer
msvensson@stripped
[phone]

Attachment: [application/pgp-signature] Detta =?ISO-8859-1?Q?=E4r?= en digitalt signerad
	meddelandedel signature.asc

Attachment: [application/pgp-signature]