[v2,2/2] test: use cmdline library to validate args

Message ID 20220520151240.139566-3-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series fix uncallable unit tests (Bugzilla 1002) |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Bruce Richardson May 20, 2022, 3:12 p.m. UTC
  When passing in test names to run via either the DPDK_TEST environment
variable or via extra argv parameters, the checks run on those commands
can miss valid commands that are registered with the cmdline library in
the initial context used to set it up. This is seen in the fact that the
"dump_*" set of commands are not callable via argv parameters, but can
be called manually.

To fix this, just use the commandline library to validate each command
before executing it, stopping execution when an error is encountered.
This also has the benefit of not having the test binrary drop to
interactive mode if all commandline parameters given are invalid.

Fixes: 9b848774a5dc ("test: use env variable to run tests")
Fixes: ace2f054ed43 ("test: take test names from command line")
Bugzilla ID: 1002

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/commands.c | 11 -----------
 app/test/test.c     | 24 +++++++-----------------
 2 files changed, 7 insertions(+), 28 deletions(-)
  

Comments

Olivier Matz June 7, 2022, 8:08 a.m. UTC | #1
On Fri, May 20, 2022 at 04:12:40PM +0100, Bruce Richardson wrote:
> When passing in test names to run via either the DPDK_TEST environment
> variable or via extra argv parameters, the checks run on those commands
> can miss valid commands that are registered with the cmdline library in
> the initial context used to set it up. This is seen in the fact that the
> "dump_*" set of commands are not callable via argv parameters, but can
> be called manually.
> 
> To fix this, just use the commandline library to validate each command
> before executing it, stopping execution when an error is encountered.
> This also has the benefit of not having the test binrary drop to
> interactive mode if all commandline parameters given are invalid.
> 
> Fixes: 9b848774a5dc ("test: use env variable to run tests")
> Fixes: ace2f054ed43 ("test: take test names from command line")
> Bugzilla ID: 1002
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>
  

Patch

diff --git a/app/test/commands.c b/app/test/commands.c
index 887cabad64..31259e5c21 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -378,14 +378,3 @@  int commands_init(void)
 	cmd_autotest_autotest.string_data.str = commands;
 	return 0;
 }
-
-int command_valid(const char *cmd)
-{
-	struct test_command *t;
-
-	TAILQ_FOREACH(t, &commands_list, next) {
-		if (strcmp(t->command, cmd) == 0)
-			return 1;
-	}
-	return 0;
-}
diff --git a/app/test/test.c b/app/test/test.c
index e69cae3eea..ce25f468a6 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -186,22 +186,10 @@  main(int argc, char **argv)
 #ifdef RTE_LIB_CMDLINE
 	char *dpdk_test = getenv("DPDK_TEST");
 
-	if (dpdk_test && strlen(dpdk_test) == 0)
-		dpdk_test = NULL;
-
-	if (dpdk_test && !command_valid(dpdk_test)) {
-		RTE_LOG(WARNING, APP, "Invalid DPDK_TEST value '%s'\n", dpdk_test);
-		dpdk_test = NULL;
-	}
-
-	if (dpdk_test)
+	if (dpdk_test && strlen(dpdk_test) > 0)
 		tests[test_count++] = dpdk_test;
-	for (i = 1; i < argc; i++) {
-		if (!command_valid(argv[i]))
-			RTE_LOG(WARNING, APP, "Invalid test requested: '%s'\n", argv[i]);
-		else
-			tests[test_count++] = argv[i];
-	}
+	for (i = 1; i < argc; i++)
+		tests[test_count++] = argv[i];
 
 	if (test_count > 0) {
 		char buf[1024];
@@ -214,9 +202,11 @@  main(int argc, char **argv)
 
 		for (i = 0; i < test_count; i++) {
 			snprintf(buf, sizeof(buf), "%s\n", tests[i]);
-			if (cmdline_in(cl, buf, strlen(buf)) < 0) {
+			if (cmdline_parse_check(cl, buf) < 0) {
+				printf("Error: invalid test command: '%s'\n", tests[i]);
+				ret = -1;
+			} else if (cmdline_in(cl, buf, strlen(buf)) < 0) {
 				printf("error on cmdline input\n");
-
 				ret = -1;
 			} else
 				ret = last_test_result;