Re: [Qemu-devel] 答复： Re: [PATCH v3 2/3] qapi: auto generate enum value strings

[Lin Ma]

>>> Markus Armbruster  2016/10/11 星期二 下午 2:56 >>>
>Eric Blake  writes:
>
>> On 10/10/2016 10:09 AM, Lin Ma wrote:
>>> 
>>> 
>> Eric Blake  2016/9/27 星期二 上午 4:17 >>>
 On 09/26/2016 05:38 AM, Daniel P. Berrange wrote:
> On Mon, Sep 26, 2016 at 06:16:26PM +0800, Lin Ma wrote:
>> Automatically generate enum value strings that containing the acceptable 
>> values.
>> (Borrowwed Daniel's code.)

 s/Borrowwed/Borrowed/
>>> Sorry for the late reply, I was on vacation.
>>> Thanks for the review.

>>
>> Signed-off-by: Lin Ma 
>> ---
>>  scripts/qapi-types.py | 2 ++
>>  scripts/qapi.py   | 9 +
>>  2 files changed, 11 insertions(+)
>
> This will need some test case coverage in tests/ somewhere, but I'm
> not sure exactly which place is best - Eric/Markus can probably advise

 tests/test-qmp-commands.c is the first one that comes to mind, for
 adding another test case to an existing program.
>
>Yes, that's the closest we got.
>
>>> I'm not familiar with how to write qapi generator code and related test
>>> code at all. I'll start to dig, Any guidance is appreciated.
>>> For adding test case, Only this tests/test-qmp-commands.c needs to be
>>> modified, right? 
>>
>> Yes, I think the easiest approach is to add a new line in the main()
>> file that calls out to a new function, and the new function tests that
>> an existing QAPI enum (from tests/qapi-schema/qapi-schema-test.json) has
>> a sane conversion to a string listing all its members.  Markus may have
>> better ideas on where to place a new test, though.
>
>I think tests/test-qmp-commands.c should be split.  See
>Message-ID: <8760p7yv8n@dusky.pond.sub.org>
>http://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00664.html
>
>However, splitting it out of scope of Lin Ma's work.  Go ahead and add
>to tests/test-qmp-commands.c.
>
Thanks for your information.
 
How about these changes in tests/test-qmp-commands.c ?
@@ -262,6 +262,23 @@ static void test_dealloc_partial(void)
 qapi_free_UserDefTwo(ud2);
 }
 
+/* test generated enum value str */
+static void test_enum_value_str(void)
+{
+EnumOne i;
+char *expected_str = NULL;
+
+for (i = 0; i < ENUM_ONE__MAX; i++) {
+   if (i == 0) {
+   expected_str = g_strdup_printf("\'%s\'", EnumOne_lookup[i]);
+   } else {
+   expected_str = g_strdup_printf("%s, \'%s\'",
+   
expected_str, EnumOne_lookup[i]);
+   }
+}
+g_assert_cmpstr(EnumOne_value_str, ==, expected_str);
+}
+
 
 int main(int argc, char **argv)
 {
@@ -272,6 +289,7 @@ int main(int argc, char **argv)
 g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
 g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
 g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+g_test_add_func("/0.15/enum_value_str", test_enum_value_str);
 
 module_call_init(MODULE_INIT_QAPI);
 g_test_run();
 
 
Thanks,
Lin

Re: [Qemu-devel] 答复： Re: [PATCH v3 2/3] qapi: auto generate enum value strings

[Markus Armbruster]

Eric Blake  writes:

> On 10/10/2016 10:09 AM, Lin Ma wrote:
>> 
>> 
> Eric Blake  2016/9/27 星期二 上午 4:17 >>>
>>> On 09/26/2016 05:38 AM, Daniel P. Berrange wrote:
 On Mon, Sep 26, 2016 at 06:16:26PM +0800, Lin Ma wrote:
> Automatically generate enum value strings that containing the acceptable 
> values.
> (Borrowwed Daniel's code.)
>>>
>>> s/Borrowwed/Borrowed/
>> Sorry for the late reply, I was on vacation.
>> Thanks for the review.
>>>
>
> Signed-off-by: Lin Ma 
> ---
>  scripts/qapi-types.py | 2 ++
>  scripts/qapi.py   | 9 +
>  2 files changed, 11 insertions(+)

 This will need some test case coverage in tests/ somewhere, but I'm
 not sure exactly which place is best - Eric/Markus can probably advise
>>>
>>> tests/test-qmp-commands.c is the first one that comes to mind, for
>>> adding another test case to an existing program.

Yes, that's the closest we got.

>> I'm not familiar with how to write qapi generator code and related test
>> code at all. I'll start to dig, Any guidance is appreciated.
>> For adding test case, Only this tests/test-qmp-commands.c needs to be
>> modified, right? 
>
> Yes, I think the easiest approach is to add a new line in the main()
> file that calls out to a new function, and the new function tests that
> an existing QAPI enum (from tests/qapi-schema/qapi-schema-test.json) has
> a sane conversion to a string listing all its members.  Markus may have
> better ideas on where to place a new test, though.

I think tests/test-qmp-commands.c should be split.  See
Message-ID: <8760p7yv8n@dusky.pond.sub.org>
http://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00664.html

However, splitting it out of scope of Lin Ma's work.  Go ahead and add
to tests/test-qmp-commands.c.

Re: [Qemu-devel] 答复： Re: [PATCH v3 2/3] qapi: auto generate enum value strings

[Eric Blake]

On 10/10/2016 10:09 AM, Lin Ma wrote:
> 
> 
 Eric Blake  2016/9/27 星期二 上午 4:17 >>>
>> On 09/26/2016 05:38 AM, Daniel P. Berrange wrote:
>>> On Mon, Sep 26, 2016 at 06:16:26PM +0800, Lin Ma wrote:
 Automatically generate enum value strings that containing the acceptable 
 values.
 (Borrowwed Daniel's code.)
>>
>> s/Borrowwed/Borrowed/
> Sorry for the late reply, I was on vacation.
> Thanks for the review.
>>

 Signed-off-by: Lin Ma 
 ---
  scripts/qapi-types.py | 2 ++
  scripts/qapi.py   | 9 +
  2 files changed, 11 insertions(+)
>>>
>>> This will need some test case coverage in tests/ somewhere, but I'm
>>> not sure exactly which place is best - Eric/Markus can probably advise
>>
>> tests/test-qmp-commands.c is the first one that comes to mind, for
>> adding another test case to an existing program.
>>
> I'm not familiar with how to write qapi generator code and related test
> code at all. I'll start to dig, Any guidance is appreciated.
> For adding test case, Only this tests/test-qmp-commands.c needs to be
> modified, right? 

Yes, I think the easiest approach is to add a new line in the main()
file that calls out to a new function, and the new function tests that
an existing QAPI enum (from tests/qapi-schema/qapi-schema-test.json) has
a sane conversion to a string listing all its members.  Markus may have
better ideas on where to place a new test, though.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature