Re: [Qemu-block] [Qemu-devel] [PATCH v4 2/2] qemu-io: Add regression tests

[Eric Blake]

On 01/31/2017 07:57 AM, Nir Soffer wrote:
> From: Nir Soffer <address@hidden>
> 
> Add regression tests checking that qemu-io fail with non-zero exit code

s/fail/fails/

> when reading non-existing file or using the wrong format.
> 
> Signed-off-by: Nir Soffer <address@hidden>

Odd to have the 'From:' and the 'S-o-b:' from different addresses.  More
on that below...

> ---
>  tests/qemu-iotests/173     | 59 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/173.out |  9 +++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 69 insertions(+)
>  create mode 100755 tests/qemu-iotests/173
>  create mode 100644 tests/qemu-iotests/173.out
> 
> diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173
> new file mode 100755
> index 0000000..1d1fd6d
> --- /dev/null
> +++ b/tests/qemu-iotests/173
> @@ -0,0 +1,59 @@
> +#!/bin/bash
> +#
> +# Test that qemu-io fail with non-zero exit code
> +#
> +# Copyright (C) 2017 Red Hat, Inc.

A bit unusual to use a Red Hat copyright,...

> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> address@hidden

...but a personal address.  But as I understand, Red Hat doesn't see
anything wrong with that, so the copyright line is no need to respin
(other companies may have different policies, but that's up to employees
of those companies to comply with).  Still, you may want to check
whether .mailmap needs an entry, and/or fix your git config settings to
use a consistent address, since mixing email addresses in the git log
can make it harder to trace which email address you prefer.

> +
> +_supported_fmt raw

Is it worth making this test work for more than just raw?

> +
> +
> +size=256K
> +_make_test_img $size
> +
> +echo
> +echo "== reading wrong format should fail =="
> +$QEMU_IO -f qcow2 -c "read 0 $size" "$TEST_IMG" 2>&1 | _filter_testdir

If we widen the test to support additional formats, then the '-f qcow2'
would have to be variable to guarantee that it is not the same type as
the current format.

I guess that the point of this test is to ensure qemu-io behaves sanely,
and we've done that with a single format (even if it means that test
doesn't get run on other formats, we aren't really losing much because
it is not a format-specific failure).  So I guess I'm okay with your
decision to just support raw.

> +test "${PIPESTATUS[0]}" -eq 1 || _fail "did not fail"

Thank goodness this is a bash script, as that is a bashism.  But it
works for me.

> +++ b/tests/qemu-iotests/group
> @@ -165,3 +165,4 @@
>  170 rw auto quick
>  171 rw auto quick
>  172 auto
> +173 auto

I already have a pending patch series that adds test 173; it's a trivial
merge conflict for whoever lands second, although if you have a reason
to respin, you may want to grab 174.
https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg02834.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature