* [PATCH 1/3] ktest: add 2nd parameter of run_command() to set the redirect target file
@ 2014-03-09 14:29 Satoru Takeuchi
2014-03-09 14:32 ` [PATCH 2/3] ktest: Some cleanup for improving readability Satoru Takeuchi
0 siblings, 1 reply; 7+ messages in thread
From: Satoru Takeuchi @ 2014-03-09 14:29 UTC (permalink / raw)
To: LKML; +Cc: Steven Rostedt
From: Satoru Takeuchi <satoru.takeuchi@gmail.com>
If we'd like to set the redirect target file of run_command(),
we should define $redirect before this function and should undef it after
this function. Since it's user-unfriendly, add 2nd parameter of
run_command() for this purpose.
Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
tools/testing/ktest/ktest.pl | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 4063156..f731ef6 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -149,7 +149,6 @@ my $bisect_ret_abort;
my $bisect_ret_default;
my $in_patchcheck = 0;
my $run_test;
-my $redirect;
my $buildlog;
my $testlog;
my $dmesg;
@@ -1529,7 +1528,7 @@ sub fail {
}
sub run_command {
- my ($command) = @_;
+ my ($command, $redirect) = @_;
my $dolog = 0;
my $dord = 0;
my $pid;
@@ -2265,9 +2264,7 @@ sub build {
# Run old config regardless, to enforce min configurations
make_oldconfig;
- $redirect = "$buildlog";
- my $build_ret = run_command "$make $build_options";
- undef $redirect;
+ my $build_ret = run_command "$make $build_options", $buildlog;
if (defined($post_build)) {
# Because a post build may change the kernel version
@@ -2360,9 +2357,7 @@ sub child_run_test {
$poweroff_on_error = 0;
$die_on_failure = 1;
- $redirect = "$testlog";
- run_command $run_test or $failed = 1;
- undef $redirect;
+ run_command $run_test, $testlog or $failed = 1;
exit $failed;
}
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] ktest: Some cleanup for improving readability
2014-03-09 14:29 [PATCH 1/3] ktest: add 2nd parameter of run_command() to set the redirect target file Satoru Takeuchi
@ 2014-03-09 14:32 ` Satoru Takeuchi
2014-03-09 14:36 ` [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep Satoru Takeuchi
0 siblings, 1 reply; 7+ messages in thread
From: Satoru Takeuchi @ 2014-03-09 14:32 UTC (permalink / raw)
To: LKML; +Cc: Steven Rostedt
From: Satoru Takeuchi <satoru.takeuchi@gmail.com>
Some cleanup for improving readability as follows.
- Initialize $ktest_config at its definition.
- Put parentheses around the `config-file' argument in the usage message
because it's a optional one.
- Rename get_ktest_config{,s} to more descriptive get_mandatory_config{,s}.
Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
tools/testing/ktest/ktest.pl | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index f731ef6..c34f0de 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -72,7 +72,7 @@ my %default = (
"IGNORE_UNUSED" => 0,
);
-my $ktest_config;
+my $ktest_config = "ktest.conf";
my $version;
my $have_version = 0;
my $machine;
@@ -521,7 +521,7 @@ sub read_ync {
return read_prompt 1, $prompt;
}
-sub get_ktest_config {
+sub get_mandatory_config {
my ($config) = @_;
my $ans;
@@ -552,29 +552,29 @@ sub get_ktest_config {
}
}
-sub get_ktest_configs {
- get_ktest_config("MACHINE");
- get_ktest_config("BUILD_DIR");
- get_ktest_config("OUTPUT_DIR");
+sub get_mandatory_configs {
+ get_mandatory_config("MACHINE");
+ get_mandatory_config("BUILD_DIR");
+ get_mandatory_config("OUTPUT_DIR");
if ($newconfig) {
- get_ktest_config("BUILD_OPTIONS");
+ get_mandatory_config("BUILD_OPTIONS");
}
# options required for other than just building a kernel
if (!$buildonly) {
- get_ktest_config("POWER_CYCLE");
- get_ktest_config("CONSOLE");
+ get_mandatory_config("POWER_CYCLE");
+ get_mandatory_config("CONSOLE");
}
# options required for install and more
if ($buildonly != 1) {
- get_ktest_config("SSH_USER");
- get_ktest_config("BUILD_TARGET");
- get_ktest_config("TARGET_IMAGE");
+ get_mandatory_config("SSH_USER");
+ get_mandatory_config("BUILD_TARGET");
+ get_mandatory_config("TARGET_IMAGE");
}
- get_ktest_config("LOCALVERSION");
+ get_mandatory_config("LOCALVERSION");
return if ($buildonly);
@@ -582,7 +582,7 @@ sub get_ktest_configs {
if (!defined($rtype)) {
if (!defined($opt{"GRUB_MENU"})) {
- get_ktest_config("REBOOT_TYPE");
+ get_mandatory_config("REBOOT_TYPE");
$rtype = $entered_configs{"REBOOT_TYPE"};
} else {
$rtype = "grub";
@@ -590,16 +590,16 @@ sub get_ktest_configs {
}
if ($rtype eq "grub") {
- get_ktest_config("GRUB_MENU");
+ get_mandatory_config("GRUB_MENU");
}
if ($rtype eq "grub2") {
- get_ktest_config("GRUB_MENU");
- get_ktest_config("GRUB_FILE");
+ get_mandatory_config("GRUB_MENU");
+ get_mandatory_config("GRUB_FILE");
}
if ($rtype eq "syslinux") {
- get_ktest_config("SYSLINUX_LABEL");
+ get_mandatory_config("SYSLINUX_LABEL");
}
}
@@ -1089,7 +1089,7 @@ sub read_config {
$test_case = __read_config $config, \$test_num;
# make sure we have all mandatory configs
- get_ktest_configs;
+ get_mandatory_configs;
# was a test specified?
if (!$test_case) {
@@ -3858,7 +3858,7 @@ sub make_warnings_file {
success $i;
}
-$#ARGV < 1 or die "ktest.pl version: $VERSION\n usage: ktest.pl config-file\n";
+$#ARGV < 1 or die "ktest.pl version: $VERSION\n usage: ktest.pl [config-file]\n";
if ($#ARGV == 0) {
$ktest_config = $ARGV[0];
@@ -3868,8 +3868,6 @@ if ($#ARGV == 0) {
exit 0;
}
}
-} else {
- $ktest_config = "ktest.conf";
}
if (! -f $ktest_config) {
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep.
2014-03-09 14:32 ` [PATCH 2/3] ktest: Some cleanup for improving readability Satoru Takeuchi
@ 2014-03-09 14:36 ` Satoru Takeuchi
2014-03-10 16:13 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Satoru Takeuchi @ 2014-03-09 14:36 UTC (permalink / raw)
To: LKML; +Cc: Steven Rostedt
From: Satoru Takeuchi <satoru.takeuchi@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
Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
tools/testing/ktest/ktest.pl | 70 +++++++++++++++++++++-----------------------
1 file changed, 33 insertions(+), 37 deletions(-)
diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index c34f0de..ec82487 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -196,9 +196,14 @@ my $patchcheck_type;
my $patchcheck_start;
my $patchcheck_end;
-# set when a test is something other that just building or install
-# which would require more options.
-my $buildonly = 1;
+# Each test of ktest consists of the following steps.
+use constant {
+ STEP_BUILD => 0,
+ STEP_INSTALL => 1,
+ STEP_BOOT_OR_MORE => 2,
+};
+# The last step of test.
+my $laststep = STEP_BUILD;
# tell build not to worry about warnings, even when WARNINGS_FILE is set
my $warnings_ok = 0;
@@ -553,30 +558,27 @@ sub get_mandatory_config {
}
sub get_mandatory_configs {
+ # Options required for build step.
get_mandatory_config("MACHINE");
get_mandatory_config("BUILD_DIR");
get_mandatory_config("OUTPUT_DIR");
+ get_mandatory_config("LOCALVERSION");
if ($newconfig) {
get_mandatory_config("BUILD_OPTIONS");
}
+ return if ($laststep == STEP_BUILD);
- # options required for other than just building a kernel
- if (!$buildonly) {
- get_mandatory_config("POWER_CYCLE");
- get_mandatory_config("CONSOLE");
- }
+ # Options required for install step.
+ get_mandatory_config("SSH_USER");
+ get_mandatory_config("BUILD_TARGET");
+ get_mandatory_config("TARGET_IMAGE");
- # options required for install and more
- if ($buildonly != 1) {
- get_mandatory_config("SSH_USER");
- get_mandatory_config("BUILD_TARGET");
- get_mandatory_config("TARGET_IMAGE");
- }
-
- get_mandatory_config("LOCALVERSION");
+ return if ($laststep == STEP_INSTALL);
- return if ($buildonly);
+ # Options required for boot step.
+ get_mandatory_config("POWER_CYCLE");
+ get_mandatory_config("CONSOLE");
my $rtype = $opt{"REBOOT_TYPE"};
@@ -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;
+ 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;
}
- } 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;
}
}
@@ -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));
--
1.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep.
2014-03-09 14:36 ` [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep Satoru Takeuchi
@ 2014-03-10 16:13 ` Steven Rostedt
2014-03-11 11:43 ` Satoru Takeuchi
0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2014-03-10 16:13 UTC (permalink / raw)
To: Satoru Takeuchi; +Cc: LKML
On Sun, 09 Mar 2014 23:36:49 +0900
Satoru Takeuchi <satoru.takeuchi@gmail.com> wrote:
> From: Satoru Takeuchi <satoru.takeuchi@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));
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep.
2014-03-10 16:13 ` Steven Rostedt
@ 2014-03-11 11:43 ` Satoru Takeuchi
2014-03-11 11:50 ` Satoru Takeuchi
0 siblings, 1 reply; 7+ messages in thread
From: Satoru Takeuchi @ 2014-03-11 11:43 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Satoru Takeuchi, LKML
At Mon, 10 Mar 2014 12:13:00 -0400,
Steven Rostedt wrote:
>
> On Sun, 09 Mar 2014 23:36:49 +0900
> Satoru Takeuchi <satoru.takeuchi@gmail.com> wrote:
>
> > From: Satoru Takeuchi <satoru.takeuchi@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.
Sorry, my code review and test are insufficient.
>
> 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.
Thank you fo your comment.
>
> Just send a fix of this patch, I have already pulled in the other two.
Thanks, I'll do.
> 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).
I'll make and send my testsuite later. I considers that
tools/testing/ktest/example is suitable to place it.
Thanks,
Satoru
>
> 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));
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep.
2014-03-11 11:43 ` Satoru Takeuchi
@ 2014-03-11 11:50 ` Satoru Takeuchi
2014-03-11 19:56 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Satoru Takeuchi @ 2014-03-11 11:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Satoru Takeuchi, LKML
At Tue, 11 Mar 2014 20:43:40 +0900,
Satoru Takeuchi wrote:
>
> At Mon, 10 Mar 2014 12:13:00 -0400,
> Steven Rostedt wrote:
> >
> > On Sun, 09 Mar 2014 23:36:49 +0900
> > Satoru Takeuchi <satoru.takeuchi@gmail.com> wrote:
> >
> > > From: Satoru Takeuchi <satoru.takeuchi@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.
>
> Sorry, my code review and test are insufficient.
>
> >
> > 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.
>
> Thank you fo your comment.
>
> >
> > Just send a fix of this patch, I have already pulled in the other two.
>
> Thanks, I'll do.
>
> > 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).
>
> I'll make and send my testsuite later. I considers that
> tools/testing/ktest/example is suitable to place it.
Sorry, actually tools/testing/ktest/tests.
.../examples/ is for examples for configs file as written in examples/README.
Thanks,
Satoru
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep.
2014-03-11 11:50 ` Satoru Takeuchi
@ 2014-03-11 19:56 ` Steven Rostedt
0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2014-03-11 19:56 UTC (permalink / raw)
To: Satoru Takeuchi; +Cc: LKML
On Tue, 11 Mar 2014 20:50:53 +0900
Satoru Takeuchi <satoru.takeuchi@gmail.com> wrote:
> > > 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).
> >
> > I'll make and send my testsuite later. I considers that
> > tools/testing/ktest/example is suitable to place it.
>
> Sorry, actually tools/testing/ktest/tests.
> .../examples/ is for examples for configs file as written in examples/README.
If the code looks clean, I have no problems with adding it.
Perhaps call the directory "selftests" that way it wont be confusing to
what is there. Especially if you add a README that states that the
tests in that directory are for the purpose of testing ktest.pl itself.
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-11 19:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-09 14:29 [PATCH 1/3] ktest: add 2nd parameter of run_command() to set the redirect target file Satoru Takeuchi
2014-03-09 14:32 ` [PATCH 2/3] ktest: Some cleanup for improving readability Satoru Takeuchi
2014-03-09 14:36 ` [PATCH 3/3] ktest: remove the misleading $buildonly and introduce $laststep Satoru Takeuchi
2014-03-10 16:13 ` Steven Rostedt
2014-03-11 11:43 ` Satoru Takeuchi
2014-03-11 11:50 ` Satoru Takeuchi
2014-03-11 19:56 ` Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.