Re: [libvirt] [PATCH] tests: redo test argv file line wrapping
On Fri, Nov 06, 2015 at 01:30:35PM +0000, Daniel P. Berrange wrote:
Back in
commit bd6c46fa0cfe275c24debc1152cfc5206c04b59b
Author: Juerg Haefliger <juerg.haefliger(a)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(a)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/4fe45c39dae176ae4dbc7e126d2a2f...
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
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)