Re: [for-next][PATCH 7/8] ktest.pl: Add the log of last test in email on failure

[Greg KH]

On Wed, Jul 01, 2020 at 07:17:24PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@xxxxxxxxxxx>
> 
> If a failure happens and an email is sent, show the contents of the log of
> the last test that failed in the email.
> 
> Cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> ---
>  tools/testing/ktest/ktest.pl | 44 ++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
> index e90e2e7cb72c..945a7d8c178c 100755
> --- a/tools/testing/ktest/ktest.pl
> +++ b/tools/testing/ktest/ktest.pl
> @@ -82,6 +82,8 @@ my %default = (
>      "IGNORE_UNUSED"		=> 0,
>  );
>  
> +my $test_log_start = 0;
> +
>  my $ktest_config = "ktest.conf";
>  my $version;
>  my $have_version = 0;
> @@ -1492,8 +1494,21 @@ sub dodie {
>  
>      if ($email_on_error) {
>  	my $name = get_test_name;
> +	my $log_file;
> +
> +	if (defined($opt{"LOG_FILE"})) {
> +	    $log_file = "$tmpdir/log";
> +	    open (L, "$opt{LOG_FILE}") or die "Can't open $opt{LOG_FILE} to read)";
> +	    open (O, "> $tmpdir/log") or die "Can't open $tmpdir/log\n";
> +	    seek(L, $test_log_start, 0);
> +	    while (<L>) {
> +		print O;
> +	    }
> +	    close O;
> +	    close L;
> +	}
>          send_email("KTEST: critical failure for test $i [$name]",
> -                "Your test started at $script_start_time has failed with:\n@_\n");
> +                "Your test started at $script_start_time has failed with:\n@_\n", $log_file);
>      }
>  
>      if ($monitor_cnt) {
> @@ -4185,7 +4200,7 @@ sub find_mailer {
>  }
>  
>  sub do_send_mail {
> -    my ($subject, $message) = @_;
> +    my ($subject, $message, $file) = @_;
>  
>      if (!defined($mail_path)) {
>  	# find the mailer
> @@ -4195,22 +4210,37 @@ sub do_send_mail {
>  	}
>      }
>  
> +    my $header_file = "$tmpdir/header";
> +    open (HEAD, ">$header_file") or die "Can not create $header_file\n";
> +    print HEAD "To: $mailto\n";
> +    print HEAD "Subject: $subject\n\n";
> +    print HEAD "$message\n";
> +    close HEAD;
> +
>      if (!defined($mail_command)) {
>  	if ($mailer eq "mail" || $mailer eq "mailx") {
> -	    $mail_command = "\$MAIL_PATH/\$MAILER -s \'\$SUBJECT\' \$MAILTO <<< \'\$MESSAGE\'";
> +	    $mail_command = "cat \$HEADER_FILE \$BODY_FILE | \$MAIL_PATH/\$MAILER -s \'\$SUBJECT\' \$MAILTO";
>  	} elsif ($mailer eq "sendmail" ) {
> -	    $mail_command =  "echo \'Subject: \$SUBJECT\n\n\$MESSAGE\' | \$MAIL_PATH/\$MAILER -t \$MAILTO";
> +	    $mail_command =  "cat \$HEADER_FILE \$BODY_FILE | \$MAIL_PATH/\$MAILER -t \$MAILTO";
>  	} else {
>  	    die "\nYour mailer: $mailer is not supported.\n";
>  	}
>      }
>  
> +    if (defined($file)) {
> +	$mail_command =~ s/\$BODY_FILE/$file/g;
> +    } else {
> +	$mail_command =~ s/\$BODY_FILE//g;
> +    }
> +
> +    $mail_command =~ s/\$HEADER_FILE/$header_file/g;
>      $mail_command =~ s/\$MAILER/$mailer/g;
>      $mail_command =~ s/\$MAIL_PATH/$mail_path/g;
>      $mail_command =~ s/\$MAILTO/$mailto/g;
>      $mail_command =~ s/\$SUBJECT/$subject/g;
>      $mail_command =~ s/\$MESSAGE/$message/g;
>  
> +	    print ">$mail_command<\n";
>      run_command $mail_command;
>  }
>  
> @@ -4352,6 +4382,11 @@ for (my $i = 1; $i <= $opt{"NUM_TESTS"}; $i++) {
>      }
>  
>      doprint "\n\n";
> +
> +    if (defined($opt{"LOG_FILE"})) {
> +	$test_log_start = tell(LOG);
> +    }

Nit, the mix of spaces and tabs in this file is always annoying to me,
but it's your code...

> +
>      doprint "RUNNING TEST $i of $opt{NUM_TESTS}$name with option $test_type $run_type$installme\n\n";
>  
>      if (defined($pre_test)) {
> @@ -4461,6 +4496,7 @@ if ($email_when_finished) {
>  }
>  
>  if (defined($opt{"LOG_FILE"})) {
> +
>      print "\n See $opt{LOG_FILE} for the record of results.\n\n";
>      close LOG;
>  }

Extra blank line?

Anyway, looks sane to me

Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>