| Message ID | 20160926101627.14296-3-lma@suse.com |
|---|---|
| State | New |
| Headers | show |
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.) > > Signed-off-by: Lin Ma <lma@suse.com> > --- > 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 Regards, Daniel
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/ >> >> Signed-off-by: Lin Ma <lma@suse.com> >> --- >> 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.
>>> Eric Blake <eblake@redhat.com> 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 <lma@suse.com> >>> --- >>> 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? Thanks, Lin
On 10/10/2016 10:09 AM, Lin Ma wrote: > > >>>> Eric Blake <eblake@redhat.com> 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 <lma@suse.com> >>>> --- >>>> 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> writes: > On 10/10/2016 10:09 AM, Lin Ma wrote: >> >> >>>>> Eric Blake <eblake@redhat.com> 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 <lma@suse.com> >>>>> --- >>>>> 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.fsf@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.
>>> Markus Armbruster <armbru@redhat.com> 2016/10/11 星期二 下午 2:56 >>> >Eric Blake <eblake@redhat.com> writes: > >> On 10/10/2016 10:09 AM, Lin Ma wrote: >>> >>> >>>>>> Eric Blake <eblake@redhat.com> 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 <lma@suse.com> >>>>>> --- >>>>>> 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.fsf@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
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index dabc42e..0446839 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self._btin += gen_enum(name, values, prefix) if do_builtins: self.defn += gen_enum_lookup(name, values, prefix) + self._btin += gen_enum_value_str(name, values) else: self._fwdecl += gen_enum(name, values, prefix) self.defn += gen_enum_lookup(name, values, prefix) + self._fwdecl += gen_enum_value_str(name, values) def visit_array_type(self, name, info, element_type): if isinstance(element_type, QAPISchemaBuiltinType): diff --git a/scripts/qapi.py b/scripts/qapi.py index 21bc32f..d11c414 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = { return ret +def gen_enum_value_str(name, values): + return mcgen(''' + +#define %(c_name)s_value_str "%(value_str)s" +''', + c_name=c_name(name), + value_str=", ".join(["'%s'" % c for c in values])) + + def gen_enum(name, values, prefix=None): # append automatically generated _MAX value enum_values = values + ['_MAX']
Automatically generate enum value strings that containing the acceptable values. (Borrowwed Daniel's code.) Signed-off-by: Lin Ma <lma@suse.com> --- scripts/qapi-types.py | 2 ++ scripts/qapi.py | 9 +++++++++ 2 files changed, 11 insertions(+)