DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/7] refactor kvargs test
@ 2023-11-03  9:53 Chengwen Feng
  2023-11-03  9:53 ` [PATCH 1/7] app/test: introduce UT suite framework for kvargs Chengwen Feng
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] app/test: introduce UT suite framework for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 2/7] app/test: extract basic token count testcase " Chengwen Feng
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/7] app/test: extract basic token count testcase for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
  2023-11-03  9:53 ` [PATCH 1/7] app/test: introduce UT suite framework for kvargs Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 3/7] app/test: extract without keys " Chengwen Feng
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/7] app/test: extract without keys testcase for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
  2023-11-03  9:53 ` [PATCH 1/7] app/test: introduce UT suite framework for kvargs Chengwen Feng
  2023-11-03  9:53 ` [PATCH 2/7] app/test: extract basic token count testcase " Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 4/7] app/test: extract with " Chengwen Feng
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/7] app/test: extract with keys testcase for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
                   ` (2 preceding siblings ...)
  2023-11-03  9:53 ` [PATCH 3/7] app/test: extract without keys " Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 5/7] app/test: extract parse list value " Chengwen Feng
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/7] app/test: extract parse list value testcase for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
                   ` (3 preceding siblings ...)
  2023-11-03  9:53 ` [PATCH 4/7] app/test: extract with " Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 6/7] app/test: extract parse empty elements " Chengwen Feng
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6/7] app/test: extract parse empty elements testcase for kvargs
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
                   ` (4 preceding siblings ...)
  2023-11-03  9:53 ` [PATCH 5/7] app/test: extract parse list value " Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-11-03  9:53 ` [PATCH 7/7] maintainers: update for kvargs library Chengwen Feng
  2023-12-05  2:55 ` [PATCH 0/7] refactor kvargs test fengchengwen
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 7/7] maintainers: update for kvargs library
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
                   ` (5 preceding siblings ...)
  2023-11-03  9:53 ` [PATCH 6/7] app/test: extract parse empty elements " Chengwen Feng
@ 2023-11-03  9:53 ` Chengwen Feng
  2023-12-05  2:55 ` [PATCH 0/7] refactor kvargs test fengchengwen
  7 siblings, 0 replies; 9+ messages in thread
From: Chengwen Feng @ 2023-11-03  9:53 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/7] refactor kvargs test
  2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
                   ` (6 preceding siblings ...)
  2023-11-03  9:53 ` [PATCH 7/7] maintainers: update for kvargs library Chengwen Feng
@ 2023-12-05  2:55 ` fengchengwen
  7 siblings, 0 replies; 9+ messages in thread
From: fengchengwen @ 2023-12-05  2:55 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

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(-)
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-12-05  2:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-03  9:53 [PATCH 0/7] refactor kvargs test Chengwen Feng
2023-11-03  9:53 ` [PATCH 1/7] app/test: introduce UT suite framework for kvargs Chengwen Feng
2023-11-03  9:53 ` [PATCH 2/7] app/test: extract basic token count testcase " Chengwen Feng
2023-11-03  9:53 ` [PATCH 3/7] app/test: extract without keys " Chengwen Feng
2023-11-03  9:53 ` [PATCH 4/7] app/test: extract with " Chengwen Feng
2023-11-03  9:53 ` [PATCH 5/7] app/test: extract parse list value " Chengwen Feng
2023-11-03  9:53 ` [PATCH 6/7] app/test: extract parse empty elements " Chengwen Feng
2023-11-03  9:53 ` [PATCH 7/7] maintainers: update for kvargs library Chengwen Feng
2023-12-05  2:55 ` [PATCH 0/7] refactor kvargs test fengchengwen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).