[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test |
Date: |
Tue, 09 Oct 2018 18:41:16 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> Thomas Huth <address@hidden> writes:
>
>> On 2018-08-31 15:24, Marc-André Lureau wrote:
>>> Hi
>>> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <address@hidden> wrote:
>>>>
>>>> On 2018-08-31 14:04, Markus Armbruster wrote:
>>>>> Thomas Huth <address@hidden> writes:
>>>>>
>>>>>> From: Marc-André Lureau <address@hidden>
>>>>>>
>>>>>> test_qom_set_without_value() is about a bug in infrastructure used by
>>>>>> the QMP core, fixed in commit c489780203. We covered the bug in
>>>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
>>>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>>>>>
>>>>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>>>>> Reviewed-by: Thomas Huth <address@hidden>
>>>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>>>> ---
>>>>>> tests/qmp-test.c | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>>>>>> index b347228..2b923f0 100644
>>>>>> --- a/tests/qmp-test.c
>>>>>> +++ b/tests/qmp-test.c
>>>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>>>>>> qtest_quit(qs);
>>>>>> }
>>>>>>
>>>>>> +static void test_qom_set_without_value(void)
>>>>>> +{
>>>>>> + QTestState *qts;
>>>>>> + QDict *resp;
>>>>>> +
>>>>>> + qts = qtest_init(common_args);
>>>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>>>>>> + " { 'path': '/machine', 'property': 'rtc-time' }
>>>>>> }");
>>>>>> + g_assert_nonnull(resp);
>>>>>> + qmp_assert_error_class(resp, "GenericError");
>>>>>> + qtest_quit(qts);
>>>>>> +}
>>>>>> +
>>>>>> int main(int argc, char *argv[])
>>>>>> {
>>>>>> g_test_init(&argc, &argv, NULL);
>>>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>>>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>>>>>> qtest_add_func("qmp/oob", test_qmp_oob);
>>>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>>>>>> + qtest_add_func("qmp/qom-set-without-value",
>>>>>> test_qom_set_without_value);
>>>>>>
>>>>>> return g_test_run();
>>>>>> }
>>>>>
>>>>> I'm afraid you missed my objection to naming:
>>>>> Message-ID: <address@hidden>
>>>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>>>>
>>>> Sorry about that, I was not on CC: for that series. I used the patches
>>>> from v5 where Marc-André put me on CC:.
>>>>
>>>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
>>>>> to address it in a follow-up patch.
>>>>
>>>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
>>>> away? ... so we've got plenty of time to sort this out anyway.
>>>> Marc-André, could you send a new version of the patch?
>>>
>>> Tbh, I don't care so much about the naming of the test, but (for once)
>>> I respectfully don't think Markus suggestion is better.
>>>
>>> The function checks "qom-set" without 'value' argument:
>>> "qom-set-without-value", no brainer..
>
> Nope, that's not what it tests. It tests the visitor, the marshalling
> code generator, and the QMP core handle a certain kind of invalid
> arguments correctly. It does not test qom-set. I explained all that
> already.
>
>>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
>
> I can accept "missing-any" or "missing-any-arg". I object to any name
> involving qom-set, because the test is not about qom-set at all.
>
> If it was, then putting it in qmp-test.c would be wrong.
>
>> Ok, then let's keep it this way. As I said, IMHO the current naming is
>> not really bad, and I also don't have any suggestions for a perfect name
>> right now.
>
> We don't need a perfect name. We need one that's not actively
> misleading.
Marc-André, would "qmp/missing-any-arg" and test_missing_any_arg() work
for you?
- Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test,
Markus Armbruster <=