diff mbox series

test: allow taking test names from commandline

Message ID 20210127174255.1671738-1-bruce.richardson@intel.com (mailing list archive)
State New
Delegated to: David Marchand
Headers show
Series test: allow taking test names from commandline | expand

Checks

Context Check Description
ci/iol-testing fail Testing issues
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Jan. 27, 2021, 5:42 p.m. UTC
While having the ability to run a test based off the DPDK_TEST environment
variable is useful, it's often easier to specify the test name as a
commandline parameter to a test binary. This also allows the test runs to
be saved as part of the shell cmdline history.

This patch adds support for checking all parameters after the EAL ones, and
running all valid autotests requested - either from DPDK_TEST or on the
commandline. This also allows multiple tests to be run in a single
automated session, which is useful for working with components which have
multiple test suites.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/commands.c | 11 +++++++++++
 app/test/test.c     | 40 +++++++++++++++++++++++++++++++++-------
 app/test/test.h     |  1 +
 3 files changed, 45 insertions(+), 7 deletions(-)

--
2.27.0

Comments

Aaron Conole Jan. 29, 2021, 8:22 p.m. UTC | #1
Bruce Richardson <bruce.richardson@intel.com> writes:

> While having the ability to run a test based off the DPDK_TEST environment
> variable is useful, it's often easier to specify the test name as a
> commandline parameter to a test binary. This also allows the test runs to
> be saved as part of the shell cmdline history.
>
> This patch adds support for checking all parameters after the EAL ones, and
> running all valid autotests requested - either from DPDK_TEST or on the
> commandline. This also allows multiple tests to be run in a single
> automated session, which is useful for working with components which have
> multiple test suites.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

LGTM, and I think it can be useful for running specific tests when
developers make changes.

Acked-by: Aaron Conole <aconole@redhat.com>
David Marchand April 9, 2021, 1:27 p.m. UTC | #2
On Wed, Jan 27, 2021 at 6:43 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> While having the ability to run a test based off the DPDK_TEST environment
> variable is useful, it's often easier to specify the test name as a
> commandline parameter to a test binary. This also allows the test runs to
> be saved as part of the shell cmdline history.

I don't get the argument about history:

$ history |grep DPDK_TEST
10615  2021-03-24 10:42:11 ninja-build -C build -j4 &&
DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512
--log-level=lib.eal:debug
10636  2021-03-24 10:51:09 ninja-build -C build -j4 &&
DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512
--log-level=lib.eal:debug
10653  2021-03-24 11:17:01 ninja-build -C build -j4 &&
DPDK_TEST=kvargs_autotest ./build/app/test/dpdk-test --no-huge -m 512
--log-level=lib.eal:debug
10794  2021-03-25 18:37:48 history |grep DPDK_TEST


>
> This patch adds support for checking all parameters after the EAL ones, and
> running all valid autotests requested - either from DPDK_TEST or on the
> commandline. This also allows multiple tests to be run in a single
> automated session, which is useful for working with components which have
> multiple test suites.

The same could be achieved splitting DPDK_TEST content with spaces,
since test names don't contain one.
Bruce Richardson April 9, 2021, 1:41 p.m. UTC | #3
On Fri, Apr 09, 2021 at 03:27:17PM +0200, David Marchand wrote:
> On Wed, Jan 27, 2021 at 6:43 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > While having the ability to run a test based off the DPDK_TEST environment
> > variable is useful, it's often easier to specify the test name as a
> > commandline parameter to a test binary. This also allows the test runs to
> > be saved as part of the shell cmdline history.
> 
> I don't get the argument about history:
> 
> $ history |grep DPDK_TEST
> 10615  2021-03-24 10:42:11 ninja-build -C build -j4 &&
> DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512
> --log-level=lib.eal:debug
> 10636  2021-03-24 10:51:09 ninja-build -C build -j4 &&
> DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512
> --log-level=lib.eal:debug
> 10653  2021-03-24 11:17:01 ninja-build -C build -j4 &&
> DPDK_TEST=kvargs_autotest ./build/app/test/dpdk-test --no-huge -m 512
> --log-level=lib.eal:debug
> 10794  2021-03-25 18:37:48 history |grep DPDK_TEST
> 

Sure, if you always specify the test name explicitly for each command,
rather than running the one test multiple times having set it separately in
the environment.
Overall, though I take the point that from a history saving point of view
it's a minor saving.

> 
> >
> > This patch adds support for checking all parameters after the EAL ones, and
> > running all valid autotests requested - either from DPDK_TEST or on the
> > commandline. This also allows multiple tests to be run in a single
> > automated session, which is useful for working with components which have
> > multiple test suites.
> 
> The same could be achieved splitting DPDK_TEST content with spaces,
> since test names don't contain one.
> 

Yep, that's a useful enhancement too, but I still thing it's better to just
have the list of tests appended to the test binary command rather than have
to be worrying about properly quoting a specific environment variable at
the start of each command.

/Bruce
diff mbox series

Patch

diff --git a/app/test/commands.c b/app/test/commands.c
index d48dd513d7..76f6ee5d23 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -381,3 +381,14 @@  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 624dd48042..b4ef2bf22b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -93,6 +93,9 @@  main(int argc, char **argv)
 {
 #ifdef RTE_LIB_CMDLINE
 	struct cmdline *cl;
+	char *tests[argc]; /* store an array of tests to run */
+	int test_count = 0;
+	int i;
 #endif
 	char *extra_args;
 	int ret;
@@ -147,6 +150,7 @@  main(int argc, char **argv)
 	}

 	argv += ret;
+	argc -= ret;

 	prgname = argv[0];

@@ -165,7 +169,25 @@  main(int argc, char **argv)

 #ifdef RTE_LIB_CMDLINE
 	char *dpdk_test = getenv("DPDK_TEST");
-	if (dpdk_test && strlen(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)
+		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];
+	}
+
+	if (test_count > 0) {
 		char buf[1024];

 		cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
@@ -174,13 +196,17 @@  main(int argc, char **argv)
 			goto out;
 		}

-		snprintf(buf, sizeof(buf), "%s\n", dpdk_test);
-		if (cmdline_in(cl, buf, strlen(buf)) < 0) {
-			printf("error on cmdline input\n");
+		for (i = 0; i < test_count; i++) {
+			snprintf(buf, sizeof(buf), "%s\n", tests[i]);
+			if (cmdline_in(cl, buf, strlen(buf)) < 0) {
+				printf("error on cmdline input\n");

-			ret = -1;
-		} else {
-			ret = last_test_result;
+				ret = -1;
+			} else
+				ret = last_test_result;
+
+			if (ret != 0)
+				break;
 		}
 		cmdline_free(cl);
 		goto out;
diff --git a/app/test/test.h b/app/test/test.h
index b07f6c1ef0..cd047eb26c 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -152,6 +152,7 @@  extern int last_test_result;
 extern const char *prgname;

 int commands_init(void);
+int command_valid(const char *cmd);

 int test_mp_secondary(void);
 int test_timer_secondary(void);