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.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.
>
> 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
--
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/
