diff mbox

[dpdk-dev,1/2] test: use env variable to run test if set

Message ID 1513598038-148115-2-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry Dec. 18, 2017, 11:53 a.m. UTC 
  This commit paves the way for the meson tests in the next
patch. With this patch the test binary checks the DPDK_TEST
environment variable and if set, the contents of the var
are inserted on the test app command line, and run.

This allows testing of various different unit tests without
manual interaction with the RTE>> test prompt, instead automating
it using the DPDK_TEST environment variable.

If the DPDK_TEST env variable is not set, or has zero lenght,
the test app behaves as normal.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 test/test/test.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Bruce Richardson Dec. 18, 2017, 1:50 p.m. UTC | #1 
On Mon, Dec 18, 2017 at 11:53:57AM +0000, Harry van Haaren wrote:
> This commit paves the way for the meson tests in the next
> patch. With this patch the test binary checks the DPDK_TEST
> environment variable and if set, the contents of the var
> are inserted on the test app command line, and run.
> 
> This allows testing of various different unit tests without
> manual interaction with the RTE>> test prompt, instead automating
> it using the DPDK_TEST environment variable.
> 
> If the DPDK_TEST env variable is not set, or has zero lenght,
> the test app behaves as normal.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---

This is very useful, even without the meson test part added in the next
patch.

Probably a matter of personal taste, but I think I would prefer to see
the tests to be run specified as args on the cmdline rather than passed
through the environment. I'd tend to view environment variables as
things to use for long-lasting values, rather than parameters you might
want to change between each run. Using cmdline args would also save you
having to split commands, since you'd get the list pre-split in
argv.

/Bruce
Jerin Jacob Dec. 18, 2017, 2:59 p.m. UTC | #2 
-----Original Message-----
> Date: Mon, 18 Dec 2017 11:53:57 +0000
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: bruce.richardson@intel.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if set
> X-Mailer: git-send-email 2.7.4
> 
> This commit paves the way for the meson tests in the next
> patch. With this patch the test binary checks the DPDK_TEST
> environment variable and if set, the contents of the var
> are inserted on the test app command line, and run.
> 
> This allows testing of various different unit tests without
> manual interaction with the RTE>> test prompt, instead automating
> it using the DPDK_TEST environment variable.

Another alternative is to pipe the command.
example:
echo "eventdev_common_autotest" | sudo ./build/app/test
Van Haaren, Harry Dec. 18, 2017, 3:24 p.m. UTC | #3 
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, December 18, 2017 2:59 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if
> set
> 
> -----Original Message-----
> > Date: Mon, 18 Dec 2017 11:53:57 +0000
> > From: Harry van Haaren <harry.van.haaren@intel.com>
> > To: dev@dpdk.org
> > CC: bruce.richardson@intel.com, Harry van Haaren
> >  <harry.van.haaren@intel.com>
> > Subject: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if set
> > X-Mailer: git-send-email 2.7.4
> >
> > This commit paves the way for the meson tests in the next
> > patch. With this patch the test binary checks the DPDK_TEST
> > environment variable and if set, the contents of the var
> > are inserted on the test app command line, and run.
> >
> > This allows testing of various different unit tests without
> > manual interaction with the RTE>> test prompt, instead automating
> > it using the DPDK_TEST environment variable.
> 
> Another alternative is to pipe the command.
> example:
> echo "eventdev_common_autotest" | sudo ./build/app/test


With the current implementation, meson handles which tests to run, and the command line. This gives us a clean interface from which to run tests. Note that the following command will run the tests requested:

$ meson test ring_autotest ring_perf_autotest acl_autotest

Meson itself supports two methods of launching tests from the same binary: argv and env variables. In this implementation, the DPDK_TEST env is set by the test runner - and the user doesn't have to use it manually at all, and it is not exported in the shell after the tests have run.

In short - I don't see added value in reworking this to argc argv, or in using terminal tricks like echo "test" | sudo ./test.

Actually, the current method has an easter egg included:
If a developer is focused on a single test-case (TDD anyone? :), then they could use the DPDK_TEST env var as a feature, $ export DPDK_TEST=ring_autotest  and run that test automatically when the binary is launched.
Jerin Jacob Dec. 18, 2017, 3:41 p.m. UTC | #4 
-----Original Message-----
> Date: Mon, 18 Dec 2017 15:24:22 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce"
>  <bruce.richardson@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if
>  set
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, December 18, 2017 2:59 PM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if
> > set
> > 
> > -----Original Message-----
> > > Date: Mon, 18 Dec 2017 11:53:57 +0000
> > > From: Harry van Haaren <harry.van.haaren@intel.com>
> > > To: dev@dpdk.org
> > > CC: bruce.richardson@intel.com, Harry van Haaren
> > >  <harry.van.haaren@intel.com>
> > > Subject: [dpdk-dev] [PATCH 1/2] test: use env variable to run test if set
> > > X-Mailer: git-send-email 2.7.4
> > >
> > > This commit paves the way for the meson tests in the next
> > > patch. With this patch the test binary checks the DPDK_TEST
> > > environment variable and if set, the contents of the var
> > > are inserted on the test app command line, and run.
> > >
> > > This allows testing of various different unit tests without
> > > manual interaction with the RTE>> test prompt, instead automating
> > > it using the DPDK_TEST environment variable.
> > 
> > Another alternative is to pipe the command.
> > example:
> > echo "eventdev_common_autotest" | sudo ./build/app/test
> 
> 
> With the current implementation, meson handles which tests to run, and the command line. This gives us a clean interface from which to run tests. Note that the following command will run the tests requested:
> 
> $ meson test ring_autotest ring_perf_autotest acl_autotest
> 
> Meson itself supports two methods of launching tests from the same binary: argv and env variables. In this implementation, the DPDK_TEST env is set by the test runner - and the user doesn't have to use it manually at all, and it is not exported in the shell after the tests have run.
> 
> In short - I don't see added value in reworking this to argc argv, or in using terminal tricks like echo "test" | sudo ./test.
> 
> Actually, the current method has an easter egg included:
> If a developer is focused on a single test-case (TDD anyone? :), then they could use the DPDK_TEST env var as a feature, $ export DPDK_TEST=ring_autotest  and run that test automatically when the binary is launched.

Yup. I don't see any harm in exposing DPDK_TEST means of test selection.
diff mbox

Patch

diff --git a/test/test/test.c b/test/test/test.c
index 0e6ff7c..fb4d475 100644
--- a/test/test/test.c
+++ b/test/test/test.c
@@ -102,6 +102,8 @@  do_recursive_call(void)
 	return -1;
 }
 
+static int last_test_result;
+
 int
 main(int argc, char **argv)
 {
@@ -140,7 +142,27 @@  main(int argc, char **argv)
 	if (cl == NULL) {
 		return -1;
 	}
-	cmdline_interact(cl);
+
+	char *dpdk_test = getenv("DPDK_TEST");
+	if (dpdk_test && strlen(dpdk_test)) {
+		char buf[1024];
+		snprintf(buf, sizeof(buf), "%s\n", dpdk_test);
+		if (cmdline_in(cl, buf, strlen(buf)) < 0) {
+			printf("error on cmdline input\n");
+			return -1;
+		}
+
+		/* check the last unit test suite return, and error out if
+		 * it failed - this causes Meson to pick up the failure.
+		 */
+		if (last_test_result) {
+			cmdline_stdin_exit(cl);
+			exit(-1);
+		}
+
+	} else {
+		cmdline_interact(cl);
+	}
 	cmdline_stdin_exit(cl);
 #endif
 
@@ -231,6 +253,8 @@  unit_test_suite_runner(struct unit_test_suite *suite)
 	printf(" + Tests Failed :      %2d\n", failed);
 	printf(" + ------------------------------------------------------- +\n");
 
+	last_test_result = failed;
+
 	if (failed)
 		return -1;