[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] tests: Expose regression in QemuOpts visito
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] tests: Expose regression in QemuOpts visitor |
Date: |
Tue, 21 Mar 2017 14:33:19 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 21/03/2017 14:21, Eric Blake wrote:
> On 03/21/2017 04:01 AM, Laurent Vivier wrote:
>> On 21/03/2017 04:17, Eric Blake wrote:
>>> Commit 15c2f669e broke the ability of the QemuOpts visitor to
>>> flag extra input parameters, but the regression went unnoticed
>>> because of missing testsuite coverage. Add a test to cover this.
>>
>> I don't know where I'm wrong, but when I run this test without the fix
>> it never fails.
>
> Intentional:
>
>
>>> + v = opts_visitor_new(opts);
>>> + /* FIXME: bogus should be diagnosed */
>>> + visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
>
> The test is written with a FIXME here, then updated in the next patch to
> remove the fixme and adjust the condition to what we really want, so
> that 'make check-unit' is not broken in the meantime.
OK.
Why don't you reverse the patch order to have a commit to apply the fix
and a commit to apply the test (fully)?
Like this, it easy to not apply the fix and only the test to check the
test really detects the problem and the fix really fix it (it's what
I've tried to do)... and the "make check" is never broken.
Thanks,
Laurent
[Qemu-devel] [PATCH 2/2] qapi: Fix QemuOpts visitor regression on unvisited input, Eric Blake, 2017/03/20
Re: [Qemu-devel] [PATCH for-2.9 0/2] Fix QemuOpts regression on bogus keys, Markus Armbruster, 2017/03/21