From: Magnus Svensson
Date: November 13 2008 7:27pm
Subject: Re: bzr commit into mysql-5.1 branch (skozlov:2692) Bug#39861
List-Archive: http://lists.mysql.com/commits/58692
Message-Id: <491C7F86.6020201@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT

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;
>      }
> 
>