From f4be85471d4b9a555233f3cfc6ddaa9eb4b49a13 Mon Sep 17 00:00:00 2001
From: Tina Mueller <tina.mueller@suse.com>
Date: Wed, 14 Aug 2019 18:58:22 +0200
Subject: [PATCH] Improve result display of validate_script_output

See also https://progress.opensuse.org/issues/54548

Before the output showed:

    # wait_serial expected: (whatever the output was)

    # Result:
    (whatever the output was)

"wait_serial" is wrong here, and it showed the actual output twice.

Now it shows:

    validate_script_output got:
    (whatever the output was)

    Check function (deparsed code):
    {
        package your::test::package;
        /some.*regex/;
    }

* Also validate_script_output() can now take a simple regular expression qr/foo/
  instead of a coderef.

* Add explicit return statement to make current behaviour transparent
---
 cpanfile                  |  1 +
 dist/rpm/os-autoinst.spec |  2 +-
 t/03-testapi.t            |  7 +++++
 testapi.pm                | 55 ++++++++++++++++++++++++++++++---------
 4 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/cpanfile b/cpanfile
index 4b8e68592..da23f89d4 100644
--- a/cpanfile
+++ b/cpanfile
@@ -1,3 +1,4 @@
+requires 'B::Deparse';
 requires 'Carp';
 requires 'Carp::Always';
 requires 'Class::Accessor::Fast';
diff --git a/t/03-testapi.t b/t/03-testapi.t
index e893c1ddc..63aab9aca 100755
--- a/t/03-testapi.t
+++ b/t/03-testapi.t
@@ -489,6 +489,7 @@ subtest 'validate_script_output' => sub {
     my $mock_testapi = Test::MockModule->new('testapi');
     $mock_testapi->mock(script_output => sub { return 'output'; });
     ok(!validate_script_output('script', sub { m/output/ }), 'validating output with default timeout');
+    ok(!validate_script_output('script', qr/output/),        'validating output with regex and default timeout');
     ok(!validate_script_output('script', sub { m/output/ }, 30), 'specifying timeout');
     like(
         exception {
@@ -496,6 +497,12 @@ subtest 'validate_script_output' => sub {
         },
         qr/output not validating/
     );
+    like(
+        exception {
+            validate_script_output('script', ['Invalid parameter']);
+        },
+        qr/coderef or regexp/
+    );
 };
 
 subtest 'wait_still_screen' => sub {
diff --git a/testapi.pm b/testapi.pm
index 1b36c73cc..b5bdb4abc 100755
--- a/testapi.pm
+++ b/testapi.pm
@@ -33,7 +33,8 @@ use OpenQA::Isotovideo::NeedleDownloader;
 use Digest::MD5 'md5_base64';
 use Carp qw(cluck croak);
 use MIME::Base64 'decode_base64';
-use Scalar::Util 'looks_like_number';
+use Scalar::Util qw(looks_like_number reftype);
+use B::Deparse;
 
 require bmwqemu;
 use constant OPENQA_LIBPATH => '/usr/share/openqa/lib';
@@ -1117,20 +1118,23 @@ sub get_test_data {
 
 =head2 validate_script_output
 
-  validate_script_output($script, $code [, timeout => $timeout] [,quiet => $quiet])
+  validate_script_output($script, $code | $regexp [, timeout => $timeout] [,quiet => $quiet])
 
 Deprecated mode
 
   validate_script_output($script, $code, [$wait])
 
-Wrapper around script_output, that runs a callback on the output. Use it as
+Wrapper around script_output, that runs a callback on the output, or
+alternatively matches a regular expression. Use it as
 
-  validate_script_output "cat /etc/hosts", sub { m/127.*localhost/ }
+  validate_script_output "cat /etc/hosts", sub { m/127.*localhost/ };
+  validate_script_output "cat /etc/hosts", qr/127.*localhost/;
+  validate_script_output "cat /etc/hosts", sub { $_ !~ m/987.*somehost/ };
 
 =cut
 
 sub validate_script_output {
-    my ($script, $code) = splice(@_, 0, 2);
+    my ($script, $check) = splice(@_, 0, 2);
     my %args = compat_args(
         {
             timeout => 30,
@@ -1140,17 +1144,44 @@ sub validate_script_output {
     my $output = script_output($script, %args);
     my $res    = 'ok';
 
-    # set $_ so the callbacks can be simpler code
-    $_ = $output;
-    if (!$code->()) {
-        $res = 'fail';
-        bmwqemu::diag("output does not pass the code block:\n$output");
+    my $message = '';
+    if (reftype $check eq 'CODE') {
+        # set $_ so the callbacks can be simpler code
+        $_ = $output;
+        if (!$check->()) {
+            $res = 'fail';
+            bmwqemu::diag("output does not pass the code block:\n$output");
+        }
+        my $deparse = B::Deparse->new("-p");
+        # avoid "use strict; use warnings" in the output to make it shorter
+        $deparse->ambient_pragmas(warnings => [], strict => "all");
+
+        my $body = $deparse->coderef2text($check);
+
+        $message = sprintf
+          "validate_script_output got:\n%s\n\nCheck function (deparsed code):\n%s",
+          $output, $body;
+    }
+    elsif (reftype $check eq 'REGEXP') {
+        if ($output !~ $check) {
+            $res = 'fail';
+            bmwqemu::diag("output does not match the regex:\n$output");
+        }
+        $message = sprintf
+          "validate_script_output got:\n%s\n\nRegular expression:\n%s",
+          $output, $check;
+    }
+    else {
+        croak "Invalid use of validate_script_output(), second arg must be a coderef or regexp";
     }
-    # abusing the function
-    $autotest::current_test->record_serialresult($output, $res, $output) unless ($args{quiet});
+    $autotest::current_test->record_resultfile(
+        'validate_script_output', $message,
+        result => $res,
+    );
     if ($res eq 'fail') {
         croak "output not validating";
     }
+    return 0;
 }
 
 =head2 become_root