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 <mre...@redhat.com> >>>> --- >>>> 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
