Hi, Magnus.
Thank for review.
I've updated the patch by your notes with some small changes.
http://lists.mysql.com/commits/58701
Magnus Svensson wrote:
> Serge Kozlov wrote:
>> #At file:///home/ksm/sun/repo/bug39861/mysql-5.1-rpl/
>>
>> 2692 Serge Kozlov 2008-11-12
>> Bug#39861:
>> 1. mysqltest.cc - added flush to log file after each executed
>> command in a test case.
>
> magnus: ^ no mysqltest.cc change in the patch? Did you forget to add it?
>
>
>> 2. mtr shows 20 last lines from test case log file if timeout
>> reached.
>> modified:
>> mysql-test/lib/mtr_cases.pm
>> mysql-test/lib/mtr_io.pl
>> mysql-test/mysql-test-run.pl
>>
>> === modified file 'mysql-test/lib/mtr_cases.pm'
>> --- a/mysql-test/lib/mtr_cases.pm 2008-10-23 19:27:09 +0000
>> +++ b/mysql-test/lib/mtr_cases.pm 2008-11-12 20:02:34 +0000
>> @@ -688,6 +688,7 @@ sub collect_one_test_case {
>> my $tinfo= My::Test->new
>> (
>> name => "$suitename.$tname",
>> + shortname => $tname,
>> path => "$testdir/$filename",
>>
>> );
>>
>> === modified file 'mysql-test/lib/mtr_io.pl'
>> --- a/mysql-test/lib/mtr_io.pl 2008-08-09 09:16:12 +0000
>> +++ b/mysql-test/lib/mtr_io.pl 2008-11-12 20:02:34 +0000
>> @@ -26,7 +26,7 @@ sub mtr_tonewfile($@);
>> sub mtr_appendfile_to_file ($$);
>> sub mtr_grab_file($);
>> sub mtr_printfile($);
>> -sub mtr_lastlinefromfile ($);
>> +sub mtr_lastlinesfromfile ($$);
>>
>> # Read a whole file, stripping leading and trailing whitespace.
>> sub mtr_fromfile ($) {
>> @@ -94,16 +94,22 @@ sub mtr_printfile($) {
>> return;
>> }
>>
>> -sub mtr_lastlinefromfile ($) {
>> +sub mtr_lastlinesfromfile ($$) {
>> my $file= shift;
>> + my $num= shift;
>
> magnus: change variable name to $num_lines and either check that 2
> arguments are passed to the function
>
> croak "usage: mtr_lastlinesfromfile(file, numlines)" unless (@_ == 1);
> my ($file, $num_lines) = @_;
>
>> my $text;
>> -
>> open(FILE,"<",$file) or mtr_error("can't open file \"$file\": $!");
>> - while (my $line= <FILE>)
>> + my @lines= <FILE>;
>> + close FILE;
>
> magnus: ok, you read the whole file reversed into and array.
>
> Then... return a join of the first £num_lines? Like this:
>
> my @lines= reverse <FILE>;
> return join('', splice($lines, 0, $num_lines));
>
>
>
>> + my $size= scalar(@lines);
>> + if ($size < $num) {
>> - $text= $line;
>> + $num= $size;
>> + }
>> + for (my $i= ($size-$num); $i < ($size); $i++) + {
>> + $text .= $lines[$i];
>> }
>> - close FILE;
>> return $text;
>> }
>>
>>
>> === modified file 'mysql-test/mysql-test-run.pl'
>> --- a/mysql-test/mysql-test-run.pl 2008-11-04 15:25:37 +0000
>> +++ b/mysql-test/mysql-test-run.pl 2008-11-12 20:02:34 +0000
>> @@ -3245,8 +3245,15 @@ sub run_testcase ($) {
>> {
>> $tinfo->{comment}=
>> "Test case timeout after $opt_testcase_timeout minute(s)\n\n";
>
> magnus:
>
> 1. create a variable with the name of the file, so we only need ot do it
> once.
>
> my $log_file_name= $opt_vardir."/log/".$tinfo->{shortname}.".log";
>
> Then use that variable throughout the rest of the function.
>
>> + if (-e $opt_vardir."/log/".$tinfo->{shortname}.".log")
>> + {
>> + $tinfo->{comment}.=
>> + " == ".$opt_vardir."/log/".$tinfo->{shortname}.".log ==
>> \n" .
>
> magnus: thus this ^ line becomes
> " == $log_file_name ==\n";
>
> which is far more readable. ;)
>
>
>> +
>> mtr_lastlinesfromfile($opt_vardir."/log/".$tinfo->{shortname}.".log",20)."\n";
>
>>
>> + }
>> $tinfo->{'timeout'}= $opt_testcase_timeout; # Mark as timeout
>> run_on_all($tinfo, 'analyze-timeout');
>> + report_failure_and_restart($tinfo);
>> return 1;
>> }
>>
>>
>
--
Serge Kozlov, QA Developer
Sun Microsystems, Inc. www.sun.com
Office:
Are you MySQL certified? www.mysql.com/certification
