On Sun, 09 Mar 2014 23:36:49 +0900
Satoru Takeuchi <satoru.takeu...@gmail.com> wrote:

> From: Satoru Takeuchi <satoru.takeu...@gmail.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to