| Message ID | 1470300360-4435-12-git-send-email-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Wei Liu writes ("[OSSTEST PATCH RFC 11/14] Introduce ts-xtf-run"):
> This is the main script for running XTF. It will first perform
> selftest, and then run each XTF test case as a substep.
>
> It does the following things:
>
> 1. Run self tests for individual environment and record the result.
> 2. Collect tests according to available environments.
> 3. Run the collected tests one by one.
>
> The script may exit early if it detects the test host is down or
> xtf-runner returns non-recognisable exit code.
...
> + foreach my $e (split('\n', $output)) {
> + push @all_environments, $e
> + }
Why not
push @all_environments, split /\n/, $output;
?
(NB split takes a pattern - a regexp - not a string. Your code
provides a literal string which is then used as a regexp.)
(Another occurrence of this pattern, later.)
> +# Call the runner on one test case, generate a substep for it in final test
> +# report. Return runner exit code to caller. May exit the script early if
> +# things go really bad.
> +sub do_one_test ($) {
> + my ($name) = @_;
> + my $tid = "xtf/$name";
> + my $cmd = "xtf-runner $name";
> +
> + substep_start($tid, $cmd);
> + my $output = target_cmd_output_root($ho, <<END, 600);
> + $runner $name 1>&2; echo \$\?
> +END
^ this second \ is superfluous
> + my ($ret) = $output =~ /(\d+)/;
die unless the pattern matches.
This code seems to be lacking the error handling we discussed. In
particular, if target_cmd_output_root fails (because the dom0
crashed), target_cmd_output_root will die. The script will exit
nonzero and the step will be left `running'.
> + # If the host doesn't respond after a test case, always make this substep
> + # fail and exit the script early with 0
> + my $msg = target_ping_check_up($ho);
I think it would be better to use
target_cmd_output($ho, 'echo ok.');
than target_ping_check_up.
There are some kinds of failure which leave the host responding to
ping but not actually usefully alive.
> + # If xtf result can't be recognised, always make this substep fail and exit
> + # the script early with status 0.
> + if (!xtf_result_defined($ret)) {
> + substep_finish($tid, "fail");
> + exit 0;
The lack of use of `eval' (and appropriate use of `die') has resulted
in a lot of explicit repetition of this error path.
Please arrange that all problems which ought to cause "record step as
`fail' and run no more tests" are handled by:
- having the code which detects the problem calling die
- that die being caught by a single eval instance
- the code after the eval handling all exceptions that way
Also there is a latent bug here. You have xtf_result_defined accept
exit statuses 1 and 2, but those are actually not defined exit
statuses for the xtf runner.
I think that this would be best fixed by using something like this:
+sub xtf_result_to_osstest_result ($) {
+ my ($xret) = @_;
+
+ return "pass" if $xret == 0;
+ return "skip" if $xret == 3;
+ return "fail" if $xret == 4;
+ return "fail" if $xret == 5;
+ die "xtf runner gave unexpected exit status $xret";
+}
(And, obviously, calling it within the eval.)
Then you can abolish xtf_result_defined.
And another thing: AFAICT there is nothing that prints the XTF exit
status. You need at least to report the numerical exit status;
ideally you would print some human-readable interpretation of it.
The person reading the logs may not be familiar with osstest or xtf.
They ought to be told the xtf name for the exit status as well as the
osstest mapping of it.
> +# Run selftest for each environment, record the ones that are
> +# funtional to get maximum coverage.
^c
> +sub get_tests_list () {
> + foreach my $e (sort @environments) {
> + my $output = target_cmd_output_root($ho, <<END);
> + $runner --list $e --all --host
> +END
> + foreach my $t (split('\n', $output)) {
> + push @tests, $t
> + }
It might be worth recording the environment for each test, for the
log, unless the xtf runner prints that.
Ian.
On Thu, Aug 04, 2016 at 01:19:15PM +0100, Ian Jackson wrote: > Wei Liu writes ("[OSSTEST PATCH RFC 11/14] Introduce ts-xtf-run"): > > This is the main script for running XTF. It will first perform > > selftest, and then run each XTF test case as a substep. > > > > It does the following things: > > > > 1. Run self tests for individual environment and record the result. > > 2. Collect tests according to available environments. > > 3. Run the collected tests one by one. > > > > The script may exit early if it detects the test host is down or > > xtf-runner returns non-recognisable exit code. > ... > > + foreach my $e (split('\n', $output)) { > > + push @all_environments, $e > > + } > > Why not > push @all_environments, split /\n/, $output; > ? > > (NB split takes a pattern - a regexp - not a string. Your code > provides a literal string which is then used as a regexp.) > > (Another occurrence of this pattern, later.) > Ack. > > +# Call the runner on one test case, generate a substep for it in final test > > +# report. Return runner exit code to caller. May exit the script early if > > +# things go really bad. > > +sub do_one_test ($) { > > + my ($name) = @_; > > + my $tid = "xtf/$name"; > > + my $cmd = "xtf-runner $name"; > > + > > + substep_start($tid, $cmd); > > + my $output = target_cmd_output_root($ho, <<END, 600); > > + $runner $name 1>&2; echo \$\? > > +END > ^ this second \ is superfluous > > + my ($ret) = $output =~ /(\d+)/; > > die unless the pattern matches. Ack. > > This code seems to be lacking the error handling we discussed. In > particular, if target_cmd_output_root fails (because the dom0 > crashed), target_cmd_output_root will die. The script will exit > nonzero and the step will be left `running'. > > > + # If the host doesn't respond after a test case, always make this substep > > + # fail and exit the script early with 0 > > + my $msg = target_ping_check_up($ho); > > I think it would be better to use > target_cmd_output($ho, 'echo ok.'); > than target_ping_check_up. > > There are some kinds of failure which leave the host responding to > ping but not actually usefully alive. > > > + # If xtf result can't be recognised, always make this substep fail and exit > > + # the script early with status 0. > > + if (!xtf_result_defined($ret)) { > > + substep_finish($tid, "fail"); > > + exit 0; > > The lack of use of `eval' (and appropriate use of `die') has resulted > in a lot of explicit repetition of this error path. > > Please arrange that all problems which ought to cause "record step as > `fail' and run no more tests" are handled by: > > - having the code which detects the problem calling die > - that die being caught by a single eval instance > - the code after the eval handling all exceptions that way > I will see what I can do regarding this. I'm not very familiar with perl, will need to read some docs first. > Also there is a latent bug here. You have xtf_result_defined accept > exit statuses 1 and 2, but those are actually not defined exit > statuses for the xtf runner. FAOD, 1 and 2 are defined xtf exit statuses -- reserved for anything python interpreter related. But I think this is going to be moot because they are going to be mapped to fail anyway. > > I think that this would be best fixed by using something like this: > > +sub xtf_result_to_osstest_result ($) { > + my ($xret) = @_; > + > + return "pass" if $xret == 0; > + return "skip" if $xret == 3; > + return "fail" if $xret == 4; > + return "fail" if $xret == 5; > + die "xtf runner gave unexpected exit status $xret"; > +} > > (And, obviously, calling it within the eval.) > > Then you can abolish xtf_result_defined. > > And another thing: AFAICT there is nothing that prints the XTF exit > status. You need at least to report the numerical exit status; > ideally you would print some human-readable interpretation of it. > > The person reading the logs may not be familiar with osstest or xtf. > They ought to be told the xtf name for the exit status as well as the > osstest mapping of it. > The XTF exit status is already available in the log, as is the osstest result (in substep status line). I guess you want them on the same line? One logm would do. > > +# Run selftest for each environment, record the ones that are > > +# funtional to get maximum coverage. > ^c > > > +sub get_tests_list () { > > + foreach my $e (sort @environments) { > > + my $output = target_cmd_output_root($ho, <<END); > > + $runner --list $e --all --host > > +END > > + foreach my $t (split('\n', $output)) { > > + push @tests, $t > > + } > > It might be worth recording the environment for each test, for the > log, unless the xtf runner prints that. > It is encoded in test case name. Wei. > Ian.
Wei Liu writes ("Re: [OSSTEST PATCH RFC 11/14] Introduce ts-xtf-run"):
> On Thu, Aug 04, 2016 at 01:19:15PM +0100, Ian Jackson wrote:
> > Please arrange that all problems which ought to cause "record step as
> > `fail' and run no more tests" are handled by:
> >
> > - having the code which detects the problem calling die
> > - that die being caught by a single eval instance
> > - the code after the eval handling all exceptions that way
> >
>
> I will see what I can do regarding this. I'm not very familiar with
> perl, will need to read some docs first.
Sure. I'm happy to handwave IRL or on IRC if you prefer.
If you want some full-on examples of the use of eval and indeed die,
there are:
ts-logs-capture (several smallish examples)
cs-bisection-step
Osstest::db_retry (subrefs, hairy)
Osstest::Executive::alloc_resources (one nested inside another)
> > Also there is a latent bug here. You have xtf_result_defined accept
> > exit statuses 1 and 2, but those are actually not defined exit
> > statuses for the xtf runner.
>
> FAOD, 1 and 2 are defined xtf exit statuses -- reserved for anything
> python interpreter related. But I think this is going to be moot because
> they are going to be mapped to fail anyway.
What I meant was that exit statuses 1 and 2 mean `it has all gone
horribly wrong'. They are not supposed to occur. (They may be
`defined' by the xtf runner spec, but only to reserve them.)
> The XTF exit status is already available in the log, as is the
> osstest result (in substep status line).
Oh good. Sorry for quibbling, then.
> I guess you want them on the same line? One logm would do.
But, yes, that would be nice.
> > It might be worth recording the environment for each test, for the
> > log, unless the xtf runner prints that.
>
> It is encoded in test case name.
Jolly good.
Ian.
diff --git a/ts-xtf-run b/ts-xtf-run new file mode 100755 index 0000000..596ebc4 --- /dev/null +++ b/ts-xtf-run @@ -0,0 +1,139 @@ +#!/usr/bin/perl -w +# This is part of "osstest", an automated testing framework for Xen. +# Copyright (C) 2016 Citrix Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +use strict qw(vars); +use Osstest; +use POSIX; +use Osstest::TestSupport; + +tsreadconfig(); + +our $ho = selecthost('host'); + +our $runner; +our @tests; +our @all_environments; +our @environments; + +# XTF results (runner returned numeric values) and OSStest results: +# +# SUCCESS(0) -> pass +# SKIP(3) -> skip +# ERROR(4) -> fail +# FAILURE(5) -> fail +# +# All other return codes are mapped to fail. + +sub xtf_result_to_osstest_result ($) { + my ($xret) = @_; + + return "pass" if $xret == 0; + return "skip" if $xret == 3; + return "fail"; +} + +# Check if the xtf result is within pre-defined range +# See xtf-runner --help +sub xtf_result_defined ($) { + my ($xret) = @_; + return $xret >= 0 && $xret <= 5; +} + +sub prep () { + my $xtfdir = get_runvar('xtfdir', $r{xtfbuildjob}); + $runner = "$xtfdir/xtf-runner"; +} + +sub get_environments () { + my $output = target_cmd_output_root($ho, <<END); + $runner --list --environments +END + foreach my $e (split('\n', $output)) { + push @all_environments, $e + } +} + +# Call the runner on one test case, generate a substep for it in final test +# report. Return runner exit code to caller. May exit the script early if +# things go really bad. +sub do_one_test ($) { + my ($name) = @_; + my $tid = "xtf/$name"; + my $cmd = "xtf-runner $name"; + + substep_start($tid, $cmd); + my $output = target_cmd_output_root($ho, <<END, 600); + $runner $name 1>&2; echo \$\? +END + my ($ret) = $output =~ /(\d+)/; + + # If the host doesn't respond after a test case, always make this substep + # fail and exit the script early with 0 + my $msg = target_ping_check_up($ho); + if ($msg) { + logm("$msg"); + substep_finish($tid, "fail"); + exit 0; + } + + # If xtf result can't be recognised, always make this substep fail and exit + # the script early with status 0. + if (!xtf_result_defined($ret)) { + substep_finish($tid, "fail"); + exit 0; + } + + substep_finish($tid, xtf_result_to_osstest_result($ret)); + + return $ret; +} + +# Run selftest for each environment, record the ones that are +# funtional to get maximum coverage. +sub do_selftest () { + foreach my $e (sort @all_environments) { + my $output = target_cmd_output_root($ho, <<END); + $runner --list $e selftest +END + my $ret = do_one_test($output); + push @environments, $e if $ret == 0; + } +} + +sub get_tests_list () { + foreach my $e (sort @environments) { + my $output = target_cmd_output_root($ho, <<END); + $runner --list $e --all --host +END + foreach my $t (split('\n', $output)) { + push @tests, $t + } + } +} + +sub do_all_tests () { + foreach my $t (sort @tests) { + do_one_test($t); + } +} + +prep(); +get_environments(); +do_selftest(); +get_tests_list(); +do_all_tests(); +exit 0;
This is the main script for running XTF. It will first perform selftest, and then run each XTF test case as a substep. It does the following things: 1. Run self tests for individual environment and record the result. 2. Collect tests according to available environments. 3. Run the collected tests one by one. The script may exit early if it detects the test host is down or xtf-runner returns non-recognisable exit code. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- ts-xtf-run | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100755 ts-xtf-run