[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140310121300.485d2202@gandalf.local.home>
Date: Mon, 10 Mar 2014 12:13:00 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Satoru Takeuchi <satoru.takeuchi@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] ktest: remove the misleading $buildonly and
introduce $laststep.
On Sun, 09 Mar 2014 23:36:49 +0900
Satoru Takeuchi <satoru.takeuchi@...il.com> wrote:
> From: Satoru Takeuchi <satoru.takeuchi@...il.com>
>
> Each test of ktest consists of the following steps.
>
> build -> install -> boot -> run user defined tests.
>
> $buildonly means not whether the test is build onlyor not. Actually
> this variable mean the last step of the test as follows.
>
> 0: boot or more
> 1: build
> 2: install
>
> AS you can see, these are random numeric literals. In addition,
> there is no explanation about them.
>
> To improve readability, introduce $laststep instead of $buildonly.
> This variable means the last step of the test as follows.
>
> STEP_BUILD (=0): build
> STEP_INSTALL (=1): install
> STEP_BOOT_OR_MORE (=2): boot or more
Nice clean up. But there's some bugs in this patch.
> @@ -649,26 +651,20 @@ sub set_value {
>
> my $prvalue = process_variables($rvalue);
>
> - if ($buildonly && $lvalue =~ /^TEST_TYPE(\[.*\])?$/ && $prvalue ne "build") {
> - # Note if a test is something other than build, then we
> - # will need other manditory options.
> - if ($prvalue ne "install") {
> - # for bisect, we need to check BISECT_TYPE
> - if ($prvalue ne "bisect") {
> - $buildonly = 0;
When prvalue ne "bisect" we set it to boot or more.
> + if ($laststep <= STEP_INSTALL) {
> + if ($lvalue =~ /^TEST_TYPE(\[.*\])?$/ && $prvalue ne "build") {
> + # Note if a test is something other than build, then we
> + # will need other manditory options.
> + if ($prvalue eq "install") {
> + # install still limits some manditory options.
> + $laststep = STEP_INSTALL;
> + } elsif ($prvalue ne "bisect") {
> + # for bisect, we need to check BISECT_TYPE
> + $laststep = STEP_BUILD;
Here you set it back to build.
> }
> - } else {
> - # install still limits some manditory options.
> - $buildonly = 2;
> - }
> - }
> -
> - if ($buildonly && $lvalue =~ /^BISECT_TYPE(\[.*\])?$/ && $prvalue ne "build") {
> - if ($prvalue ne "install") {
> - $buildonly = 0;
> - } else {
> - # install still limits some manditory options.
> - $buildonly = 2;
> + } elsif ($lvalue =~ /^BISECT_TYPE(\[.*\])?$/ &&
> + $prvalue ne "build") {
> + $laststep = ($prvalue eq "install") ? STEP_INSTALL : STEP_BUILD;
Here too.
In fact, with this patch, we never set to boot or more.
Also, you can make it even cleaner, by having the outer if condition be:
if ($laststep <= STEP_INSTALL && $prvalue ne "build")
And remove the prvalue check from the inner conditions.
Just send a fix of this patch, I have already pulled in the other two.
I just need to test them for a bit before I push them to my kernel.org
repo. I don't actually have a test suite for ktest. My testing is that
I use ktest on a daily basis, and I just use the latest devel ktest for
my daily activities. If something breaks, I usually notice, unless it's
affects something I haven't done recently (like a bisect).
Thanks,
-- Steve
> }
> }
>
> @@ -4045,7 +4041,7 @@ for (my $i = 1; $i <= $opt{"NUM_TESTS"}; $i++) {
> $dmesg = "$tmpdir/dmesg-$machine";
> $output_config = "$outputdir/.config";
>
> - if (!$buildonly) {
> + if ($laststep >= STEP_BOOT_OR_MORE) {
> $target = "$ssh_user\@$machine";
> if ($reboot_type eq "grub") {
> dodie "GRUB_MENU not defined" if (!defined($grub_menu));
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists