Libguestfs - Oct 2015 - Re: [PATCH 3/6] v2v:test:win: actually check the eventual layout

On Monday 05 October 2015 16:05:54 Roman Kagan wrote:
> On Mon, Oct 05, 2015 at 02:50:52PM +0200, Pino Toscano wrote:
> > On Monday 05 October 2015 15:40:03 Roman Kagan wrote:
> > > +{
> > >  guestfish --ro -a $d/windows-sda -i <<EOF
> > > +  trace 1
> > >    is-dir "/Program Files/Red Hat/Firstboot"
> > >    is-file "/Program Files/Red
Hat/Firstboot/firstboot.bat"
> > >    is-dir "/Program Files/Red Hat/Firstboot/scripts"
> > >    is-dir "/Windows/Drivers/VirtIO"
> > > +  trace 0
> > >  EOF
> > > +} | {
> > > +    ret=0
> > > +    while read r; do
> > > +        [ "$r" = "true" ] || ret=1
> > > +    done
> > > +    exit $ret
> > > +}
> > 
> > Note we have tests that check the output of guestfish -- for example,
> > fish/test-edit.sh (it is not the only one). I guess you could turn
this
> > test to do the same, which could be also easier to expand if there
will
> > be added more commands that output things different than
"true".
> 
> Yes I have seen it but exactly because I foresee nothing but
"true" in
> guestfish's output I want to avoid tedious beancounting when
maintaining
> the expected result string.
Never say never... really, better make it slightly more generic, so
expanding it later is no hassle.
Also, comparing to the exact output expected is a check more that we
got the number of lines expected.
> > > +(( PIPESTATUS[0] == 0 ))
> > 
> > set -o pipefail (which is bash-specific)
> 
> I don't mind this change; want me to resubmit with it?
Yes, with the aforementioned change of the output check, which could
make checking for the pipe status no more needed.

Thanks,
-- 
Pino Toscano

On Mon, Oct 05, 2015 at 03:19:21PM +0200, Pino Toscano
wrote:
> On Monday 05 October 2015 16:05:54 Roman Kagan wrote:
> > On Mon, Oct 05, 2015 at 02:50:52PM +0200, Pino Toscano wrote:
> > > On Monday 05 October 2015 15:40:03 Roman Kagan wrote:
> > > > +{
> > > >  guestfish --ro -a $d/windows-sda -i <<EOF
> > > > +  trace 1
> > > >    is-dir "/Program Files/Red Hat/Firstboot"
> > > >    is-file "/Program Files/Red
Hat/Firstboot/firstboot.bat"
> > > >    is-dir "/Program Files/Red
Hat/Firstboot/scripts"
> > > >    is-dir "/Windows/Drivers/VirtIO"
> > > > +  trace 0
> > > >  EOF
> > > > +} | {
> > > > +    ret=0
> > > > +    while read r; do
> > > > +        [ "$r" = "true" ] || ret=1
> > > > +    done
> > > > +    exit $ret
> > > > +}
> > > 
> > > Note we have tests that check the output of guestfish -- for
example,
> > > fish/test-edit.sh (it is not the only one). I guess you could
turn this
> > > test to do the same, which could be also easier to expand if
there will
> > > be added more commands that output things different than
"true".
> > 
> > Yes I have seen it but exactly because I foresee nothing but
"true" in
> > guestfish's output I want to avoid tedious beancounting when
maintaining
> > the expected result string.
> 
> Never say never... really, better make it slightly more generic, so
> expanding it later is no hassle.
It sure is.  E.g. in another patch in the series I add more is-file
commands here, and I generate them with a shell "for" loop; a constant
string for the expected reply with a dozen of "true" lines would look
weird IMO.
> Also, comparing to the exact output expected is a check more that we
> got the number of lines expected.
This is rather a check for guestfish which we assume already tested
here.

Roman.

On Monday 05 October 2015 16:34:31 Roman Kagan wrote:
> On Mon, Oct 05, 2015 at 03:19:21PM +0200, Pino Toscano wrote:
> > On Monday 05 October 2015 16:05:54 Roman Kagan wrote:
> > > On Mon, Oct 05, 2015 at 02:50:52PM +0200, Pino Toscano wrote:
> > > > On Monday 05 October 2015 15:40:03 Roman Kagan wrote:
> > > > > +{
> > > > >  guestfish --ro -a $d/windows-sda -i <<EOF
> > > > > +  trace 1
> > > > >    is-dir "/Program Files/Red Hat/Firstboot"
> > > > >    is-file "/Program Files/Red
Hat/Firstboot/firstboot.bat"
> > > > >    is-dir "/Program Files/Red
Hat/Firstboot/scripts"
> > > > >    is-dir "/Windows/Drivers/VirtIO"
> > > > > +  trace 0
> > > > >  EOF
> > > > > +} | {
> > > > > +    ret=0
> > > > > +    while read r; do
> > > > > +        [ "$r" = "true" ] || ret=1
> > > > > +    done
> > > > > +    exit $ret
> > > > > +}
> > > > 
> > > > Note we have tests that check the output of guestfish -- for
example,
> > > > fish/test-edit.sh (it is not the only one). I guess you
could turn this
> > > > test to do the same, which could be also easier to expand if
there will
> > > > be added more commands that output things different than
"true".
> > > 
> > > Yes I have seen it but exactly because I foresee nothing but
"true" in
> > > guestfish's output I want to avoid tedious beancounting when
maintaining
> > > the expected result string.
> > 
> > Never say never... really, better make it slightly more generic, so
> > expanding it later is no hassle.
> 
> It sure is.  E.g. in another patch in the series I add more is-file
> commands here, and I generate them with a shell "for" loop; a
constant
> string for the expected reply with a dozen of "true" lines would
look
> weird IMO.
Again: what if tomorrow we add checks that output something different
than "true"? Also, printing all the output on error would help in
debugging eventual failures of this guestfish run.
> > Also, comparing to the exact output expected is a check more that we
> > got the number of lines expected.
> 
> This is rather a check for guestfish which we assume already tested
> here.
More checks don't hurt, I'd say.

-- 
Pino Toscano