[PATCH 0/7] refactor kvargs test

[PATCH 0/7] refactor kvargs test

[Chengwen Feng]

When developing patchset [1], I found the kvargs test is hard to 
understand when tried to add some testcase.

So refactor kvargs by:
1. introduce UT suite framework.
2. extract big test_valid_kvargs() to five part.

And add myself for voluntary maintenance of the kvargs library.

Note: to ensure patch independenct, new API which in patchset [1]
are not included in this patchset.

[1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/

Chengwen Feng (7):
  app/test: introduce UT suite framework for kvargs
  app/test: extract basic token count testcase for kvargs
  app/test: extract without keys testcase for kvargs
  app/test: extract with keys testcase for kvargs
  app/test: extract parse list value testcase for kvargs
  app/test: extract parse empty elements testcase for kvargs
  maintainers: update for kvargs library

 MAINTAINERS            |   1 +
 app/test/test_kvargs.c | 256 ++++++++++++++++++++---------------------
 2 files changed, 128 insertions(+), 129 deletions(-)

-- 
2.17.1

[PATCH 1/7] app/test: introduce UT suite framework for kvargs

[Chengwen Feng]

Introduce unit test suite framework for test_kvargs.c.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7a60cac4c1..9aeb9aa0aa 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright 2014 6WIND S.A.
+ * Copyright(c) 2023 HiSilicon Limited
  */
 
 #include <stdlib.h>
@@ -280,16 +281,21 @@ static int test_invalid_kvargs(void)
 	return -1;
 }
 
+static struct unit_test_suite kvargs_test_suite  = {
+	.suite_name = "Kvargs Unit Test Suite",
+	.setup = NULL,
+	.teardown = NULL,
+	.unit_test_cases = {
+		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_invalid_kvargs),
+		TEST_CASES_END() /**< NULL terminate unit test array */
+	}
+};
+
 static int
 test_kvargs(void)
 {
-	printf("== test valid case ==\n");
-	if (test_valid_kvargs() < 0)
-		return -1;
-	printf("== test invalid case ==\n");
-	if (test_invalid_kvargs() < 0)
-		return -1;
-	return 0;
+	return unit_test_suite_runner(&kvargs_test_suite);
 }
 
 REGISTER_FAST_TEST(kvargs_autotest, true, true, test_kvargs);
-- 
2.17.1

Re: [PATCH 1/7] app/test: introduce UT suite framework for kvargs

[Stephen Hemminger]

On Fri, 3 Nov 2023 09:53:19 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:

> Introduce unit test suite framework for test_kvargs.c.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  app/test/test_kvargs.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
> index 7a60cac4c1..9aeb9aa0aa 100644
> --- a/app/test/test_kvargs.c
> +++ b/app/test/test_kvargs.c
> @@ -1,5 +1,6 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright 2014 6WIND S.A.
> + * Copyright(c) 2023 HiSilicon Limited
>   */

The test changes look good and it is always better to have more tests.

IANAL but starting the precedent of allowing adding copyright whenever a file
is touched just leads to lots of meaningless fluff.

Re: [PATCH 1/7] app/test: introduce UT suite framework for kvargs

[fengchengwen]

On 2024/10/11 10:08, Stephen Hemminger wrote:
> On Fri, 3 Nov 2023 09:53:19 +0000
> Chengwen Feng <fengchengwen@huawei.com> wrote:
> 
>> Introduce unit test suite framework for test_kvargs.c.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>  app/test/test_kvargs.c | 20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
>> index 7a60cac4c1..9aeb9aa0aa 100644
>> --- a/app/test/test_kvargs.c
>> +++ b/app/test/test_kvargs.c
>> @@ -1,5 +1,6 @@
>>  /* SPDX-License-Identifier: BSD-3-Clause
>>   * Copyright 2014 6WIND S.A.
>> + * Copyright(c) 2023 HiSilicon Limited
>>   */
> 
> The test changes look good and it is always better to have more tests.
> 
> IANAL but starting the precedent of allowing adding copyright whenever a file
> is touched just leads to lots of meaningless fluff.

OK, I sent v2 which remove this line.

[PATCH 2/7] app/test: extract basic token count testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the basic token count tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 120 ++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 75 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 9aeb9aa0aa..a27b2aabfb 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -36,25 +36,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test parsing. */
-static int test_kvargs_parsing(const char *args, unsigned int n)
-{
-	struct rte_kvargs *kvlist;
-
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error: %s\n", args);
-		return -1;
-	}
-	if (kvlist->count != n) {
-		printf("invalid count value %d: %s\n", kvlist->count, args);
-		rte_kvargs_free(kvlist);
-		return -1;
-	}
-	rte_kvargs_free(kvlist);
-	return 0;
-}
-
 /* test a valid case */
 static int test_valid_kvargs(void)
 {
@@ -62,29 +43,6 @@ static int test_valid_kvargs(void)
 	const char *args;
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
-	static const struct {
-		unsigned int expected;
-		const char *input;
-	} valid_inputs[] = {
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo" },
-		{ 2, "foo=1,=2" },
-		{ 1, "foo=[1,2" },
-		{ 1, ",=" },
-		{ 1, "foo=[" },
-	};
-	unsigned int i;
-
-	/* empty args is valid */
-	args = "";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
 
 	/* first test without valid_keys */
 	args = "foo=1234,check=value0,check=value1";
@@ -128,14 +86,6 @@ static int test_valid_kvargs(void)
 		rte_kvargs_free(kvlist);
 		goto fail;
 	}
-	/* count all entries */
-	count = rte_kvargs_count(kvlist, NULL);
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(NULL)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
 	/* count all entries with key="nonexistent_key" */
 	count = rte_kvargs_count(kvlist, "nonexistent_key");
 	if (count != 0) {
@@ -190,19 +140,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	/* test using empty string (it is valid) */
-	args = "";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, NULL) != 0) {
-		printf("invalid count value\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
 	kvlist = rte_kvargs_parse(args, NULL);
@@ -210,10 +147,6 @@ static int test_valid_kvargs(void)
 		printf("rte_kvargs_parse() error\n");
 		goto fail;
 	}
-	if (rte_kvargs_count(kvlist, NULL) != 2) {
-		printf("invalid count value\n");
-		goto fail;
-	}
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		goto fail;
@@ -224,14 +157,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	valid_keys = NULL;
-
-	for (i = 0; i < RTE_DIM(valid_inputs); ++i) {
-		args = valid_inputs[i].input;
-		if (test_kvargs_parsing(args, valid_inputs[i].expected))
-			goto fail;
-	}
-
 	return 0;
 
  fail:
@@ -246,6 +171,50 @@ static int test_valid_kvargs(void)
 	return -1;
 }
 
+static int
+test_basic_token_count(void)
+{
+	static const struct {
+		unsigned int expected;
+		const char *input;
+	} valid_inputs[] = {
+		{ 3, "foo=1,check=1,check=2" },
+		{ 3, "foo=1,check,check=2"   },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo"             },
+		{ 2, "foo=1,=2"              },
+		{ 2, "foo=1,,foo=2,,"        },
+		{ 1, "foo=[1,2"              },
+		{ 1, ",="                    },
+		{ 1, "foo=["                 },
+		{ 0, ""                      },
+	};
+	struct rte_kvargs *kvlist;
+	unsigned int count;
+	const char *args;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(valid_inputs); i++) {
+		args = valid_inputs[i].input;
+		kvlist = rte_kvargs_parse(args, NULL);
+		if (kvlist == NULL) {
+			printf("rte_kvargs_parse() error: %s\n", args);
+			return -1;
+		}
+		count = rte_kvargs_count(kvlist, NULL);
+		if (count != valid_inputs[i].expected) {
+			printf("invalid count value %u (expected %u): %s\n",
+			       count, valid_inputs[i].expected, args);
+			rte_kvargs_free(kvlist);
+			return -1;
+		}
+		rte_kvargs_free(kvlist);
+	}
+
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -287,6 +256,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH 3/7] app/test: extract without keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the without keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 115 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 52 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index a27b2aabfb..7ab4becc0b 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -44,58 +44,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* first test without valid_keys */
-	args = "foo=1234,check=value0,check=value1";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() for all entries with key="check" */
-	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 2) {
-		printf("invalid count value %d after rte_kvargs_process(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = 0;
-	/* call check_handler() for all entries with key="nonexistent_key" */
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="foo" */
-	count = rte_kvargs_count(kvlist, "foo");
-	if (count != 1) {
-		printf("invalid count value %d after rte_kvargs_count(foo)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="nonexistent_key" */
-	count = rte_kvargs_count(kvlist, "nonexistent_key");
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* second test using valid_keys */
 	args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	valid_keys = valid_keys_list;
@@ -215,6 +163,68 @@ test_basic_token_count(void)
 	return 0;
 }
 
+static int
+test_parse_without_valid_keys(void)
+{
+	const char *args = "foo=1234,check=value0,check=value1";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="check" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 2) {
+		printf("invalid count value %u after rte_kvargs_process(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="nonexistent_key" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(nonexistent_key) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="foo" */
+	count = rte_kvargs_count(kvlist, "foo");
+	if (count != 1) {
+		printf("invalid count value %d after rte_kvargs_count(foo)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="nonexistent_key" */
+	count = rte_kvargs_count(kvlist, "nonexistent_key");
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -257,6 +267,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
+		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH 4/7] app/test: extract with keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the with keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 59 +++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7ab4becc0b..28b93680f0 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -44,30 +44,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* second test using valid_keys */
-	args = "foo=droids,check=value0,check=value1,check=wrong_value";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() on all entries with key="check", it
-	 * should fail as the value is not recognized by the handler */
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
-		printf("rte_kvargs_process() is success but should not\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = rte_kvargs_count(kvlist, "check");
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* third test using list as value */
 	args = "foo=[0,1],check=value2";
 	valid_keys = valid_keys_list;
@@ -225,6 +201,40 @@ test_parse_without_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_with_valid_keys(void)
+{
+	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
+	const char *valid_keys[] = { "foo", "check", NULL };
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() on all entries with key="check", it
+	 * should fail as the value is not recognized by the handler
+	 */
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
+		printf("rte_kvargs_process(check) is success but should not\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	count = rte_kvargs_count(kvlist, "check");
+	if (count != 3) {
+		printf("invalid count value %u after rte_kvargs_count(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -268,6 +278,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
+		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH 5/7] app/test: extract parse list value testcase for kvargs

[Chengwen Feng]

Extract parse list value test as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 54 +++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 28b93680f0..f4257c176f 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -41,28 +41,7 @@ static int test_valid_kvargs(void)
 {
 	struct rte_kvargs *kvlist;
 	const char *args;
-	const char *valid_keys_list[] = { "foo", "check", NULL };
-	const char **valid_keys;
-
-	/* third test using list as value */
-	args = "foo=[0,1],check=value2";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
-		printf("wrong value %s", kvlist->pairs[0].value);
-		goto fail;
-	}
-	count = kvlist->count;
-	if (count != 2) {
-		printf("invalid count value %d\n", count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
+	const char **valid_keys = NULL;
 
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
@@ -235,6 +214,36 @@ test_parse_with_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_list_value(void)
+{
+	const char *valid_keys[] = { "foo", "check", NULL };
+	const char *args = "foo=[0,1],check=value2";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
+		printf("wrong value %s", kvlist->pairs[0].value);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -279,6 +288,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH 6/7] app/test: extract parse empty elements testcase for kvargs

[Chengwen Feng]

Extract parse empty elements test as one stand-alone testcase. And
also fix the kvlist was not released when the branch fails.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 68 ++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index f4257c176f..8cd84fcec0 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -36,44 +36,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test a valid case */
-static int test_valid_kvargs(void)
-{
-	struct rte_kvargs *kvlist;
-	const char *args;
-	const char **valid_keys = NULL;
-
-	/* test using empty elements (it is valid) */
-	args = "foo=1,,check=value2,,";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "foo") != 1) {
-		printf("invalid count value for 'foo'\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "check") != 1) {
-		printf("invalid count value for 'check'\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
-	return 0;
-
- fail:
-	printf("while processing <%s>", args);
-	if (valid_keys != NULL && *valid_keys != NULL) {
-		printf(" using valid_keys=<%s", *valid_keys);
-		while (*(++valid_keys) != NULL)
-			printf(",%s", *valid_keys);
-		printf(">");
-	}
-	printf("\n");
-	return -1;
-}
-
 static int
 test_basic_token_count(void)
 {
@@ -244,6 +206,34 @@ test_parse_list_value(void)
 	return 0;
 }
 
+static int
+test_parse_empty_elements(void)
+{
+	const char *args = "foo=1,,check=value2,,";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "foo") != 1) {
+		printf("invalid count value for 'foo'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "check") != 1) {
+		printf("invalid count value for 'check'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -284,11 +274,11 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
-		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_parse_list_value),
+		TEST_CASE(test_parse_empty_elements),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH 7/7] maintainers: update for kvargs library

[Chengwen Feng]

Add myself for voluntary maintenance of the kvargs library.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4083658697..81f97a5e1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1651,6 +1651,7 @@ F: examples/cmdline/
 F: doc/guides/sample_app_ug/cmd_line.rst
 
 Key/Value parsing
+M: Chengwen Feng <fengchengwen@huawei.com>
 F: lib/kvargs/
 F: app/test/test_kvargs.c
 
-- 
2.17.1

Re: [PATCH 0/7] refactor kvargs test

[fengchengwen]

Hi Thomas,

  Could you please apply the commits 1~6 ? There commits are independent and could use for current code.

Thanks

On 2023/11/3 17:53, Chengwen Feng wrote:
> When developing patchset [1], I found the kvargs test is hard to 
> understand when tried to add some testcase.
> 
> So refactor kvargs by:
> 1. introduce UT suite framework.
> 2. extract big test_valid_kvargs() to five part.
> 
> And add myself for voluntary maintenance of the kvargs library.
> 
> Note: to ensure patch independenct, new API which in patchset [1]
> are not included in this patchset.
> 
> [1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/
> 
> Chengwen Feng (7):
>   app/test: introduce UT suite framework for kvargs
>   app/test: extract basic token count testcase for kvargs
>   app/test: extract without keys testcase for kvargs
>   app/test: extract with keys testcase for kvargs
>   app/test: extract parse list value testcase for kvargs
>   app/test: extract parse empty elements testcase for kvargs
>   maintainers: update for kvargs library
> 
>  MAINTAINERS            |   1 +
>  app/test/test_kvargs.c | 256 ++++++++++++++++++++---------------------
>  2 files changed, 128 insertions(+), 129 deletions(-)
> 

Re: [PATCH 0/7] refactor kvargs test

[Stephen Hemminger]

On Fri, 3 Nov 2023 09:53:18 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:

> When developing patchset [1], I found the kvargs test is hard to 
> understand when tried to add some testcase.
> 
> So refactor kvargs by:
> 1. introduce UT suite framework.
> 2. extract big test_valid_kvargs() to five part.
> 
> And add myself for voluntary maintenance of the kvargs library.
> 
> Note: to ensure patch independenct, new API which in patchset [1]
> are not included in this patchset.
> 
> [1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/
> 
> Chengwen Feng (7):
>   app/test: introduce UT suite framework for kvargs
>   app/test: extract basic token count testcase for kvargs
>   app/test: extract without keys testcase for kvargs
>   app/test: extract with keys testcase for kvargs
>   app/test: extract parse list value testcase for kvargs
>   app/test: extract parse empty elements testcase for kvargs
>   maintainers: update for kvargs library
> 
>  MAINTAINERS            |   1 +
>  app/test/test_kvargs.c | 256 ++++++++++++++++++++---------------------
>  2 files changed, 128 insertions(+), 129 deletions(-)
> 

Looks good and still applies clean

Series-Acked-by: Stephen Hemminger <stephen@networkplumber.org.

[PATCH v2 0/7] refactor kvargs test

[Chengwen Feng]

When developing patchset [1], I found the kvargs test is hard to
understand when tried to add some testcase.

So refactor kvargs by:
1. introduce UT suite framework.
2. extract big test_valid_kvargs() to five part.

And add myself for voluntary maintenance of the kvargs library.

Note: to ensure patch independenct, new API which in patchset [1]
are not included in this patchset.

[1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/

Chengwen Feng (7):
  app/test: introduce UT suite framework for kvargs
  app/test: extract basic token count testcase for kvargs
  app/test: extract without keys testcase for kvargs
  app/test: extract with keys testcase for kvargs
  app/test: extract parse list value testcase for kvargs
  app/test: extract parse empty elements testcase for kvargs
  maintainers: update for kvargs library

---
v2: remove copyright line and add Stephen's ack.

 MAINTAINERS            |   1 +
 app/test/test_kvargs.c | 255 ++++++++++++++++++++---------------------
 2 files changed, 127 insertions(+), 129 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] app/test: introduce UT suite framework for kvargs

[Chengwen Feng]

Introduce unit test suite framework for test_kvargs.c.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7a60cac4c1..1c46fbcaab 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -280,16 +280,21 @@ static int test_invalid_kvargs(void)
 	return -1;
 }
 
+static struct unit_test_suite kvargs_test_suite  = {
+	.suite_name = "Kvargs Unit Test Suite",
+	.setup = NULL,
+	.teardown = NULL,
+	.unit_test_cases = {
+		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_invalid_kvargs),
+		TEST_CASES_END() /**< NULL terminate unit test array */
+	}
+};
+
 static int
 test_kvargs(void)
 {
-	printf("== test valid case ==\n");
-	if (test_valid_kvargs() < 0)
-		return -1;
-	printf("== test invalid case ==\n");
-	if (test_invalid_kvargs() < 0)
-		return -1;
-	return 0;
+	return unit_test_suite_runner(&kvargs_test_suite);
 }
 
 REGISTER_FAST_TEST(kvargs_autotest, true, true, test_kvargs);
-- 
2.17.1

[PATCH v2 2/7] app/test: extract basic token count testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the basic token count tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 120 ++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 75 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 1c46fbcaab..a9808a6f87 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,25 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test parsing. */
-static int test_kvargs_parsing(const char *args, unsigned int n)
-{
-	struct rte_kvargs *kvlist;
-
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error: %s\n", args);
-		return -1;
-	}
-	if (kvlist->count != n) {
-		printf("invalid count value %d: %s\n", kvlist->count, args);
-		rte_kvargs_free(kvlist);
-		return -1;
-	}
-	rte_kvargs_free(kvlist);
-	return 0;
-}
-
 /* test a valid case */
 static int test_valid_kvargs(void)
 {
@@ -61,29 +42,6 @@ static int test_valid_kvargs(void)
 	const char *args;
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
-	static const struct {
-		unsigned int expected;
-		const char *input;
-	} valid_inputs[] = {
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo" },
-		{ 2, "foo=1,=2" },
-		{ 1, "foo=[1,2" },
-		{ 1, ",=" },
-		{ 1, "foo=[" },
-	};
-	unsigned int i;
-
-	/* empty args is valid */
-	args = "";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
 
 	/* first test without valid_keys */
 	args = "foo=1234,check=value0,check=value1";
@@ -127,14 +85,6 @@ static int test_valid_kvargs(void)
 		rte_kvargs_free(kvlist);
 		goto fail;
 	}
-	/* count all entries */
-	count = rte_kvargs_count(kvlist, NULL);
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(NULL)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
 	/* count all entries with key="nonexistent_key" */
 	count = rte_kvargs_count(kvlist, "nonexistent_key");
 	if (count != 0) {
@@ -189,19 +139,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	/* test using empty string (it is valid) */
-	args = "";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, NULL) != 0) {
-		printf("invalid count value\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
 	kvlist = rte_kvargs_parse(args, NULL);
@@ -209,10 +146,6 @@ static int test_valid_kvargs(void)
 		printf("rte_kvargs_parse() error\n");
 		goto fail;
 	}
-	if (rte_kvargs_count(kvlist, NULL) != 2) {
-		printf("invalid count value\n");
-		goto fail;
-	}
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		goto fail;
@@ -223,14 +156,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	valid_keys = NULL;
-
-	for (i = 0; i < RTE_DIM(valid_inputs); ++i) {
-		args = valid_inputs[i].input;
-		if (test_kvargs_parsing(args, valid_inputs[i].expected))
-			goto fail;
-	}
-
 	return 0;
 
  fail:
@@ -245,6 +170,50 @@ static int test_valid_kvargs(void)
 	return -1;
 }
 
+static int
+test_basic_token_count(void)
+{
+	static const struct {
+		unsigned int expected;
+		const char *input;
+	} valid_inputs[] = {
+		{ 3, "foo=1,check=1,check=2" },
+		{ 3, "foo=1,check,check=2"   },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo"             },
+		{ 2, "foo=1,=2"              },
+		{ 2, "foo=1,,foo=2,,"        },
+		{ 1, "foo=[1,2"              },
+		{ 1, ",="                    },
+		{ 1, "foo=["                 },
+		{ 0, ""                      },
+	};
+	struct rte_kvargs *kvlist;
+	unsigned int count;
+	const char *args;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(valid_inputs); i++) {
+		args = valid_inputs[i].input;
+		kvlist = rte_kvargs_parse(args, NULL);
+		if (kvlist == NULL) {
+			printf("rte_kvargs_parse() error: %s\n", args);
+			return -1;
+		}
+		count = rte_kvargs_count(kvlist, NULL);
+		if (count != valid_inputs[i].expected) {
+			printf("invalid count value %u (expected %u): %s\n",
+			       count, valid_inputs[i].expected, args);
+			rte_kvargs_free(kvlist);
+			return -1;
+		}
+		rte_kvargs_free(kvlist);
+	}
+
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -286,6 +255,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v2 3/7] app/test: extract without keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the without keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 115 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 52 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index a9808a6f87..2147080160 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,58 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* first test without valid_keys */
-	args = "foo=1234,check=value0,check=value1";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() for all entries with key="check" */
-	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 2) {
-		printf("invalid count value %d after rte_kvargs_process(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = 0;
-	/* call check_handler() for all entries with key="nonexistent_key" */
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="foo" */
-	count = rte_kvargs_count(kvlist, "foo");
-	if (count != 1) {
-		printf("invalid count value %d after rte_kvargs_count(foo)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="nonexistent_key" */
-	count = rte_kvargs_count(kvlist, "nonexistent_key");
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* second test using valid_keys */
 	args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	valid_keys = valid_keys_list;
@@ -214,6 +162,68 @@ test_basic_token_count(void)
 	return 0;
 }
 
+static int
+test_parse_without_valid_keys(void)
+{
+	const char *args = "foo=1234,check=value0,check=value1";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="check" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 2) {
+		printf("invalid count value %u after rte_kvargs_process(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="nonexistent_key" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(nonexistent_key) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="foo" */
+	count = rte_kvargs_count(kvlist, "foo");
+	if (count != 1) {
+		printf("invalid count value %d after rte_kvargs_count(foo)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="nonexistent_key" */
+	count = rte_kvargs_count(kvlist, "nonexistent_key");
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -256,6 +266,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
+		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v2 4/7] app/test: extract with keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the with keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 59 +++++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 2147080160..2869cd16c8 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,30 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* second test using valid_keys */
-	args = "foo=droids,check=value0,check=value1,check=wrong_value";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() on all entries with key="check", it
-	 * should fail as the value is not recognized by the handler */
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
-		printf("rte_kvargs_process() is success but should not\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = rte_kvargs_count(kvlist, "check");
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* third test using list as value */
 	args = "foo=[0,1],check=value2";
 	valid_keys = valid_keys_list;
@@ -224,6 +200,40 @@ test_parse_without_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_with_valid_keys(void)
+{
+	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
+	const char *valid_keys[] = { "foo", "check", NULL };
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() on all entries with key="check", it
+	 * should fail as the value is not recognized by the handler
+	 */
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
+		printf("rte_kvargs_process(check) is success but should not\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	count = rte_kvargs_count(kvlist, "check");
+	if (count != 3) {
+		printf("invalid count value %u after rte_kvargs_count(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -267,6 +277,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
+		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v2 5/7] app/test: extract parse list value testcase for kvargs

[Chengwen Feng]

Extract parse list value test as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 54 +++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 2869cd16c8..7aad68590b 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -40,28 +40,7 @@ static int test_valid_kvargs(void)
 {
 	struct rte_kvargs *kvlist;
 	const char *args;
-	const char *valid_keys_list[] = { "foo", "check", NULL };
-	const char **valid_keys;
-
-	/* third test using list as value */
-	args = "foo=[0,1],check=value2";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
-		printf("wrong value %s", kvlist->pairs[0].value);
-		goto fail;
-	}
-	count = kvlist->count;
-	if (count != 2) {
-		printf("invalid count value %d\n", count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
+	const char **valid_keys = NULL;
 
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
@@ -234,6 +213,36 @@ test_parse_with_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_list_value(void)
+{
+	const char *valid_keys[] = { "foo", "check", NULL };
+	const char *args = "foo=[0,1],check=value2";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
+		printf("wrong value %s", kvlist->pairs[0].value);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -278,6 +287,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v2 6/7] app/test: extract parse empty elements testcase for kvargs

[Chengwen Feng]

Extract parse empty elements test as one stand-alone testcase. And
also fix the kvlist was not released when the branch fails.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 68 ++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7aad68590b..d7041ba056 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,44 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test a valid case */
-static int test_valid_kvargs(void)
-{
-	struct rte_kvargs *kvlist;
-	const char *args;
-	const char **valid_keys = NULL;
-
-	/* test using empty elements (it is valid) */
-	args = "foo=1,,check=value2,,";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "foo") != 1) {
-		printf("invalid count value for 'foo'\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "check") != 1) {
-		printf("invalid count value for 'check'\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
-	return 0;
-
- fail:
-	printf("while processing <%s>", args);
-	if (valid_keys != NULL && *valid_keys != NULL) {
-		printf(" using valid_keys=<%s", *valid_keys);
-		while (*(++valid_keys) != NULL)
-			printf(",%s", *valid_keys);
-		printf(">");
-	}
-	printf("\n");
-	return -1;
-}
-
 static int
 test_basic_token_count(void)
 {
@@ -243,6 +205,34 @@ test_parse_list_value(void)
 	return 0;
 }
 
+static int
+test_parse_empty_elements(void)
+{
+	const char *args = "foo=1,,check=value2,,";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "foo") != 1) {
+		printf("invalid count value for 'foo'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "check") != 1) {
+		printf("invalid count value for 'check'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -283,11 +273,11 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
-		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_parse_list_value),
+		TEST_CASE(test_parse_empty_elements),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v2 7/7] maintainers: update for kvargs library

[Chengwen Feng]

Add myself for voluntary maintenance of the kvargs library.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d171e3d45..7e320335d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1713,6 +1713,7 @@ F: doc/guides/prog_guide/cmdline.rst
 F: doc/guides/sample_app_ug/cmd_line.rst
 
 Key/Value parsing
+M: Chengwen Feng <fengchengwen@huawei.com>
 F: lib/kvargs/
 F: app/test/test_kvargs.c
 
-- 
2.17.1

[PATCH v3 0/8] refactor kvargs test

[Chengwen Feng]

When developing patchset [1], I found the kvargs test is hard to
understand when tried to add some testcase.

So refactor kvargs by:
1. introduce UT suite framework.
2. extract big test_valid_kvargs() to five part.

And add myself for voluntary maintenance of the kvargs library.

[1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/

Chengwen Feng (8):
  app/test: introduce UT suite framework for kvargs
  app/test: extract basic token count testcase for kvargs
  app/test: extract without keys testcase for kvargs
  app/test: extract with keys testcase for kvargs
  app/test: extract parse list value testcase for kvargs
  app/test: extract parse empty elements testcase for kvargs
  app/test: add process opt testcase for kvargs
  maintainers: update for kvargs library

---
v3: add rte_kvargs_process_opt()'s testcase.
v2: remove copyright line and add Stephen's ack.

 MAINTAINERS            |   1 +
 app/test/test_kvargs.c | 326 +++++++++++++++++++++++++----------------
 2 files changed, 200 insertions(+), 127 deletions(-)

-- 
2.17.1

[PATCH v3 1/8] app/test: introduce UT suite framework for kvargs

[Chengwen Feng]

Introduce unit test suite framework for test_kvargs.c.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7a60cac4c1..1c46fbcaab 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -280,16 +280,21 @@ static int test_invalid_kvargs(void)
 	return -1;
 }
 
+static struct unit_test_suite kvargs_test_suite  = {
+	.suite_name = "Kvargs Unit Test Suite",
+	.setup = NULL,
+	.teardown = NULL,
+	.unit_test_cases = {
+		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_invalid_kvargs),
+		TEST_CASES_END() /**< NULL terminate unit test array */
+	}
+};
+
 static int
 test_kvargs(void)
 {
-	printf("== test valid case ==\n");
-	if (test_valid_kvargs() < 0)
-		return -1;
-	printf("== test invalid case ==\n");
-	if (test_invalid_kvargs() < 0)
-		return -1;
-	return 0;
+	return unit_test_suite_runner(&kvargs_test_suite);
 }
 
 REGISTER_FAST_TEST(kvargs_autotest, true, true, test_kvargs);
-- 
2.17.1

[PATCH v3 2/8] app/test: extract basic token count testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the basic token count tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 120 ++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 75 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 1c46fbcaab..a9808a6f87 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,25 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test parsing. */
-static int test_kvargs_parsing(const char *args, unsigned int n)
-{
-	struct rte_kvargs *kvlist;
-
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error: %s\n", args);
-		return -1;
-	}
-	if (kvlist->count != n) {
-		printf("invalid count value %d: %s\n", kvlist->count, args);
-		rte_kvargs_free(kvlist);
-		return -1;
-	}
-	rte_kvargs_free(kvlist);
-	return 0;
-}
-
 /* test a valid case */
 static int test_valid_kvargs(void)
 {
@@ -61,29 +42,6 @@ static int test_valid_kvargs(void)
 	const char *args;
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
-	static const struct {
-		unsigned int expected;
-		const char *input;
-	} valid_inputs[] = {
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo" },
-		{ 2, "foo=1,=2" },
-		{ 1, "foo=[1,2" },
-		{ 1, ",=" },
-		{ 1, "foo=[" },
-	};
-	unsigned int i;
-
-	/* empty args is valid */
-	args = "";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
 
 	/* first test without valid_keys */
 	args = "foo=1234,check=value0,check=value1";
@@ -127,14 +85,6 @@ static int test_valid_kvargs(void)
 		rte_kvargs_free(kvlist);
 		goto fail;
 	}
-	/* count all entries */
-	count = rte_kvargs_count(kvlist, NULL);
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(NULL)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
 	/* count all entries with key="nonexistent_key" */
 	count = rte_kvargs_count(kvlist, "nonexistent_key");
 	if (count != 0) {
@@ -189,19 +139,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	/* test using empty string (it is valid) */
-	args = "";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, NULL) != 0) {
-		printf("invalid count value\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
 	kvlist = rte_kvargs_parse(args, NULL);
@@ -209,10 +146,6 @@ static int test_valid_kvargs(void)
 		printf("rte_kvargs_parse() error\n");
 		goto fail;
 	}
-	if (rte_kvargs_count(kvlist, NULL) != 2) {
-		printf("invalid count value\n");
-		goto fail;
-	}
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		goto fail;
@@ -223,14 +156,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	valid_keys = NULL;
-
-	for (i = 0; i < RTE_DIM(valid_inputs); ++i) {
-		args = valid_inputs[i].input;
-		if (test_kvargs_parsing(args, valid_inputs[i].expected))
-			goto fail;
-	}
-
 	return 0;
 
  fail:
@@ -245,6 +170,50 @@ static int test_valid_kvargs(void)
 	return -1;
 }
 
+static int
+test_basic_token_count(void)
+{
+	static const struct {
+		unsigned int expected;
+		const char *input;
+	} valid_inputs[] = {
+		{ 3, "foo=1,check=1,check=2" },
+		{ 3, "foo=1,check,check=2"   },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo"             },
+		{ 2, "foo=1,=2"              },
+		{ 2, "foo=1,,foo=2,,"        },
+		{ 1, "foo=[1,2"              },
+		{ 1, ",="                    },
+		{ 1, "foo=["                 },
+		{ 0, ""                      },
+	};
+	struct rte_kvargs *kvlist;
+	unsigned int count;
+	const char *args;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(valid_inputs); i++) {
+		args = valid_inputs[i].input;
+		kvlist = rte_kvargs_parse(args, NULL);
+		if (kvlist == NULL) {
+			printf("rte_kvargs_parse() error: %s\n", args);
+			return -1;
+		}
+		count = rte_kvargs_count(kvlist, NULL);
+		if (count != valid_inputs[i].expected) {
+			printf("invalid count value %u (expected %u): %s\n",
+			       count, valid_inputs[i].expected, args);
+			rte_kvargs_free(kvlist);
+			return -1;
+		}
+		rte_kvargs_free(kvlist);
+	}
+
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -286,6 +255,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 3/8] app/test: extract without keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the without keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 115 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 52 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index a9808a6f87..2147080160 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,58 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* first test without valid_keys */
-	args = "foo=1234,check=value0,check=value1";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() for all entries with key="check" */
-	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 2) {
-		printf("invalid count value %d after rte_kvargs_process(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = 0;
-	/* call check_handler() for all entries with key="nonexistent_key" */
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="foo" */
-	count = rte_kvargs_count(kvlist, "foo");
-	if (count != 1) {
-		printf("invalid count value %d after rte_kvargs_count(foo)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="nonexistent_key" */
-	count = rte_kvargs_count(kvlist, "nonexistent_key");
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* second test using valid_keys */
 	args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	valid_keys = valid_keys_list;
@@ -214,6 +162,68 @@ test_basic_token_count(void)
 	return 0;
 }
 
+static int
+test_parse_without_valid_keys(void)
+{
+	const char *args = "foo=1234,check=value0,check=value1";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="check" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 2) {
+		printf("invalid count value %u after rte_kvargs_process(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="nonexistent_key" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(nonexistent_key) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="foo" */
+	count = rte_kvargs_count(kvlist, "foo");
+	if (count != 1) {
+		printf("invalid count value %d after rte_kvargs_count(foo)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="nonexistent_key" */
+	count = rte_kvargs_count(kvlist, "nonexistent_key");
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -256,6 +266,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
+		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 4/8] app/test: extract with keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the with keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 60 +++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 2147080160..8b53d6d585 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,30 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* second test using valid_keys */
-	args = "foo=droids,check=value0,check=value1,check=wrong_value";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() on all entries with key="check", it
-	 * should fail as the value is not recognized by the handler */
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
-		printf("rte_kvargs_process() is success but should not\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = rte_kvargs_count(kvlist, "check");
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* third test using list as value */
 	args = "foo=[0,1],check=value2";
 	valid_keys = valid_keys_list;
@@ -224,6 +200,41 @@ test_parse_without_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_with_valid_keys(void)
+{
+	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
+	const char *valid_keys[] = { "foo", "check", NULL };
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() on all entries with key="check", it
+	 * should fail as the value is not recognized by the handler
+	 */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
+		printf("rte_kvargs_process(check) is success but should not\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	count = rte_kvargs_count(kvlist, "check");
+	if (count != 3) {
+		printf("invalid count value %u after rte_kvargs_count(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -267,6 +278,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
+		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 5/8] app/test: extract parse list value testcase for kvargs

[Chengwen Feng]

Extract parse list value test as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 54 +++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 8b53d6d585..f8d668f2cc 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -40,28 +40,7 @@ static int test_valid_kvargs(void)
 {
 	struct rte_kvargs *kvlist;
 	const char *args;
-	const char *valid_keys_list[] = { "foo", "check", NULL };
-	const char **valid_keys;
-
-	/* third test using list as value */
-	args = "foo=[0,1],check=value2";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
-		printf("wrong value %s", kvlist->pairs[0].value);
-		goto fail;
-	}
-	count = kvlist->count;
-	if (count != 2) {
-		printf("invalid count value %d\n", count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
+	const char **valid_keys = NULL;
 
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
@@ -235,6 +214,36 @@ test_parse_with_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_list_value(void)
+{
+	const char *valid_keys[] = { "foo", "check", NULL };
+	const char *args = "foo=[0,1],check=value2";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
+		printf("wrong value %s", kvlist->pairs[0].value);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -279,6 +288,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 6/8] app/test: extract parse empty elements testcase for kvargs

[Chengwen Feng]

Extract parse empty elements test as one stand-alone testcase. And
also fix the kvlist was not released when the branch fails.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 68 ++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index f8d668f2cc..d1e668ac4a 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,44 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test a valid case */
-static int test_valid_kvargs(void)
-{
-	struct rte_kvargs *kvlist;
-	const char *args;
-	const char **valid_keys = NULL;
-
-	/* test using empty elements (it is valid) */
-	args = "foo=1,,check=value2,,";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "foo") != 1) {
-		printf("invalid count value for 'foo'\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "check") != 1) {
-		printf("invalid count value for 'check'\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
-	return 0;
-
- fail:
-	printf("while processing <%s>", args);
-	if (valid_keys != NULL && *valid_keys != NULL) {
-		printf(" using valid_keys=<%s", *valid_keys);
-		while (*(++valid_keys) != NULL)
-			printf(",%s", *valid_keys);
-		printf(">");
-	}
-	printf("\n");
-	return -1;
-}
-
 static int
 test_basic_token_count(void)
 {
@@ -244,6 +206,34 @@ test_parse_list_value(void)
 	return 0;
 }
 
+static int
+test_parse_empty_elements(void)
+{
+	const char *args = "foo=1,,check=value2,,";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "foo") != 1) {
+		printf("invalid count value for 'foo'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "check") != 1) {
+		printf("invalid count value for 'check'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -284,11 +274,11 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
-		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_parse_list_value),
+		TEST_CASE(test_parse_empty_elements),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 7/8] app/test: add process opt testcase for kvargs

[Chengwen Feng]

This commit adds rte_kvargs_process_opt() API's testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 106 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 16 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index d1e668ac4a..43bb7a0243 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -11,14 +11,20 @@
 
 #include "test.h"
 
+typedef int (*f_kvargs_process)(const struct rte_kvargs *kvlist,
+				const char *key_match, arg_handler_t handler,
+				void *opaque_arg);
+
+static bool use_kvargs_process_opt[] = { false, true };
+
 /* incremented in handler, to check it is properly called once per
  * key/value association */
 static unsigned count;
 
 /* this handler increment the "count" variable at each call and check
  * that the key is "check" and the value is "value%d" */
-static int check_handler(const char *key, const char *value,
-	__rte_unused void *opaque)
+static int
+check_handler(const char *key, const char *value, __rte_unused void *opaque)
 {
 	char buf[16];
 
@@ -35,6 +41,18 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
+static int
+check_only_handler(const char *key, const char *value, __rte_unused void *opaque)
+{
+	if (strcmp(key, "check"))
+		return -1;
+
+	if (value != NULL)
+		return -1;
+
+	return 0;
+}
+
 static int
 test_basic_token_count(void)
 {
@@ -80,8 +98,11 @@ test_basic_token_count(void)
 }
 
 static int
-test_parse_without_valid_keys(void)
+test_parse_without_valid_keys(const void *params)
 {
+	const bool use_opt = *(const bool *)params;
+	f_kvargs_process proc_func = use_opt ? rte_kvargs_process_opt : rte_kvargs_process;
+	const char *proc_name = use_opt ? "rte_kvargs_process_opt" : "rte_kvargs_process";
 	const char *args = "foo=1234,check=value0,check=value1";
 	struct rte_kvargs *kvlist;
 
@@ -93,28 +114,28 @@ test_parse_without_valid_keys(void)
 
 	/* call check_handler() for all entries with key="check" */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process(check) error\n");
+	if (proc_func(kvlist, "check", check_handler, NULL) < 0) {
+		printf("%s(check) error\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 	if (count != 2) {
-		printf("invalid count value %u after rte_kvargs_process(check)\n",
-			count);
+		printf("invalid count value %u after %s(check)\n",
+			count, proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 
 	/* call check_handler() for all entries with key="nonexistent_key" */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process(nonexistent_key) error\n");
+	if (proc_func(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("%s(nonexistent_key) error\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
+		printf("invalid count value %d after %s(nonexistent_key)\n",
+			count, proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
@@ -142,8 +163,11 @@ test_parse_without_valid_keys(void)
 }
 
 static int
-test_parse_with_valid_keys(void)
+test_parse_with_valid_keys(const void *params)
 {
+	const bool use_opt = *(const bool *)params;
+	f_kvargs_process proc_func = use_opt ? rte_kvargs_process_opt : rte_kvargs_process;
+	const char *proc_name = use_opt ? "rte_kvargs_process_opt" : "rte_kvargs_process";
 	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	const char *valid_keys[] = { "foo", "check", NULL };
 	struct rte_kvargs *kvlist;
@@ -158,8 +182,8 @@ test_parse_with_valid_keys(void)
 	 * should fail as the value is not recognized by the handler
 	 */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
-		printf("rte_kvargs_process(check) is success but should not\n");
+	if (proc_func(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
+		printf("%s(check) is success but should not\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
@@ -218,6 +242,13 @@ test_parse_empty_elements(void)
 		return -1;
 	}
 
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		rte_kvargs_free(kvlist);
@@ -234,6 +265,34 @@ test_parse_empty_elements(void)
 	return 0;
 }
 
+static int
+test_parse_with_only_key(void)
+{
+	const char *args = "foo,check";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_process(kvlist, "check", check_only_handler, NULL) == 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_process_opt(kvlist, "check", check_only_handler, NULL) != 0) {
+		printf("rte_kvargs_process_opt(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -275,10 +334,25 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_basic_token_count),
-		TEST_CASE(test_parse_without_valid_keys),
-		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_without_valid_keys_no_opt",
+					  NULL, NULL,
+					  test_parse_without_valid_keys,
+					  &use_kvargs_process_opt[0]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_without_valid_keys_with_opt",
+					  NULL, NULL,
+					  test_parse_without_valid_keys,
+					  &use_kvargs_process_opt[1]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_with_valid_keys_no_opt",
+					  NULL, NULL,
+					  test_parse_with_valid_keys,
+					  &use_kvargs_process_opt[0]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_with_valid_keys_with_opt",
+					  NULL, NULL,
+					  test_parse_with_valid_keys,
+					  &use_kvargs_process_opt[1]),
 		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_parse_empty_elements),
+		TEST_CASE(test_parse_with_only_key),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v3 8/8] maintainers: update for kvargs library

[Chengwen Feng]

Add myself for voluntary maintenance of the kvargs library.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cd78bc7db1..f0bcebc651 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1718,6 +1718,7 @@ F: doc/guides/prog_guide/cmdline.rst
 F: doc/guides/sample_app_ug/cmd_line.rst
 
 Key/Value parsing
+M: Chengwen Feng <fengchengwen@huawei.com>
 F: lib/kvargs/
 F: app/test/test_kvargs.c
 
-- 
2.17.1

[PATCH v4 0/7] refactor kvargs test

[Chengwen Feng]

When developing patchset [1], I found the kvargs test is hard to
understand when tried to add some testcase.

So refactor kvargs by:
1. introduce UT suite framework.
2. extract big test_valid_kvargs() to five part.
3. add new introduced rte_kvargs_process_opt() UT.

[1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/

Chengwen Feng (7):
  app/test: introduce UT suite framework for kvargs
  app/test: extract basic token count testcase for kvargs
  app/test: extract without keys testcase for kvargs
  app/test: extract with keys testcase for kvargs
  app/test: extract parse list value testcase for kvargs
  app/test: extract parse empty elements testcase for kvargs
  app/test: add process opt testcase for kvargs

---
v4: remove maintainer commit.
v3: add rte_kvargs_process_opt()'s testcase.
v2: remove copyright line and add Stephen's ack.

 app/test/test_kvargs.c | 326 +++++++++++++++++++++++++----------------
 1 file changed, 199 insertions(+), 127 deletions(-)

-- 
2.17.1

[PATCH v4 1/7] app/test: introduce UT suite framework for kvargs

[Chengwen Feng]

Introduce unit test suite framework for test_kvargs.c.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 7a60cac4c1..1c46fbcaab 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -280,16 +280,21 @@ static int test_invalid_kvargs(void)
 	return -1;
 }
 
+static struct unit_test_suite kvargs_test_suite  = {
+	.suite_name = "Kvargs Unit Test Suite",
+	.setup = NULL,
+	.teardown = NULL,
+	.unit_test_cases = {
+		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_invalid_kvargs),
+		TEST_CASES_END() /**< NULL terminate unit test array */
+	}
+};
+
 static int
 test_kvargs(void)
 {
-	printf("== test valid case ==\n");
-	if (test_valid_kvargs() < 0)
-		return -1;
-	printf("== test invalid case ==\n");
-	if (test_invalid_kvargs() < 0)
-		return -1;
-	return 0;
+	return unit_test_suite_runner(&kvargs_test_suite);
 }
 
 REGISTER_FAST_TEST(kvargs_autotest, true, true, test_kvargs);
-- 
2.17.1

[PATCH v4 2/7] app/test: extract basic token count testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the basic token count tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 120 ++++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 75 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 1c46fbcaab..a9808a6f87 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,25 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test parsing. */
-static int test_kvargs_parsing(const char *args, unsigned int n)
-{
-	struct rte_kvargs *kvlist;
-
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error: %s\n", args);
-		return -1;
-	}
-	if (kvlist->count != n) {
-		printf("invalid count value %d: %s\n", kvlist->count, args);
-		rte_kvargs_free(kvlist);
-		return -1;
-	}
-	rte_kvargs_free(kvlist);
-	return 0;
-}
-
 /* test a valid case */
 static int test_valid_kvargs(void)
 {
@@ -61,29 +42,6 @@ static int test_valid_kvargs(void)
 	const char *args;
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
-	static const struct {
-		unsigned int expected;
-		const char *input;
-	} valid_inputs[] = {
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo=" },
-		{ 2, "foo=1,foo" },
-		{ 2, "foo=1,=2" },
-		{ 1, "foo=[1,2" },
-		{ 1, ",=" },
-		{ 1, "foo=[" },
-	};
-	unsigned int i;
-
-	/* empty args is valid */
-	args = "";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
 
 	/* first test without valid_keys */
 	args = "foo=1234,check=value0,check=value1";
@@ -127,14 +85,6 @@ static int test_valid_kvargs(void)
 		rte_kvargs_free(kvlist);
 		goto fail;
 	}
-	/* count all entries */
-	count = rte_kvargs_count(kvlist, NULL);
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(NULL)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
 	/* count all entries with key="nonexistent_key" */
 	count = rte_kvargs_count(kvlist, "nonexistent_key");
 	if (count != 0) {
@@ -189,19 +139,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	/* test using empty string (it is valid) */
-	args = "";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, NULL) != 0) {
-		printf("invalid count value\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
 	kvlist = rte_kvargs_parse(args, NULL);
@@ -209,10 +146,6 @@ static int test_valid_kvargs(void)
 		printf("rte_kvargs_parse() error\n");
 		goto fail;
 	}
-	if (rte_kvargs_count(kvlist, NULL) != 2) {
-		printf("invalid count value\n");
-		goto fail;
-	}
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		goto fail;
@@ -223,14 +156,6 @@ static int test_valid_kvargs(void)
 	}
 	rte_kvargs_free(kvlist);
 
-	valid_keys = NULL;
-
-	for (i = 0; i < RTE_DIM(valid_inputs); ++i) {
-		args = valid_inputs[i].input;
-		if (test_kvargs_parsing(args, valid_inputs[i].expected))
-			goto fail;
-	}
-
 	return 0;
 
  fail:
@@ -245,6 +170,50 @@ static int test_valid_kvargs(void)
 	return -1;
 }
 
+static int
+test_basic_token_count(void)
+{
+	static const struct {
+		unsigned int expected;
+		const char *input;
+	} valid_inputs[] = {
+		{ 3, "foo=1,check=1,check=2" },
+		{ 3, "foo=1,check,check=2"   },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo="            },
+		{ 2, "foo=1,foo"             },
+		{ 2, "foo=1,=2"              },
+		{ 2, "foo=1,,foo=2,,"        },
+		{ 1, "foo=[1,2"              },
+		{ 1, ",="                    },
+		{ 1, "foo=["                 },
+		{ 0, ""                      },
+	};
+	struct rte_kvargs *kvlist;
+	unsigned int count;
+	const char *args;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(valid_inputs); i++) {
+		args = valid_inputs[i].input;
+		kvlist = rte_kvargs_parse(args, NULL);
+		if (kvlist == NULL) {
+			printf("rte_kvargs_parse() error: %s\n", args);
+			return -1;
+		}
+		count = rte_kvargs_count(kvlist, NULL);
+		if (count != valid_inputs[i].expected) {
+			printf("invalid count value %u (expected %u): %s\n",
+			       count, valid_inputs[i].expected, args);
+			rte_kvargs_free(kvlist);
+			return -1;
+		}
+		rte_kvargs_free(kvlist);
+	}
+
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -286,6 +255,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
+		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v4 3/7] app/test: extract without keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the without keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 115 ++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 52 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index a9808a6f87..2147080160 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,58 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* first test without valid_keys */
-	args = "foo=1234,check=value0,check=value1";
-	valid_keys = NULL;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() for all entries with key="check" */
-	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 2) {
-		printf("invalid count value %d after rte_kvargs_process(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = 0;
-	/* call check_handler() for all entries with key="nonexistent_key" */
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process() error\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="foo" */
-	count = rte_kvargs_count(kvlist, "foo");
-	if (count != 1) {
-		printf("invalid count value %d after rte_kvargs_count(foo)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	/* count all entries with key="nonexistent_key" */
-	count = rte_kvargs_count(kvlist, "nonexistent_key");
-	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* second test using valid_keys */
 	args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	valid_keys = valid_keys_list;
@@ -214,6 +162,68 @@ test_basic_token_count(void)
 	return 0;
 }
 
+static int
+test_parse_without_valid_keys(void)
+{
+	const char *args = "foo=1234,check=value0,check=value1";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="check" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 2) {
+		printf("invalid count value %u after rte_kvargs_process(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* call check_handler() for all entries with key="nonexistent_key" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process(nonexistent_key) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="foo" */
+	count = rte_kvargs_count(kvlist, "foo");
+	if (count != 1) {
+		printf("invalid count value %d after rte_kvargs_count(foo)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	/* count all entries with key="nonexistent_key" */
+	count = rte_kvargs_count(kvlist, "nonexistent_key");
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_count(nonexistent_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -256,6 +266,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
+		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v4 4/7] app/test: extract with keys testcase for kvargs

[Chengwen Feng]

The test_valid_kvargs() function is too long to understand, extract
the with keys tests as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 60 +++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 2147080160..8b53d6d585 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -43,30 +43,6 @@ static int test_valid_kvargs(void)
 	const char *valid_keys_list[] = { "foo", "check", NULL };
 	const char **valid_keys;
 
-	/* second test using valid_keys */
-	args = "foo=droids,check=value0,check=value1,check=wrong_value";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error");
-		goto fail;
-	}
-	/* call check_handler() on all entries with key="check", it
-	 * should fail as the value is not recognized by the handler */
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
-		printf("rte_kvargs_process() is success but should not\n");
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	count = rte_kvargs_count(kvlist, "check");
-	if (count != 3) {
-		printf("invalid count value %d after rte_kvargs_count(check)\n",
-			count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
 	/* third test using list as value */
 	args = "foo=[0,1],check=value2";
 	valid_keys = valid_keys_list;
@@ -224,6 +200,41 @@ test_parse_without_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_with_valid_keys(void)
+{
+	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
+	const char *valid_keys[] = { "foo", "check", NULL };
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	/* call check_handler() on all entries with key="check", it
+	 * should fail as the value is not recognized by the handler
+	 */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
+		printf("rte_kvargs_process(check) is success but should not\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	count = rte_kvargs_count(kvlist, "check");
+	if (count != 3) {
+		printf("invalid count value %u after rte_kvargs_count(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -267,6 +278,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
+		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v4 5/7] app/test: extract parse list value testcase for kvargs

[Chengwen Feng]

Extract parse list value test as one stand-alone testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 54 +++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index 8b53d6d585..f8d668f2cc 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -40,28 +40,7 @@ static int test_valid_kvargs(void)
 {
 	struct rte_kvargs *kvlist;
 	const char *args;
-	const char *valid_keys_list[] = { "foo", "check", NULL };
-	const char **valid_keys;
-
-	/* third test using list as value */
-	args = "foo=[0,1],check=value2";
-	valid_keys = valid_keys_list;
-	kvlist = rte_kvargs_parse(args, valid_keys);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
-		printf("wrong value %s", kvlist->pairs[0].value);
-		goto fail;
-	}
-	count = kvlist->count;
-	if (count != 2) {
-		printf("invalid count value %d\n", count);
-		rte_kvargs_free(kvlist);
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
+	const char **valid_keys = NULL;
 
 	/* test using empty elements (it is valid) */
 	args = "foo=1,,check=value2,,";
@@ -235,6 +214,36 @@ test_parse_with_valid_keys(void)
 	return 0;
 }
 
+static int
+test_parse_list_value(void)
+{
+	const char *valid_keys[] = { "foo", "check", NULL };
+	const char *args = "foo=[0,1],check=value2";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (strcmp(kvlist->pairs[0].value, "[0,1]") != 0) {
+		printf("wrong value %s", kvlist->pairs[0].value);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -279,6 +288,7 @@ static struct unit_test_suite kvargs_test_suite  = {
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v4 6/7] app/test: extract parse empty elements testcase for kvargs

[Chengwen Feng]

Extract parse empty elements test as one stand-alone testcase. And
also fix the kvlist was not released when the branch fails.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_kvargs.c | 68 ++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index f8d668f2cc..d1e668ac4a 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -35,44 +35,6 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
-/* test a valid case */
-static int test_valid_kvargs(void)
-{
-	struct rte_kvargs *kvlist;
-	const char *args;
-	const char **valid_keys = NULL;
-
-	/* test using empty elements (it is valid) */
-	args = "foo=1,,check=value2,,";
-	kvlist = rte_kvargs_parse(args, NULL);
-	if (kvlist == NULL) {
-		printf("rte_kvargs_parse() error\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "foo") != 1) {
-		printf("invalid count value for 'foo'\n");
-		goto fail;
-	}
-	if (rte_kvargs_count(kvlist, "check") != 1) {
-		printf("invalid count value for 'check'\n");
-		goto fail;
-	}
-	rte_kvargs_free(kvlist);
-
-	return 0;
-
- fail:
-	printf("while processing <%s>", args);
-	if (valid_keys != NULL && *valid_keys != NULL) {
-		printf(" using valid_keys=<%s", *valid_keys);
-		while (*(++valid_keys) != NULL)
-			printf(",%s", *valid_keys);
-		printf(">");
-	}
-	printf("\n");
-	return -1;
-}
-
 static int
 test_basic_token_count(void)
 {
@@ -244,6 +206,34 @@ test_parse_list_value(void)
 	return 0;
 }
 
+static int
+test_parse_empty_elements(void)
+{
+	const char *args = "foo=1,,check=value2,,";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "foo") != 1) {
+		printf("invalid count value for 'foo'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_count(kvlist, "check") != 1) {
+		printf("invalid count value for 'check'\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -284,11 +274,11 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.setup = NULL,
 	.teardown = NULL,
 	.unit_test_cases = {
-		TEST_CASE(test_valid_kvargs),
 		TEST_CASE(test_basic_token_count),
 		TEST_CASE(test_parse_without_valid_keys),
 		TEST_CASE(test_parse_with_valid_keys),
 		TEST_CASE(test_parse_list_value),
+		TEST_CASE(test_parse_empty_elements),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

[PATCH v4 7/7] app/test: add process opt testcase for kvargs

[Chengwen Feng]

This commit adds rte_kvargs_process_opt() API's testcase.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 app/test/test_kvargs.c | 106 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 16 deletions(-)

diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
index d1e668ac4a..43bb7a0243 100644
--- a/app/test/test_kvargs.c
+++ b/app/test/test_kvargs.c
@@ -11,14 +11,20 @@
 
 #include "test.h"
 
+typedef int (*f_kvargs_process)(const struct rte_kvargs *kvlist,
+				const char *key_match, arg_handler_t handler,
+				void *opaque_arg);
+
+static bool use_kvargs_process_opt[] = { false, true };
+
 /* incremented in handler, to check it is properly called once per
  * key/value association */
 static unsigned count;
 
 /* this handler increment the "count" variable at each call and check
  * that the key is "check" and the value is "value%d" */
-static int check_handler(const char *key, const char *value,
-	__rte_unused void *opaque)
+static int
+check_handler(const char *key, const char *value, __rte_unused void *opaque)
 {
 	char buf[16];
 
@@ -35,6 +41,18 @@ static int check_handler(const char *key, const char *value,
 	return 0;
 }
 
+static int
+check_only_handler(const char *key, const char *value, __rte_unused void *opaque)
+{
+	if (strcmp(key, "check"))
+		return -1;
+
+	if (value != NULL)
+		return -1;
+
+	return 0;
+}
+
 static int
 test_basic_token_count(void)
 {
@@ -80,8 +98,11 @@ test_basic_token_count(void)
 }
 
 static int
-test_parse_without_valid_keys(void)
+test_parse_without_valid_keys(const void *params)
 {
+	const bool use_opt = *(const bool *)params;
+	f_kvargs_process proc_func = use_opt ? rte_kvargs_process_opt : rte_kvargs_process;
+	const char *proc_name = use_opt ? "rte_kvargs_process_opt" : "rte_kvargs_process";
 	const char *args = "foo=1234,check=value0,check=value1";
 	struct rte_kvargs *kvlist;
 
@@ -93,28 +114,28 @@ test_parse_without_valid_keys(void)
 
 	/* call check_handler() for all entries with key="check" */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process(check) error\n");
+	if (proc_func(kvlist, "check", check_handler, NULL) < 0) {
+		printf("%s(check) error\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 	if (count != 2) {
-		printf("invalid count value %u after rte_kvargs_process(check)\n",
-			count);
+		printf("invalid count value %u after %s(check)\n",
+			count, proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 
 	/* call check_handler() for all entries with key="nonexistent_key" */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
-		printf("rte_kvargs_process(nonexistent_key) error\n");
+	if (proc_func(kvlist, "nonexistent_key", check_handler, NULL) < 0) {
+		printf("%s(nonexistent_key) error\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
 	if (count != 0) {
-		printf("invalid count value %d after rte_kvargs_process(nonexistent_key)\n",
-			count);
+		printf("invalid count value %d after %s(nonexistent_key)\n",
+			count, proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
@@ -142,8 +163,11 @@ test_parse_without_valid_keys(void)
 }
 
 static int
-test_parse_with_valid_keys(void)
+test_parse_with_valid_keys(const void *params)
 {
+	const bool use_opt = *(const bool *)params;
+	f_kvargs_process proc_func = use_opt ? rte_kvargs_process_opt : rte_kvargs_process;
+	const char *proc_name = use_opt ? "rte_kvargs_process_opt" : "rte_kvargs_process";
 	const char *args = "foo=droids,check=value0,check=value1,check=wrong_value";
 	const char *valid_keys[] = { "foo", "check", NULL };
 	struct rte_kvargs *kvlist;
@@ -158,8 +182,8 @@ test_parse_with_valid_keys(void)
 	 * should fail as the value is not recognized by the handler
 	 */
 	count = 0;
-	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
-		printf("rte_kvargs_process(check) is success but should not\n");
+	if (proc_func(kvlist, "check", check_handler, NULL) == 0 || count != 2) {
+		printf("%s(check) is success but should not\n", proc_name);
 		rte_kvargs_free(kvlist);
 		return -1;
 	}
@@ -218,6 +242,13 @@ test_parse_empty_elements(void)
 		return -1;
 	}
 
+	count = kvlist->count;
+	if (count != 2) {
+		printf("invalid count value %u\n", count);
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
 	if (rte_kvargs_count(kvlist, "foo") != 1) {
 		printf("invalid count value for 'foo'\n");
 		rte_kvargs_free(kvlist);
@@ -234,6 +265,34 @@ test_parse_empty_elements(void)
 	return 0;
 }
 
+static int
+test_parse_with_only_key(void)
+{
+	const char *args = "foo,check";
+	struct rte_kvargs *kvlist;
+
+	kvlist = rte_kvargs_parse(args, NULL);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error\n");
+		return -1;
+	}
+
+	if (rte_kvargs_process(kvlist, "check", check_only_handler, NULL) == 0) {
+		printf("rte_kvargs_process(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	if (rte_kvargs_process_opt(kvlist, "check", check_only_handler, NULL) != 0) {
+		printf("rte_kvargs_process_opt(check) error\n");
+		rte_kvargs_free(kvlist);
+		return -1;
+	}
+
+	rte_kvargs_free(kvlist);
+	return 0;
+}
+
 /* test several error cases */
 static int test_invalid_kvargs(void)
 {
@@ -275,10 +334,25 @@ static struct unit_test_suite kvargs_test_suite  = {
 	.teardown = NULL,
 	.unit_test_cases = {
 		TEST_CASE(test_basic_token_count),
-		TEST_CASE(test_parse_without_valid_keys),
-		TEST_CASE(test_parse_with_valid_keys),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_without_valid_keys_no_opt",
+					  NULL, NULL,
+					  test_parse_without_valid_keys,
+					  &use_kvargs_process_opt[0]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_without_valid_keys_with_opt",
+					  NULL, NULL,
+					  test_parse_without_valid_keys,
+					  &use_kvargs_process_opt[1]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_with_valid_keys_no_opt",
+					  NULL, NULL,
+					  test_parse_with_valid_keys,
+					  &use_kvargs_process_opt[0]),
+		TEST_CASE_NAMED_WITH_DATA("test_parse_with_valid_keys_with_opt",
+					  NULL, NULL,
+					  test_parse_with_valid_keys,
+					  &use_kvargs_process_opt[1]),
 		TEST_CASE(test_parse_list_value),
 		TEST_CASE(test_parse_empty_elements),
+		TEST_CASE(test_parse_with_only_key),
 		TEST_CASE(test_invalid_kvargs),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
-- 
2.17.1

Re: [PATCH v4 0/7] refactor kvargs test

[lihuisong (C)]

For this series,
Acked-by: Huisong Li <lihuisong@huawei.com>
在 2024/10/30 16:54, Chengwen Feng 写道:
> When developing patchset [1], I found the kvargs test is hard to
> understand when tried to add some testcase.
>
> So refactor kvargs by:
> 1. introduce UT suite framework.
> 2. extract big test_valid_kvargs() to five part.
> 3. add new introduced rte_kvargs_process_opt() UT.
>
> [1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/
>
> Chengwen Feng (7):
>    app/test: introduce UT suite framework for kvargs
>    app/test: extract basic token count testcase for kvargs
>    app/test: extract without keys testcase for kvargs
>    app/test: extract with keys testcase for kvargs
>    app/test: extract parse list value testcase for kvargs
>    app/test: extract parse empty elements testcase for kvargs
>    app/test: add process opt testcase for kvargs
>
> ---
> v4: remove maintainer commit.
> v3: add rte_kvargs_process_opt()'s testcase.
> v2: remove copyright line and add Stephen's ack.
>
>   app/test/test_kvargs.c | 326 +++++++++++++++++++++++++----------------
>   1 file changed, 199 insertions(+), 127 deletions(-)
>

Re: [PATCH v4 0/7] refactor kvargs test

[Thomas Monjalon]

01/11/2024 03:26, lihuisong (C):
> For this series,
> Acked-by: Huisong Li <lihuisong@huawei.com>
> 在 2024/10/30 16:54, Chengwen Feng 写道:
> > When developing patchset [1], I found the kvargs test is hard to
> > understand when tried to add some testcase.
> >
> > So refactor kvargs by:
> > 1. introduce UT suite framework.
> > 2. extract big test_valid_kvargs() to five part.
> > 3. add new introduced rte_kvargs_process_opt() UT.
> >
> > [1] https://patchwork.dpdk.org/project/dpdk/cover/20231103073811.13196-1-fengchengwen@huawei.com/
> >
> > Chengwen Feng (7):
> >    app/test: introduce UT suite framework for kvargs
> >    app/test: extract basic token count testcase for kvargs
> >    app/test: extract without keys testcase for kvargs
> >    app/test: extract with keys testcase for kvargs
> >    app/test: extract parse list value testcase for kvargs
> >    app/test: extract parse empty elements testcase for kvargs
> >    app/test: add process opt testcase for kvargs

Applied, thanks.