tests/test-lib.sh | 13 +++++++++++++ tests/virsh-snapshot | 2 ++ tests/virsh-uriprecedence | 12 +----------- 3 files changed, 16 insertions(+), 11 deletions(-)
In a constrained CI environment, where it is intentional that attempts
to write outside the current directory will fail, virsh-snapshot was
failing:
@@ -1,2 +1,3 @@
error: invalid argument: parent s3 for snapshot s2 not found
error: marker
+error: Failed to create '/home/travis/.cache/libvirt/virsh': Permission denied
FAIL virsh-snapshot (exit status: 1)
But we've already solved the problem in virsh-uriprecedence: tell
virsh to use XDG locations pointing to somewhere we can write rather
than its default of falling back to $HOME with the test being at risk
of breaking due to the user's environment and/or unacceptably altering
the user's normal cache. Hoist that solution into test-lib.sh, so
that all scripts can use it as needed.
Fixes: 280a2b41e
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/test-lib.sh | 13 +++++++++++++
tests/virsh-snapshot | 2 ++
tests/virsh-uriprecedence | 12 +-----------
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 49e8d22095..64f0b0d401 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -222,6 +222,19 @@ mkfifo_or_skip_()
fi
}
+# Create mock XDG files/directories to avoid permission problems.
+# As it points inside $test_dir_, it is automatically cleaned.
+mock_xdg_()
+{
+ export XDG_CONFIG_HOME="$test_dir_/.config"
+ export XDG_CACHE_HOME="$test_dir_/.cache"
+ export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
+
+ mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
+ mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
+ mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
+}
+
test_dir_=$(pwd)
this_test_() { echo "./$0" | sed 's,.*/,,'; }
diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot
index fb8a99dd43..cb498cf54e 100755
--- a/tests/virsh-snapshot
+++ b/tests/virsh-snapshot
@@ -26,6 +26,8 @@ fi
fail=0
+mock_xdg_ || framework_failure
+
# The test driver loses states between restarts, so we perform a script
# with some convenient markers for later post-processing of output.
$abs_top_builddir/tools/virsh --connect test:///default >out 2>err '
diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence
index 564e3dc42c..fd6ce108c0 100755
--- a/tests/virsh-uriprecedence
+++ b/tests/virsh-uriprecedence
@@ -11,17 +11,7 @@ virsh_cmd="$virsh_bin"
counter=0
ret=0
-cleanup_() { rm -rf "$tmphome"; }
-
-# Create all mock files/directories to avoid permission problems
-tmphome="$PWD/tmp_home"
-export XDG_CONFIG_HOME="$tmphome/.config"
-export XDG_CACHE_HOME="$tmphome/.cache"
-export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
-
-mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
-mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
-mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
+mock_xdg_ || framework_failure
is_uri_good()
{
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2019-03-27 at 13:47 -0500, Eric Blake wrote: [...] > +# Create mock XDG files/directories to avoid permission problems. > +# As it points inside $test_dir_, it is automatically cleaned. > +mock_xdg_() There doesn't seem to be a consistent pattern deciding when functions should be named_like_this() instead of being named_like_that_(), so I guess either one works fine. > +{ > + export XDG_CONFIG_HOME="$test_dir_/.config" > + export XDG_CACHE_HOME="$test_dir_/.cache" > + export XDG_RUNTIME_HOME="XDG_CACHE_HOME" Aren't you missing the $ in front of XDG_CACHE_HOME here? The same was actually true of the original incarnation of the code, too. With $XDG_RUNTIME_HOME set correctly, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2019-03-28 at 10:13 +0100, Andrea Bolognani wrote: > On Wed, 2019-03-27 at 13:47 -0500, Eric Blake wrote: > > + export XDG_CONFIG_HOME="$test_dir_/.config" > > + export XDG_CACHE_HOME="$test_dir_/.cache" > > + export XDG_RUNTIME_HOME="XDG_CACHE_HOME" > > Aren't you missing the $ in front of XDG_CACHE_HOME here? The same > was actually true of the original incarnation of the code, too. Wait, I actually spotted two more issues with this patch. First of all, you'll want to add /tests/.cache/ /tests/.config/ to .gitignore; moreover, with this patch applied distcheck will fail with ERROR: files left in build directory after distclean: ./tests/.cache/libvirt/virsh/history ./tests/.config/libvirt/libvirt.conf make[1]: *** [Makefile:2414: distcleancheck] Error 1 so that will have to be addressed as well. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 3/28/19 6:53 AM, Andrea Bolognani wrote: > On Thu, 2019-03-28 at 10:13 +0100, Andrea Bolognani wrote: >> On Wed, 2019-03-27 at 13:47 -0500, Eric Blake wrote: >>> + export XDG_CONFIG_HOME="$test_dir_/.config" >>> + export XDG_CACHE_HOME="$test_dir_/.cache" >>> + export XDG_RUNTIME_HOME="XDG_CACHE_HOME" >> >> Aren't you missing the $ in front of XDG_CACHE_HOME here? The same >> was actually true of the original incarnation of the code, too. > > Wait, I actually spotted two more issues with this patch. > > First of all, you'll want to add > > /tests/.cache/ > /tests/.config/ > > to .gitignore; Or update to use something other than $test_dir_ that actually gets cleaned up on a per-test basis, so that .gitignore won't see it in the first place. > moreover, with this patch applied distcheck will > fail with > > ERROR: files left in build directory after distclean: > ./tests/.cache/libvirt/virsh/history > ./tests/.config/libvirt/libvirt.conf > make[1]: *** [Makefile:2414: distcleancheck] Error 1 > > so that will have to be addressed as well. Good catch; I'll be sure to test distcheck on my v2. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 27, 2019 at 01:47:37PM -0500, Eric Blake wrote: > In a constrained CI environment, where it is intentional that attempts > to write outside the current directory will fail, virsh-snapshot was > failing: > > @@ -1,2 +1,3 @@ > error: invalid argument: parent s3 for snapshot s2 not found > error: marker > +error: Failed to create '/home/travis/.cache/libvirt/virsh': Permission denied > FAIL virsh-snapshot (exit status: 1) > > But we've already solved the problem in virsh-uriprecedence: tell > virsh to use XDG locations pointing to somewhere we can write rather > than its default of falling back to $HOME with the test being at risk > of breaking due to the user's environment and/or unacceptably altering > the user's normal cache. Hoist that solution into test-lib.sh, so > that all scripts can use it as needed. Ahhh, I was wondering why other tests didn't fail for the same reason. In fact I wonder if more virsh tests are triggering this, but simply ignoring the error. > > Fixes: 280a2b41e > Reported-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > tests/test-lib.sh | 13 +++++++++++++ > tests/virsh-snapshot | 2 ++ > tests/virsh-uriprecedence | 12 +----------- > 3 files changed, 16 insertions(+), 11 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> 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
© 2016 - 2024 Red Hat, Inc.