[PATCH v2 23/25] bhyvexml2argvtest: Use internal wrapping of command line arguments

[Peter Krempa]

On Fri, Apr 09, 2021 at 17:36:40 +0200, Pavel Hrdina wrote:
> On Fri, Apr 09, 2021 at 02:50:25PM +0200, Peter Krempa wrote:
> > virCommandToString has the possibility to return an already wrapped
> > string with better format than what we get from the test wrapper script.
> > 
> > The main advantage is that arguments for an option are always on the
> > same line which makes it more easy to see what changed in a diff and
> > prevents re-wrapping of the line if a wrapping point moves over the
> > threshold.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---

[...]

> > diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args
> > index d7917bd8f3..bd987c86aa 100644
> > --- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args
> > +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-more-than-32-sata-disks.args
> > @@ -5,17 +5,7 @@
> >  -H \
> >  -P \
> >  -s 0:0,hostbridge \
> > --s 2:0,ahci,hd:/tmp/freebsd1.img,hd:/tmp/freebsd2.img,hd:/tmp/freebsd3.img,\
> > -hd:/tmp/freebsd4.img,hd:/tmp/freebsd5.img,hd:/tmp/freebsd6.img,\
> > -hd:/tmp/freebsd7.img,hd:/tmp/freebsd8.img,hd:/tmp/freebsd9.img,\
> > -hd:/tmp/freebsd10.img,hd:/tmp/freebsd11.img,hd:/tmp/freebsd12.img,\
> > -hd:/tmp/freebsd12.img,hd:/tmp/freebsd13.img,hd:/tmp/freebsd14.img,\
> > -hd:/tmp/freebsd15.img,hd:/tmp/freebsd16.img,hd:/tmp/freebsd17.img,\
> > -hd:/tmp/freebsd18.img,hd:/tmp/freebsd19.img,hd:/tmp/freebsd20.img,\
> > -hd:/tmp/freebsd21.img,hd:/tmp/freebsd22.img,hd:/tmp/freebsd23.img,\
> > -hd:/tmp/freebsd24.img,hd:/tmp/freebsd25.img,hd:/tmp/freebsd26.img,\
> > -hd:/tmp/freebsd27.img,hd:/tmp/freebsd28.img,hd:/tmp/freebsd29.img,\
> > -hd:/tmp/freebsd30.img \
> > --s 3:0,ahci,hd:/tmp/freebsd31.img,hd:/tmp/freebsd32.img,hd:/tmp/freebsd33.img,\
> > -hd:/tmp/freebsd34.img,hd:/tmp/freebsd35.img \
> > --s 4:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve
> > +-s 2:0,ahci,hd:/tmp/freebsd1.img,hd:/tmp/freebsd2.img,hd:/tmp/freebsd3.img,hd:/tmp/freebsd4.img,hd:/tmp/freebsd5.img,hd:/tmp/freebsd6.img,hd:/tmp/freebsd7.img,hd:/tmp/freebsd8.img,hd:/tmp/freebsd9.img,hd:/tmp/freebsd10.img,hd:/tmp/freebsd11.img,hd:/tmp/freebsd12.img,hd:/tmp/freebsd12.img,hd:/tmp/freebsd13.img,hd:/tmp/freebsd14.img,hd:/tmp/freebsd15.img,hd:/tmp/freebsd16.img,hd:/tmp/freebsd17.img,hd:/tmp/freebsd18.img,hd:/tmp/freebsd19.img,hd:/tmp/freebsd20.img,hd:/tmp/freebsd21.img,hd:/tmp/freebsd22.img,hd:/tmp/freebsd23.img,hd:/tmp/freebsd24.img,hd:/tmp/freebsd25.img,hd:/tmp/freebsd26.img,hd:/tmp/freebsd27.img,hd:/tmp/freebsd28.img,hd:/tmp/freebsd29.img,hd:/tmp/freebsd30.img \
> > +-s 3:0,ahci,hd:/tmp/freebsd31.img,hd:/tmp/freebsd32.img,hd:/tmp/freebsd33.img,hd:/tmp/freebsd34.img,hd:/tmp/freebsd35.img \
> > +-s 4:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \
> > +bhyve
> 
> Not sure about this being an improvement. Since it is one looooong line
> any change to the line will be difficult to spot at first glance.
> 
> But it's a trade-off where the new wrapping improves a lot of other
> cases so I guess we will have to live with this.

The issue and main reason for change is in most cases default diff-view
would be useless even if it were wrapped if the change triggered a
re-wrap of the commandline, which was happening quite often.

While when we have all arguments on one line:

- you know that something has changed in that argument
- you an use word-diff and word-diff regex to hilight what actually has
  changed
- you don't get any noise from rewrapping

In this very particular case the line is extremely long, but note that
exactly the same wrapping would be in the VM log file anyways so I don't
think it's that much worse.