[libvirt] [PATCH] tests: redo test argv file line wrapping

[Martin Kletzander]

On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
>Back in
>
>  commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b
>  Author: Juerg Haefliger <juerg.haefliger at hp.com>
>  Date:   Mon Jan 31 06:42:57 2011 -0500
>
>    tests: handle backspace-newline pairs in test input files
>
>all the test argv files were line wrapped so that the args
>were less than 80 characters.
>
>The way the line wrapping was done turns out to be quite
>undesirable, because it often leaves multiple parameters
>on the same line. If we later need to add or remove
>individual parameters, then it leaves us having to redo
>line wrapping.
>
>This commit changes the line wrapping so that every
>single "-param value" is one its own new line. If the
>"value" is still too long, then we break on ',' or ':'
>or ' ' as needed.
>

What if we fix the syntax-check instead and allow longer than 80
character lines in case they have no space in it, or exactly one space
(to allow --parameter option,option,option,...)?  That would make even
corner cases easier to review, e.g. when you remove or add a parameter
into the long list of parameters.

>This means that when we come to add / remove parameters
>from the test files line, the patch diffs will only
>ever show a single line added/removed which will greatly
>simplify review work.
>
>Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
>---
> .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args  |  10 +-
> .../networkxml2firewalldata/nat-default-linux.args | 137 ++++++++++++++++-----
> .../qemuxml2argv-aarch64-aavmf-virtio-mmio.args    |  33 +++--
> tests/test-wrap-argv.pl                            | 106 ++++++++++++++++
> 4 files changed, 247 insertions(+), 39 deletions(-)
> create mode 100644 tests/test-wrap-argv.pl
>
>To avoid sending a 10 MB email, I'v only include these 3 example file
>changes. To view *all* the changes see my staging branch
>
>  https://gitlab.com/berrange/libvirt/commit/4fe45c39dae176ae4dbc7e126d2a2ff29b49eceb
>

Could you please remove the maintenance branches so I can add it for
the future as a remote without having multiple sets of remote
maintenance branches?  Simple script, assuming "gitlab" is the name of
your remote, is provided below for your convenience:

  git push gitlab $(git branch -a | \
  sed -n '/^[^a-z]*remotes\/gitlab.*[0-9]-maint$/s_.*remotes/gitlab/_:_p')

>'make check' and 'make syntax-check' of course still pass after
>applying the big change.
>
>diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
>new file mode 100644
>index 0000000..e8c782b
>--- /dev/null
>+++ b/tests/test-wrap-argv.pl
>@@ -0,0 +1,106 @@
>+#!/usr/bin/perl
>+
>+
>+
>+foreach my $file (@ARGV) {
>+    &rewrap($file);
>+}
>+
>+sub rewrap {
>+    my $file = shift;
>+
>+    # Read the original file
>+    open FILE, "<", $file or die "cannot read $file: $!";
>+    my @lines;
>+    while (<FILE>) {
>+        # If there is a trailing '\' then kill the new line
>+        if (/\\$/) {
>+            chomp;

You could've removed newlines from all lines and rewrap everything so
we're consistent.

It looks like this works, But I would like to know your opinion on the
suggestion above.  If you disagree with that, then ACK to both
patches, but let me know what you think, please.

Have a nice weekend
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151106/7028efd4/attachment-0001.sig>