From: Sven Sandberg
Date: November 19 2008 2:48pm
Subject: Re: bzr commit into mysql-5.1 branch (msvensson:2695) Bug#40705
List-Archive: http://lists.mysql.com/commits/59237
Message-Id: <49242741.20501@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hi Magnus,

The refactoring is good, just one small comment regarding an error text,
see comments inline.

/Sven


Magnus Svensson wrote:
> #At file:///home/msvensson/mysql/5.1-rpl/
> 
>  2695 Magnus Svensson	2008-11-14
>       Bug#40705 mtr's check-warnings fails sporadically in pushbuild
>        - Change the logic slightly for when and how errors from
>          check-warnings and check-testcase are reported. The prevoius logic
>          reporting testfailed twice and that might have caused this.
>        - Also print result code returned from mysqltest when it is "unexpected" 
> modified:
>   mysql-test/mysql-test-run.pl
> 
> === modified file 'mysql-test/mysql-test-run.pl'
> --- a/mysql-test/mysql-test-run.pl	2008-11-14 09:22:44 +0000
> +++ b/mysql-test/mysql-test-run.pl	2008-11-14 12:19:37 +0000
> @@ -2802,7 +2802,8 @@ sub check_testcase($$)
>  	{
>  	  my $report= mtr_grab_file($err_file);
>  	  $tinfo->{comment}.=
> -	    "Could not execute 'check-testcase' $mode testcase '$tname':\n";
> +	    "Could not execute 'check-testcase' $mode ".
> +	      "testcase '$tname' (res: $res):\n";

Please replace "(res: $res)" by "(check-testcase returned $res)" or
something like that.

>  	  $tinfo->{comment}.= $report;
>  
>  	  $result= 2;
> @@ -3159,19 +3160,15 @@ sub run_testcase ($) {
>  
>        my $res= $test->exit_status();
>  
> -      if ( $res == 0 )
> +      if ($res == 0 and $opt_warnings and check_warnings($tinfo) )
>        {
> -	if ( $opt_warnings and check_warnings($tinfo) )
> -	{
> -	  # Found unexpected warnings
> -	  report_failure_and_restart($tinfo);
> -	  $res= 1;
> -	}
> -	else
> -	{
> -	  mtr_report_test_passed($tinfo);
> -	}
> +	# Test case suceeded, but it has produced unexpected
> +	# warnings, continue in $res == 1
> +	$res= 1;
> +      }
>  
> +      if ( $res == 0 )
> +      {
>  	my $check_res;
>  	if ( $opt_check_testcases and
>  	     $check_res= check_testcase($tinfo, "after"))
> @@ -3187,6 +3184,7 @@ sub run_testcase ($) {
>  	    return 1;
>  	  }
>  	}
> +	mtr_report_test_passed($tinfo);
>        }
>        elsif ( $res == 62 )
>        {
> @@ -3407,7 +3405,8 @@ sub check_warnings ($) {
>        {
>  	my $report= mtr_grab_file($err_file);
>  	$tinfo->{comment}.=
> -	  "Could not execute 'check-warnings' for testcase '$tname':";
> +	  "Could not execute 'check-warnings' for ".
> +	    "testcase '$tname' (res: $res):\n";

Here too.

>  	$tinfo->{comment}.= $report;
>  
>  	$result= 2;
> @@ -3428,11 +3427,6 @@ sub check_warnings ($) {
>      return $result;
>    }
>  
> -
> -
> -
> -  return $res;
> -
>    return $res;
>  }
>  
> 
> 


-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com