Re: [libvirt] [PATCH] commandtest: Remove commandhelper.log in test27

2019-07-29 Thread Daniel P . Berrangé
On Mon, Jul 29, 2019 at 04:17:17PM +0200, Michal Privoznik wrote:
> The recently added test27 spawns commandhelper. This is fine,
> except, one of the things that commandhelper does is it records
> arguments it was spawn with into commandhelper.log. Other test
> cases then use checkoutput() to compare the arguments against the
> expected ones and also unlink() the log file. However, test27()
> is not doing that and thus it leaves the file behind. This
> breaks distcheck.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> Another approach would be to explicitly just unlink() the file somewhere
> in the cleanup label. But I figured, comparing the args and unlinking
> the file afterwards is more robust check IMO.
> 
>  tests/commanddata/test27.log | 20 
>  tests/commandtest.c  |  3 +++
>  2 files changed, 23 insertions(+)
>  create mode 100644 tests/commanddata/test27.log

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] commandtest: Remove commandhelper.log in test27

2019-07-29 Thread Michal Privoznik
The recently added test27 spawns commandhelper. This is fine,
except, one of the things that commandhelper does is it records
arguments it was spawn with into commandhelper.log. Other test
cases then use checkoutput() to compare the arguments against the
expected ones and also unlink() the log file. However, test27()
is not doing that and thus it leaves the file behind. This
breaks distcheck.

Signed-off-by: Michal Privoznik 
---

Another approach would be to explicitly just unlink() the file somewhere
in the cleanup label. But I figured, comparing the args and unlinking
the file afterwards is more robust check IMO.

 tests/commanddata/test27.log | 20 
 tests/commandtest.c  |  3 +++
 2 files changed, 23 insertions(+)
 create mode 100644 tests/commanddata/test27.log

diff --git a/tests/commanddata/test27.log b/tests/commanddata/test27.log
new file mode 100644
index 00..40b2627c07
--- /dev/null
+++ b/tests/commanddata/test27.log
@@ -0,0 +1,20 @@
+ARG:--readfd
+ARG:5
+ARG:--readfd
+ARG:7
+ENV:DISPLAY=:0.0
+ENV:HOME=/home/test
+ENV:HOSTNAME=test
+ENV:LANG=C
+ENV:LOGNAME=test
+ENV:PATH=/usr/bin:/bin
+ENV:TMPDIR=/tmp
+ENV:USER=test
+FD:0
+FD:1
+FD:2
+FD:5
+FD:7
+DAEMON:no
+CWD:/tmp
+UMASK:0022
diff --git a/tests/commandtest.c b/tests/commandtest.c
index dfd15a2079..d7ab588969 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1231,6 +1231,9 @@ static int test27(const void *unused ATTRIBUTE_UNUSED)
 goto cleanup;
 }
 
+if (checkoutput("test27", NULL) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list