diff --git a/1199-edited.patch b/1199-edited.patch new file mode 100644 index 0000000..5ecf2f2 --- /dev/null +++ b/1199-edited.patch @@ -0,0 +1,166 @@ +From f4be85471d4b9a555233f3cfc6ddaa9eb4b49a13 Mon Sep 17 00:00:00 2001 +From: Tina Mueller +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 diff --git a/os-autoinst.spec b/os-autoinst.spec index 6ad2fb7..bbb3341 100644 --- a/os-autoinst.spec +++ b/os-autoinst.spec @@ -35,7 +35,7 @@ Name: os-autoinst Version: %{github_version} -Release: 23%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} +Release: 24%{?github_date:.%{github_date}git%{shortcommit}}%{?dist} Summary: OS-level test automation License: GPLv2+ URL: https://os-autoinst.github.io/openQA/ @@ -44,6 +44,10 @@ Source0: https://github.com/%{github_owner}/%{github_name}/archive/%{gith # https://github.com/os-autoinst/os-autoinst/pull/1202 # Handle PXE boot only once with 'PXEBOOT=once' Patch0: 0001-Allow-special-value-once-for-PXEBOOT-to-PXE-boot.onc.patch +# https://github.com/os-autoinst/os-autoinst/pull/1199 +# Improve validate_script_output result display +# spec file block dropped as it doesn't apply and doesn't matter +Patch1: 1199-edited.patch BuildRequires: autoconf BuildRequires: automake @@ -99,7 +103,7 @@ Recommends: git # Requires: for most of them, but we'd still have to add BuildRequires, # so we may as well follow the SUSE spec's approach here to make it # easier to resync with SUSE's spec... -%define t_requires perl(Carp::Always) perl(Data::Dump) perl(Crypt::DES) perl(JSON) perl(autodie) perl(Class::Accessor::Fast) perl(Exception::Class) perl(File::Touch) perl(File::Which) perl(IPC::Run::Debug) perl(Net::DBus) perl(Net::SNMP) perl(Net::IP) perl(IPC::System::Simple) perl(Net::SSH2) perl(XML::LibXML) perl(XML::SemanticDiff) perl(JSON::XS) perl(List::MoreUtils) perl(Socket::MsgHdr) perl(Cpanel::JSON::XS) perl(IO::Scalar) +%define t_requires perl(B::Deparse) perl(Carp::Always) perl(Data::Dump) perl(Crypt::DES) perl(JSON) perl(autodie) perl(Class::Accessor::Fast) perl(Exception::Class) perl(File::Touch) perl(File::Which) perl(IPC::Run::Debug) perl(Net::DBus) perl(Net::SNMP) perl(Net::IP) perl(IPC::System::Simple) perl(Net::SSH2) perl(XML::LibXML) perl(XML::SemanticDiff) perl(JSON::XS) perl(List::MoreUtils) perl(Socket::MsgHdr) perl(Cpanel::JSON::XS) perl(IO::Scalar) # [note from openSUSE spec regarding JSON::XS dependency]: # we shuffle around a lot of JSON, so make sure this is fast # and the JSON modules have subtle differences and we only test against XS in production @@ -236,6 +240,9 @@ make check VERBOSE=1 %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.opensuse.os_autoinst.switch.conf %changelog +* Wed Aug 21 2019 Adam Williamson - 4.5-24.20190806gitc597122 +- Backport PR #1199 to improve validate_script_output result display + * Tue Aug 20 2019 Adam Williamson - 4.5-23.20190806gitc597122 - Allow PXE boot only once (-boot once=n)