Re: [PATCH 15/18] iotests: Make 137 work with data_file

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2689 bytes --]

On 30.09.19 15:46, Maxim Levitsky wrote:
> On Mon, 2019-09-30 at 15:38 +0200, Max Reitz wrote:
>> On 29.09.19 18:38, Maxim Levitsky wrote:
>>> On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote:
>>>> When using an external data file, there are no refcounts for data
>>>> clusters.  We thus have to adjust the corruption test in this patch to
>>>> not be based around a data cluster allocation, but the L2 table
>>>> allocation (L2 tables are still refcounted with external data files).
>>>>
>>>> Doing so means this test works both with and without external data
>>>> files.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>>>  tests/qemu-iotests/137     | 10 ++++++----
>>>>  tests/qemu-iotests/137.out |  4 +---
>>>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
>>>> index 6cf2997577..dd3484205e 100755
>>>> --- a/tests/qemu-iotests/137
>>>> +++ b/tests/qemu-iotests/137
>>>> @@ -138,14 +138,16 @@ $QEMU_IO \
>>>>      "$TEST_IMG" 2>&1 | _filter_qemu_io
>>>>  
>>>>  # The dirty bit must not be set
>>>> -$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
>>>> +# (Filter the external data file bit)
>>>> +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features \
>>>> +    | sed -e 's/0x4/0x0/'
>>>
>>> Maybe it is better to filter all the feature bits, but the dirty bit,
>>> since only it is needed here, so that when we start running tests with
>>> more features, we won't need to do this again?
>>
>> I’d hate a filter s/[02468ace]$/no dirty bit/ though.
> Nothing a helper function can't solve IMHO, I would convert this to a number,
> and then check bitwise the bit 2, assuming that is the dirty bit)
> Again, note that my approach to code is to make it as easy as possible for the
> next guy to change, so I am noticing such places. Eventually someone of us,
> will be that next guy. Then again, I don't mind leaving this as is, just noting this.

Again, my approach to tests is that they aren’t classical code.

This is a very personal opinion, but I have found that tests that have
the most ad-hoc code with the least function calls are the easiest to
work with.

Tests that have a whole lot of infrastructure and try to have nice code
are a horror to work with because you first have to understand how they
work.

Tests should be simple, not complex.  Some ad-hoc filters do not make
them complex as long as it’s obvious what they do.


Also, the correct approach here is not to do number crunching in bash.
It is to change qcow2.py to emit more easily filterable information.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]