[dpdk-dev] [PATCH 0/2] improve meson test support

[dpdk-dev] [PATCH 0/2] improve meson test support

[Bruce Richardson]

Being able to call "meson test" to run unit tests is very handy,
but the support is still in the early days. These two patches improve
things by fixing a bug in error reporting and then allowing a test to
report as skipped if a particular lib/driver/piece of HW is missing.

Bruce Richardson (2):
  test/test: fix missed failures when running meson test
  test/test: add return value to mark unit tests as skipped

 test/test/commands.c       |  1 +
 test/test/test.c           | 16 +++++-----------
 test/test/test.h           |  6 ++++--
 test/test/test_cryptodev.c | 28 ++++++++++++++--------------
 4 files changed, 24 insertions(+), 27 deletions(-)

-- 
2.14.3

[dpdk-dev] [PATCH 1/2] test/test: fix missed failures when running meson test

[Bruce Richardson]

When using meson test, and/or the DPDK_TEST environment variable for
running specific tests, the return value from the test binary was the value
returned from the unit test runner function. However, not all tests use
that unit test runner infrastructure - some are regular functions, and so
the failure of those tests would not be recognised.

Fix this by setting the last_test_result value inside the command-line
function that calls the individual test fns and that prints the "Test OK"
or "Test Failure" messages.

Fixes: d79a9657a78c ("meson: add tests app to build")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/commands.c | 1 +
 test/test/test.c     | 2 +-
 test/test/test.h     | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test/commands.c b/test/test/commands.c
index 4097a3310..6649cfc32 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -103,6 +103,7 @@ static void cmd_autotest_parsed(void *parsed_result,
 			ret = t->callback();
 	}
 
+	last_test_result = ret;
 	if (ret == 0)
 		printf("Test OK\n");
 	else
diff --git a/test/test/test.c b/test/test/test.c
index fb4d4753e..bc273db07 100644
--- a/test/test/test.c
+++ b/test/test/test.c
@@ -102,7 +102,7 @@ do_recursive_call(void)
 	return -1;
 }
 
-static int last_test_result;
+int last_test_result;
 
 int
 main(int argc, char **argv)
diff --git a/test/test/test.h b/test/test/test.h
index 08ffe949c..2e9018437 100644
--- a/test/test/test.h
+++ b/test/test/test.h
@@ -218,6 +218,7 @@ struct unit_test_suite {
 };
 
 int unit_test_suite_runner(struct unit_test_suite *suite);
+extern int last_test_result;
 
 #define RECURSIVE_ENV_VAR "RTE_TEST_RECURSIVE"
 
-- 
2.14.3

[dpdk-dev] [PATCH 2/2] test/test: add return value to mark unit tests as skipped

[Bruce Richardson]

Add in a TEST_SKIPPED return value for unit tests to mark the tests
as skipped, rather than just failed. Use this new skipped return value for
the crypto tests which can only run if they have a particular driver.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test.c           | 14 ++++----------
 test/test/test.h           |  5 +++--
 test/test/test_cryptodev.c | 28 ++++++++++++++--------------
 3 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/test/test/test.c b/test/test/test.c
index bc273db07..49a61ce05 100644
--- a/test/test/test.c
+++ b/test/test/test.c
@@ -152,17 +152,11 @@ main(int argc, char **argv)
 			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);
+		return last_test_result;
 	}
+	/* if no DPDK_TEST env variable, go interactive */
+	cmdline_interact(cl);
 	cmdline_stdin_exit(cl);
 #endif
 
diff --git a/test/test/test.h b/test/test/test.h
index 2e9018437..df5eea088 100644
--- a/test/test/test.h
+++ b/test/test/test.h
@@ -40,8 +40,9 @@
 #include <rte_common.h>
 #include <rte_log.h>
 
-#define TEST_SUCCESS  (0)
-#define TEST_FAILED  (-1)
+#define TEST_SUCCESS EXIT_SUCCESS
+#define TEST_FAILED  -1
+#define TEST_SKIPPED  77
 
 /* Before including test.h file you can define
  * TEST_TRACE_FAILURE(_file, _line, _func) macro to better trace/debug test
diff --git a/test/test/test_cryptodev.c b/test/test/test_cryptodev.c
index 1bed65dad..54e6a9e36 100644
--- a/test/test/test_cryptodev.c
+++ b/test/test/test_cryptodev.c
@@ -9591,7 +9591,7 @@ test_cryptodev_qat(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "QAT PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_QAT is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_qat_testsuite);
@@ -9607,7 +9607,7 @@ test_cryptodev_aesni_mb(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "AESNI MB PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_AESNI_MB is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_aesni_mb_testsuite);
@@ -9623,7 +9623,7 @@ test_cryptodev_openssl(void)
 		RTE_LOG(ERR, USER1, "OPENSSL PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_OPENSSL is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_openssl_testsuite);
@@ -9639,7 +9639,7 @@ test_cryptodev_aesni_gcm(void)
 		RTE_LOG(ERR, USER1, "AESNI GCM PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_AESNI_GCM is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_aesni_gcm_testsuite);
@@ -9655,7 +9655,7 @@ test_cryptodev_null(void)
 		RTE_LOG(ERR, USER1, "NULL PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_NULL is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_null_testsuite);
@@ -9671,7 +9671,7 @@ test_cryptodev_sw_snow3g(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "SNOW3G PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_SNOW3G is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_sw_snow3g_testsuite);
@@ -9687,7 +9687,7 @@ test_cryptodev_sw_kasumi(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "ZUC PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_KASUMI is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_sw_kasumi_testsuite);
@@ -9703,7 +9703,7 @@ test_cryptodev_sw_zuc(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "ZUC PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_ZUC is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_sw_zuc_testsuite);
@@ -9719,7 +9719,7 @@ test_cryptodev_armv8(void)
 		RTE_LOG(ERR, USER1, "ARMV8 PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_ARMV8 is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_armv8_testsuite);
@@ -9735,7 +9735,7 @@ test_cryptodev_mrvl(void)
 		RTE_LOG(ERR, USER1, "MRVL PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_MRVL_CRYPTO is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_mrvl_testsuite);
@@ -9753,14 +9753,14 @@ test_cryptodev_scheduler(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "SCHEDULER PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_SCHEDULER is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	if (rte_cryptodev_driver_id_get(
 				RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD)) == -1) {
 		RTE_LOG(ERR, USER1, "CONFIG_RTE_LIBRTE_PMD_AESNI_MB must be"
 			" enabled in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 }
 	return unit_test_suite_runner(&cryptodev_scheduler_testsuite);
 }
@@ -9779,7 +9779,7 @@ test_cryptodev_dpaa2_sec(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "DPAA2 SEC PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_DPAA2_SEC is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_dpaa2_sec_testsuite);
@@ -9795,7 +9795,7 @@ test_cryptodev_dpaa_sec(void /*argv __rte_unused, int argc __rte_unused*/)
 		RTE_LOG(ERR, USER1, "DPAA SEC PMD must be loaded. Check if "
 				"CONFIG_RTE_LIBRTE_PMD_DPAA_SEC is enabled "
 				"in config file to run this testsuite.\n");
-		return TEST_FAILED;
+		return TEST_SKIPPED;
 	}
 
 	return unit_test_suite_runner(&cryptodev_dpaa_sec_testsuite);
-- 
2.14.3

Re: [dpdk-dev] [PATCH 2/2] test/test: add return value to mark unit tests as skipped

[Bruce Richardson]

On Thu, Dec 21, 2017 at 10:15:10AM +0000, Bruce Richardson wrote:
> Add in a TEST_SKIPPED return value for unit tests to mark the tests
> as skipped, rather than just failed. Use this new skipped return value for
> the crypto tests which can only run if they have a particular driver.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

+Declan and Pablo as maintainers of crypto code.

/Bruce

Re: [dpdk-dev] [PATCH 1/2] test/test: fix missed failures when running meson test

[Van Haaren, Harry]

> From: Richardson, Bruce
> Sent: Thursday, December 21, 2017 10:15 AM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: [PATCH 1/2] test/test: fix missed failures when running meson test
> 
> When using meson test, and/or the DPDK_TEST environment variable for
> running specific tests, the return value from the test binary was the value
> returned from the unit test runner function. However, not all tests use
> that unit test runner infrastructure - some are regular functions, and so
> the failure of those tests would not be recognised.
> 
> Fix this by setting the last_test_result value inside the command-line
> function that calls the individual test fns and that prints the "Test OK"
> or "Test Failure" messages.
> 
> Fixes: d79a9657a78c ("meson: add tests app to build")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>


Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH 2/2] test/test: add return value to mark unit tests as skipped

[Van Haaren, Harry]

> From: Richardson, Bruce
> Sent: Thursday, December 21, 2017 10:23 AM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: Re: [PATCH 2/2] test/test: add return value to mark unit tests as
> skipped
> 
> On Thu, Dec 21, 2017 at 10:15:10AM +0000, Bruce Richardson wrote:
> > Add in a TEST_SKIPPED return value for unit tests to mark the tests
> > as skipped, rather than just failed. Use this new skipped return value for
> > the crypto tests which can only run if they have a particular driver.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> +Declan and Pablo as maintainers of crypto code.

Looks good to me, checked unit tests using old build system and binary, but 2nd Ack from crypto maintainers welcome :)

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH 2/2] test/test: add return value to mark unit tests as skipped

[Bruce Richardson]

On Thu, Dec 21, 2017 at 10:48:22AM +0000, Van Haaren, Harry wrote:
> > From: Richardson, Bruce
> > Sent: Thursday, December 21, 2017 10:23 AM
> > To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; De Lara
> > Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: Re: [PATCH 2/2] test/test: add return value to mark unit tests as
> > skipped
> > 
> > On Thu, Dec 21, 2017 at 10:15:10AM +0000, Bruce Richardson wrote:
> > > Add in a TEST_SKIPPED return value for unit tests to mark the tests
> > > as skipped, rather than just failed. Use this new skipped return value for
> > > the crypto tests which can only run if they have a particular driver.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > +Declan and Pablo as maintainers of crypto code.
> 
> Looks good to me, checked unit tests using old build system and binary, but 2nd Ack from crypto maintainers welcome :)
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Following IRC discussion this set is now superceded by a new set
targeting these changes at the main tree.

http://dpdk.org/dev/patchwork/patch/32560/
http://dpdk.org/dev/patchwork/patch/32561/

Patchwork has been updated to reflect this.

/Bruce