[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/18] iotests: Filter refcount_order in 036
From: |
Max Reitz |
Subject: |
Re: [PATCH 01/18] iotests: Filter refcount_order in 036 |
Date: |
Mon, 30 Sep 2019 15:44:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 |
On 30.09.19 15:40, Maxim Levitsky wrote:
> On Mon, 2019-09-30 at 14:43 +0200, Max Reitz wrote:
>> On 29.09.19 18:31, Maxim Levitsky wrote:
>>> On Fri, 2019-09-27 at 11:42 +0200, Max Reitz wrote:
>>>> This test can run just fine with other values for refcount_bits, so we
>>>> should filter the value from qcow2.py's dump-header.
>>>>
>>>> (036 currently ignores user-specified image options, but that will be
>>>> fixed in the next patch.)
>>>>
>>>> Signed-off-by: Max Reitz <address@hidden>
>>>> ---
>>>> tests/qemu-iotests/036 | 9 ++++++---
>>>> tests/qemu-iotests/036.out | 6 +++---
>>>> 2 files changed, 9 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
>>>> index f06ff67408..69d0f9f903 100755
>>>> --- a/tests/qemu-iotests/036
>>>> +++ b/tests/qemu-iotests/036
>>>> @@ -55,7 +55,8 @@ $PYTHON qcow2.py "$TEST_IMG" set-feature-bit
>>>> incompatible 63
>>>>
>>>> # Without feature table
>>>> $PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
>>>> -$PYTHON qcow2.py "$TEST_IMG" dump-header
>>>> +$PYTHON qcow2.py "$TEST_IMG" dump-header \
>>>> + | sed -e 's/^\(refcount_order\s*\).*/\1(filtered)/'
>>>> _img_info
>>>>
>>>> # With feature table containing bit 63
>>>> @@ -103,14 +104,16 @@ echo === Create image with unknown autoclear feature
>>>> bit ===
>>>> echo
>>>> _make_test_img 64M
>>>> $PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 63
>>>> -$PYTHON qcow2.py "$TEST_IMG" dump-header
>>>> +$PYTHON qcow2.py "$TEST_IMG" dump-header \
>>>> + | sed -e 's/^\(refcount_order\s*\).*/\1(filtered)/'
>>>>
>>>> echo
>>>> echo === Repair image ===
>>>> echo
>>>> _check_test_img -r all
>>>>
>>>> -$PYTHON qcow2.py "$TEST_IMG" dump-header
>>>> +$PYTHON qcow2.py "$TEST_IMG" dump-header \
>>>> + | sed -e 's/^\(refcount_order\s*\).*/\1(filtered)/'
>>>>
>>>> # success, all done
>>>> echo "*** done"
>>>> diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
>>>> index e489b44386..998c2a8a35 100644
>>>> --- a/tests/qemu-iotests/036.out
>>>> +++ b/tests/qemu-iotests/036.out
>>>> @@ -19,7 +19,7 @@ snapshot_offset 0x0
>>>> incompatible_features 0x8000000000000000
>>>> compatible_features 0x0
>>>> autoclear_features 0x0
>>>> -refcount_order 4
>>>> +refcount_order (filtered)
>>>> header_length 104
>>>>
>>>> qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT
>>>> feature(s): Unknown incompatible feature: 8000000000000000
>>>> @@ -53,7 +53,7 @@ snapshot_offset 0x0
>>>> incompatible_features 0x0
>>>> compatible_features 0x0
>>>> autoclear_features 0x8000000000000000
>>>> -refcount_order 4
>>>> +refcount_order (filtered)
>>>> header_length 104
>>>>
>>>> Header extension:
>>>> @@ -81,7 +81,7 @@ snapshot_offset 0x0
>>>> incompatible_features 0x0
>>>> compatible_features 0x0
>>>> autoclear_features 0x0
>>>> -refcount_order 4
>>>> +refcount_order (filtered)
>>>> header_length 104
>>>>
>>>> Header extension:
>>>
>>> Minor notes:
>>>
>>> 1. Maybe put the sed command into a function to avoid duplication?
>>
>> Hm, maybe, but that would increase the LoC, so I’m not sure whether it
>> really would be a simplification.
> IMHO, in my opinion, regardless of LOC, duplicated code is almost always
> bad. Common functions are mostly for the next guy that will be able to use
> them instead of searching through code to see how this is done there and
> there.
In my experience, people write tests without scanning common.filter for
whether anyone has written the same code already. See the
_filter_qemu_img_check example in this series.
>>
>>> 2. As I understand the test, it only checks the feature bits.
>>> So maybe instead of blacklisting some of the values, white list
>>> only the feature bits in the output?
>>
>> Hm. Seems reasonable, but then again I’d prefer a minimal change, and
>> changing it to a whitelist would be more change...
> I don't think this is bad, again in my eyes, the code should be as friendly
> as possible for the next guy that will have to change it, so adding
> some more extra changes don't seem a problem to me.
In my eyes tests aren’t code.
Max
> Of course this is only my personal option and each approach has its own cons,
> and pros. This doesn't matter that much to me.
>
> Best regards,
> Maxim Levitsky
>
signature.asc
Description: OpenPGP digital signature
- [PATCH 00/18] iotests: Allow ./check -o data_file, Max Reitz, 2019/09/27
- [PATCH 01/18] iotests: Filter refcount_order in 036, Max Reitz, 2019/09/27
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Maxim Levitsky, 2019/09/29
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Max Reitz, 2019/09/30
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Maxim Levitsky, 2019/09/30
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036,
Max Reitz <=
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Maxim Levitsky, 2019/09/30
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Max Reitz, 2019/09/30
- Re: [PATCH 01/18] iotests: Filter refcount_order in 036, Maxim Levitsky, 2019/09/30
[PATCH 02/18] iotests: Replace IMGOPTS by _unsupported_imgopts, Max Reitz, 2019/09/27
[PATCH 03/18] iotests: Drop compat=1.1 in 050, Max Reitz, 2019/09/27