[libvirt] [PATCH v2] tests: Add URI precedence checking

[Martin Kletzander]

On Thu 22 Aug 2013 03:15:48 PM CEST, Eric Blake wrote:
> On 08/22/2013 06:19 AM, Martin Kletzander wrote:
>> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
>> to deal with this issue again, herec comes "virsh-uriprecedence".
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>
>> @@ -235,6 +236,7 @@ test_scripts +=				\
>>  	read-bufsiz			\
>>  	read-non-seekable		\
>>  	start				\
>> +	virsh-uriprecedence		\
>>  	vcpupin				\
>
> No longer sorted after the rename :)
>

Oh, rushing the v2 always gets me :(

>> +++ b/tests/virsh-uriprecedence
>> @@ -0,0 +1,76 @@
>> +#!/bin/sh
>
> I'm reviewing this for POSIX sh compatibility...
>
>> +# 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"
>
> Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to
> XDG_CACHE_HOME?  Also, you could use one 'mkdir -p' to make all the
> directories (but shaving processes isn't essential).
>

Yes, it can.  I thought it'll be better for the future and
understandability and won't hurt in case there's one 'mkdir -p'.
Unfortunately I the haven't used it.

>> +
>> +printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
>
> Style - slightly shorter as printf 'uri_default="%s"\n' ...
> Long line - is it worth wrapping with \-newline?
>

Definitely, I missed this one.

> Everything looks portable - I have no objection to this patch as-is,
> although if you haven't pushed yet, you can consider tweaking the nits I
> pointed out.
>

Unfortunately, I pushed it with the thought that 2 ACKs should cover
anything.  However, I'll be more than happy to fix these in a follow-up
patch if you think it's worth it.

Martin