Re: [Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/22/2011 09:46 AM, Alex Bradbury wrote:
> On 22 December 2011 14:23, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> This is a pretty simple test that just confirms that virtio-serial shows up and
>> is writable.  It also tests the alias for virtio-serial-pci.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>   tests/virtio-serial.sh |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 52 insertions(+), 0 deletions(-)
>>   create mode 100755 tests/virtio-serial.sh
>>
>> diff --git a/tests/virtio-serial.sh b/tests/virtio-serial.sh
>> new file mode 100755
>> index 0000000..e95ae6e
>> --- /dev/null
>> +++ b/tests/virtio-serial.sh
>> @@ -0,0 +1,52 @@
>> +#!/bin/sh
>> +
>> +canary="** waiting for... **"
>> +
>> +in_host() {
>> +    tmpchr=$tmpdir/chr.log
>
> By far the most common error likely to appear in any of these test
> scripts is insufficient shell quoting causing e.g. breakages when the
> scripts are run from a directory with spaces. Having looked at
> qemu-test, I see that $tmpdir is set to .tmp-$$ and so can never
> contain spaces, and so it's fine to not quote the variable expansion.
> That said, perhaps it would make it easier to review scripts and less
> likely for these issues to creep in to the codebase if there was a
> blanket rule that all variable expansions be quoted?

In general, I try to be very careful about that.  The only time tests ever refer 
to filenames is relative to tmpdir and as you've noted, tmpdir purposefully 
cannot have a space in it.

So I don't think it's going to be a problem.  I can't see an immediate reason 
why tests should use anything outside of tmpdir.

Regards,

Anthony Liguori

>
> Alex
>