* Re: [RFC PATCH] app/test: add support for skipping tests
2023-08-17 10:58 [RFC PATCH] app/test: add support for skipping tests Bruce Richardson
@ 2023-08-29 9:34 ` Thomas Monjalon
2023-08-29 13:27 ` Aaron Conole
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Thomas Monjalon @ 2023-08-29 9:34 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, david.marchand, probb
jeudi 17 août 2023, Bruce Richardson:
> When called from automated tools, like meson test, it is often useful to
> skip tests in a test suite, without having to alter the test build. To
> do so, we add support for DPDK_TEST_SKIP environment variable, where one
> can provide a comma-separated list of tests. When the test binary is
> called to run one of the tests on the list via either cmdline parameter
> or environment variable (as done with meson test), the test will not
> actually be run, but will be reported skipped.
>
> Example run:
> $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> ...
> 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
>
> Ok: 7
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 2
> Timeout: 0
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Looks to be a good addition.
Acked-by: Thomas Monjalon <thomas@monjalon.net>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] app/test: add support for skipping tests
2023-08-17 10:58 [RFC PATCH] app/test: add support for skipping tests Bruce Richardson
2023-08-29 9:34 ` Thomas Monjalon
@ 2023-08-29 13:27 ` Aaron Conole
2023-08-29 13:39 ` Bruce Richardson
2023-08-31 10:06 ` [PATCH v2] " Bruce Richardson
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Aaron Conole @ 2023-08-29 13:27 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, david.marchand, probb
Bruce Richardson <bruce.richardson@intel.com> writes:
> When called from automated tools, like meson test, it is often useful to
> skip tests in a test suite, without having to alter the test build. To
> do so, we add support for DPDK_TEST_SKIP environment variable, where one
> can provide a comma-separated list of tests. When the test binary is
> called to run one of the tests on the list via either cmdline parameter
> or environment variable (as done with meson test), the test will not
> actually be run, but will be reported skipped.
>
> Example run:
> $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> ...
> 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
>
> Ok: 7
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 2
> Timeout: 0
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
The idea looks fine to me, although I would really like it if we could
do a command like argument instead of env variable (but that's just a
suggestion to color the shed).
Just minor nit stuff below.
> app/test/test.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/app/test/test.c b/app/test/test.c
> index fb073ff795..3569c36049 100644
> --- a/app/test/test.c
> +++ b/app/test/test.c
> @@ -193,6 +193,20 @@ main(int argc, char **argv)
>
> if (test_count > 0) {
> char buf[1024];
> + char *dpdk_test_skip = getenv("DPDK_TEST_SKIP");
> + char *skip_tests[128] = {0};
> + size_t n_skip_tests = 0;
> +
> + if (dpdk_test_skip != NULL && strlen(dpdk_test_skip) > 0){
> + char *dpdk_test_skip_cp = strdup(dpdk_test_skip);
> + if (dpdk_test_skip_cp == NULL) {
> + ret = -1;
> + goto out;
> + }
> + dpdk_test_skip = dpdk_test_skip_cp;
> + n_skip_tests = rte_strsplit(dpdk_test_skip, strlen(dpdk_test_skip),
> + skip_tests, RTE_DIM(skip_tests), ',');
We probably should check for n_skip_tests == -1 - although it shouldn't
ever happen for this code. If it ever did happen, we'd walk right off
the for() loop below. Actually, it looks like that error condition for
rte_strsplit isn't documented.
> + }
>
> cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
> if (cl == NULL) {
> @@ -201,6 +215,15 @@ main(int argc, char **argv)
> }
>
> for (i = 0; i < test_count; i++) {
> + /* check if test is to be skipped */
> + for (size_t j = 0; j < n_skip_tests; j++) {
We might want to check j < MIN(n_skip_tests, RTE_DIM(skip_tests)) to
prevent a rogue env variable that makes rte_strsplit fail in the future.
> + if (strcmp(tests[i], skip_tests[j]) == 0) {
> + fprintf(stderr, "Skipping %s [DPDK_TEST_SKIP]\n", tests[i]);
> + ret = TEST_SKIPPED;
> + goto end_of_cmd;
> + }
> + }
> +
> snprintf(buf, sizeof(buf), "%s\n", tests[i]);
> if (cmdline_parse_check(cl, buf) < 0) {
> printf("Error: invalid test command: '%s'\n", tests[i]);
> @@ -211,9 +234,13 @@ main(int argc, char **argv)
> } else
> ret = last_test_result;
>
> +end_of_cmd:
> if (ret != 0)
> break;
> }
> + if (n_skip_tests > 0)
> + free(dpdk_test_skip);
> +
> cmdline_free(cl);
> goto out;
> } else {
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH] app/test: add support for skipping tests
2023-08-29 13:27 ` Aaron Conole
@ 2023-08-29 13:39 ` Bruce Richardson
0 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-08-29 13:39 UTC (permalink / raw)
To: Aaron Conole; +Cc: dev, david.marchand, probb
On Tue, Aug 29, 2023 at 09:27:23AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
>
> > When called from automated tools, like meson test, it is often useful to
> > skip tests in a test suite, without having to alter the test build. To
> > do so, we add support for DPDK_TEST_SKIP environment variable, where one
> > can provide a comma-separated list of tests. When the test binary is
> > called to run one of the tests on the list via either cmdline parameter
> > or environment variable (as done with meson test), the test will not
> > actually be run, but will be reported skipped.
> >
> > Example run:
> > $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> > ...
> > 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> > 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> > 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> > 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> > 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> > 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> > 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> > 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> > 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
> >
> > Ok: 7
> > Expected Fail: 0
> > Fail: 0
> > Unexpected Pass: 0
> > Skipped: 2
> > Timeout: 0
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
>
> The idea looks fine to me, although I would really like it if we could
> do a command like argument instead of env variable (but that's just a
> suggestion to color the shed).
>
Yes, I prefer cmdline args too, but I believed that for running test suites
having the tests to disable set in the environment was a better solution,
since it was easy to get at whatever way the tests were run via a
test-runner.
However, there is support for passing "--test-args" parameter to meson
test, which should allow passing this stuff via cmdline. If you prefer the
cmdline solution and if it doesn't have any gotchas like that, I can
probably look to rework this to use cmdline. I assume that any other test
runners in use would similarly be able to append addition cmdline args to
the test runs?
> Just minor nit stuff below.
>
> > app/test/test.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/app/test/test.c b/app/test/test.c
> > index fb073ff795..3569c36049 100644
> > --- a/app/test/test.c
> > +++ b/app/test/test.c
> > @@ -193,6 +193,20 @@ main(int argc, char **argv)
> >
> > if (test_count > 0) {
> > char buf[1024];
> > + char *dpdk_test_skip = getenv("DPDK_TEST_SKIP");
> > + char *skip_tests[128] = {0};
> > + size_t n_skip_tests = 0;
> > +
> > + if (dpdk_test_skip != NULL && strlen(dpdk_test_skip) > 0){
> > + char *dpdk_test_skip_cp = strdup(dpdk_test_skip);
> > + if (dpdk_test_skip_cp == NULL) {
> > + ret = -1;
> > + goto out;
> > + }
> > + dpdk_test_skip = dpdk_test_skip_cp;
> > + n_skip_tests = rte_strsplit(dpdk_test_skip, strlen(dpdk_test_skip),
> > + skip_tests, RTE_DIM(skip_tests), ',');
>
> We probably should check for n_skip_tests == -1 - although it shouldn't
> ever happen for this code. If it ever did happen, we'd walk right off
> the for() loop below. Actually, it looks like that error condition for
> rte_strsplit isn't documented.
>
Can't happen for this code. The strsplit only returns -1 if the first or
third params are NULL, and the first param (dpdk_test_skip) is checked for
NULL before the call, and the third is a local array pointer in this case.
> > + }
> >
> > cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
> > if (cl == NULL) {
> > @@ -201,6 +215,15 @@ main(int argc, char **argv)
> > }
> >
> > for (i = 0; i < test_count; i++) {
> > + /* check if test is to be skipped */
> > + for (size_t j = 0; j < n_skip_tests; j++) {
>
> We might want to check j < MIN(n_skip_tests, RTE_DIM(skip_tests)) to
> prevent a rogue env variable that makes rte_strsplit fail in the future.
>
rte_strsplit cannot return a value > RTE_DIM(skip_tests). The main loop
only increments the return value (tok) at most once per loop, and at the
start of the loop checks for tok >= maxtokens.
> > + if (strcmp(tests[i], skip_tests[j]) == 0) {
> > + fprintf(stderr, "Skipping %s [DPDK_TEST_SKIP]\n", tests[i]);
> > + ret = TEST_SKIPPED;
> > + goto end_of_cmd;
> > + }
> > + }
> > +
<snip>
Thanks for the review.
/Bruce
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] app/test: add support for skipping tests
2023-08-17 10:58 [RFC PATCH] app/test: add support for skipping tests Bruce Richardson
2023-08-29 9:34 ` Thomas Monjalon
2023-08-29 13:27 ` Aaron Conole
@ 2023-08-31 10:06 ` Bruce Richardson
2023-08-31 10:18 ` [PATCH v3] " Bruce Richardson
2023-09-14 15:16 ` [PATCH v4] " Bruce Richardson
4 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-08-31 10:06 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, Thomas Monjalon
When called from automated tools, like meson test, it is often useful to
skip tests in a test suite, without having to alter the test build. To
do so, we add support for DPDK_TEST_SKIP environment variable, where one
can provide a comma-separated list of tests. When the test binary is
called to run one of the tests on the list via either cmdline parameter
or environment variable (as done with meson test), the test will not
actually be run, but will be reported skipped.
Example run:
$ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
...
1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
2/9 DPDK:debug-tests / dump_log_types OK 1.06s
3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
5/9 DPDK:debug-tests / dump_mempool OK 1.11s
6/9 DPDK:debug-tests / dump_memzone OK 1.06s
7/9 DPDK:debug-tests / dump_physmem OK 1.13s
8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
Ok: 7
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 2
Timeout: 0
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
V2: Check return value from rte_strsplit, just in case.
---
app/test/test.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/app/test/test.c b/app/test/test.c
index fb073ff795..1143285af2 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -193,6 +193,25 @@ main(int argc, char **argv)
if (test_count > 0) {
char buf[1024];
+ char *dpdk_test_skip = getenv("DPDK_TEST_SKIP");
+ char *skip_tests[128] = {0};
+ size_t n_skip_tests = 0;
+
+ if (dpdk_test_skip != NULL && strlen(dpdk_test_skip) > 0){
+ int split_ret;
+ char *dpdk_test_skip_cp = strdup(dpdk_test_skip);
+ if (dpdk_test_skip_cp == NULL) {
+ ret = -1;
+ goto out;
+ }
+ dpdk_test_skip = dpdk_test_skip_cp;
+ split_ret = rte_strsplit(dpdk_test_skip, strlen(dpdk_test_skip),
+ skip_tests, RTE_DIM(skip_tests), ',');
+ if (split_ret > 0)
+ n_skip_tests = split_ret;
+ else
+ free(dpdk_test_skip);
+ }
cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
if (cl == NULL) {
@@ -201,6 +220,15 @@ main(int argc, char **argv)
}
for (i = 0; i < test_count; i++) {
+ /* check if test is to be skipped */
+ for (size_t j = 0; j < n_skip_tests; j++) {
+ if (strcmp(tests[i], skip_tests[j]) == 0) {
+ fprintf(stderr, "Skipping %s [DPDK_TEST_SKIP]\n", tests[i]);
+ ret = TEST_SKIPPED;
+ goto end_of_cmd;
+ }
+ }
+
snprintf(buf, sizeof(buf), "%s\n", tests[i]);
if (cmdline_parse_check(cl, buf) < 0) {
printf("Error: invalid test command: '%s'\n", tests[i]);
@@ -211,9 +239,13 @@ main(int argc, char **argv)
} else
ret = last_test_result;
+end_of_cmd:
if (ret != 0)
break;
}
+ if (n_skip_tests > 0)
+ free(dpdk_test_skip);
+
cmdline_free(cl);
goto out;
} else {
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v3] app/test: add support for skipping tests
2023-08-17 10:58 [RFC PATCH] app/test: add support for skipping tests Bruce Richardson
` (2 preceding siblings ...)
2023-08-31 10:06 ` [PATCH v2] " Bruce Richardson
@ 2023-08-31 10:18 ` Bruce Richardson
2023-08-31 13:58 ` Bruce Richardson
2023-09-14 15:16 ` [PATCH v4] " Bruce Richardson
4 siblings, 1 reply; 15+ messages in thread
From: Bruce Richardson @ 2023-08-31 10:18 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, Thomas Monjalon
When called from automated tools, like meson test, it is often useful to
skip tests in a test suite, without having to alter the test build. To
do so, we add support for DPDK_TEST_SKIP environment variable, where one
can provide a comma-separated list of tests. When the test binary is
called to run one of the tests on the list via either cmdline parameter
or environment variable (as done with meson test), the test will not
actually be run, but will be reported skipped.
Example run:
$ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
...
1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
2/9 DPDK:debug-tests / dump_log_types OK 1.06s
3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
5/9 DPDK:debug-tests / dump_mempool OK 1.11s
6/9 DPDK:debug-tests / dump_memzone OK 1.06s
7/9 DPDK:debug-tests / dump_physmem OK 1.13s
8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
Ok: 7
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 2
Timeout: 0
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
V3: Fix checkpatch whitespace issue
V2: Check return value from rte_strsplit, just in case.
---
app/test/test.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/app/test/test.c b/app/test/test.c
index fb073ff795..bfa9ea52e3 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -193,6 +193,25 @@ main(int argc, char **argv)
if (test_count > 0) {
char buf[1024];
+ char *dpdk_test_skip = getenv("DPDK_TEST_SKIP");
+ char *skip_tests[128] = {0};
+ size_t n_skip_tests = 0;
+
+ if (dpdk_test_skip != NULL && strlen(dpdk_test_skip) > 0) {
+ int split_ret;
+ char *dpdk_test_skip_cp = strdup(dpdk_test_skip);
+ if (dpdk_test_skip_cp == NULL) {
+ ret = -1;
+ goto out;
+ }
+ dpdk_test_skip = dpdk_test_skip_cp;
+ split_ret = rte_strsplit(dpdk_test_skip, strlen(dpdk_test_skip),
+ skip_tests, RTE_DIM(skip_tests), ',');
+ if (split_ret > 0)
+ n_skip_tests = split_ret;
+ else
+ free(dpdk_test_skip);
+ }
cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
if (cl == NULL) {
@@ -201,6 +220,15 @@ main(int argc, char **argv)
}
for (i = 0; i < test_count; i++) {
+ /* check if test is to be skipped */
+ for (size_t j = 0; j < n_skip_tests; j++) {
+ if (strcmp(tests[i], skip_tests[j]) == 0) {
+ fprintf(stderr, "Skipping %s [DPDK_TEST_SKIP]\n", tests[i]);
+ ret = TEST_SKIPPED;
+ goto end_of_cmd;
+ }
+ }
+
snprintf(buf, sizeof(buf), "%s\n", tests[i]);
if (cmdline_parse_check(cl, buf) < 0) {
printf("Error: invalid test command: '%s'\n", tests[i]);
@@ -211,9 +239,13 @@ main(int argc, char **argv)
} else
ret = last_test_result;
+end_of_cmd:
if (ret != 0)
break;
}
+ if (n_skip_tests > 0)
+ free(dpdk_test_skip);
+
cmdline_free(cl);
goto out;
} else {
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3] app/test: add support for skipping tests
2023-08-31 10:18 ` [PATCH v3] " Bruce Richardson
@ 2023-08-31 13:58 ` Bruce Richardson
0 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-08-31 13:58 UTC (permalink / raw)
To: dev, roretzla; +Cc: Thomas Monjalon
On Thu, Aug 31, 2023 at 11:18:06AM +0100, Bruce Richardson wrote:
> When called from automated tools, like meson test, it is often useful to
> skip tests in a test suite, without having to alter the test build. To
> do so, we add support for DPDK_TEST_SKIP environment variable, where one
> can provide a comma-separated list of tests. When the test binary is
> called to run one of the tests on the list via either cmdline parameter
> or environment variable (as done with meson test), the test will not
> actually be run, but will be reported skipped.
>
> Example run:
> $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> ...
> 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
>
> Ok: 7
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 2
> Timeout: 0
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>
+Tyler
I see this set is failing CI checks due to breaking Windows builds. The
issue seems to be the use of the "strdup" function. I notice in the log
library, that we have a "#define strdup _strdup" macro. Since strdup is
fairly common, widespread function, I think we should consider a more
general approach to it.
Tyler, looking for your input here: should we just globally define strdup
as _strdup for windows in DPDK? Alternatively, some googling indicates that
there is the "_CRT_NONSTDC_NO_DEPRECATE" define which could be used to
enable a whole range of POSIX functions. Should we, or could we, just set
this to ease porting of code over? I'd hate each of our C files to have a
bunch of duplicated #defines at the start to prefix standard unix functions
with "_"s.
Thoughts?
/Bruce
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4] app/test: add support for skipping tests
2023-08-17 10:58 [RFC PATCH] app/test: add support for skipping tests Bruce Richardson
` (3 preceding siblings ...)
2023-08-31 10:18 ` [PATCH v3] " Bruce Richardson
@ 2023-09-14 15:16 ` Bruce Richardson
2023-09-25 16:26 ` Aaron Conole
4 siblings, 1 reply; 15+ messages in thread
From: Bruce Richardson @ 2023-09-14 15:16 UTC (permalink / raw)
To: dev; +Cc: aconole, Bruce Richardson, Thomas Monjalon
When called from automated tools, like meson test, it is often useful to
skip tests in a test suite, without having to alter the test build. To
do so, we add support for DPDK_TEST_SKIP environment variable, where one
can provide a comma-separated list of tests. When the test binary is
called to run one of the tests on the list via either cmdline parameter
or environment variable (as done with meson test), the test will not
actually be run, but will be reported skipped.
Example run:
$ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
...
1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
2/9 DPDK:debug-tests / dump_log_types OK 1.06s
3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
5/9 DPDK:debug-tests / dump_mempool OK 1.11s
6/9 DPDK:debug-tests / dump_memzone OK 1.06s
7/9 DPDK:debug-tests / dump_physmem OK 1.13s
8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
Ok: 7
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 2
Timeout: 0
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
V4: Add os shim header, to avoid issues with use of strdup on windows
V3: Fix checkpatch whitespace issue
V2: Check return value from rte_strsplit, just in case.
---
app/test/test.c | 32 ++++++++++++++++++++++++++++++++
app/test/test.h | 1 +
2 files changed, 33 insertions(+)
diff --git a/app/test/test.c b/app/test/test.c
index fb073ff795..bfa9ea52e3 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -193,6 +193,25 @@ main(int argc, char **argv)
if (test_count > 0) {
char buf[1024];
+ char *dpdk_test_skip = getenv("DPDK_TEST_SKIP");
+ char *skip_tests[128] = {0};
+ size_t n_skip_tests = 0;
+
+ if (dpdk_test_skip != NULL && strlen(dpdk_test_skip) > 0) {
+ int split_ret;
+ char *dpdk_test_skip_cp = strdup(dpdk_test_skip);
+ if (dpdk_test_skip_cp == NULL) {
+ ret = -1;
+ goto out;
+ }
+ dpdk_test_skip = dpdk_test_skip_cp;
+ split_ret = rte_strsplit(dpdk_test_skip, strlen(dpdk_test_skip),
+ skip_tests, RTE_DIM(skip_tests), ',');
+ if (split_ret > 0)
+ n_skip_tests = split_ret;
+ else
+ free(dpdk_test_skip);
+ }
cl = cmdline_new(main_ctx, "RTE>>", 0, 1);
if (cl == NULL) {
@@ -201,6 +220,15 @@ main(int argc, char **argv)
}
for (i = 0; i < test_count; i++) {
+ /* check if test is to be skipped */
+ for (size_t j = 0; j < n_skip_tests; j++) {
+ if (strcmp(tests[i], skip_tests[j]) == 0) {
+ fprintf(stderr, "Skipping %s [DPDK_TEST_SKIP]\n", tests[i]);
+ ret = TEST_SKIPPED;
+ goto end_of_cmd;
+ }
+ }
+
snprintf(buf, sizeof(buf), "%s\n", tests[i]);
if (cmdline_parse_check(cl, buf) < 0) {
printf("Error: invalid test command: '%s'\n", tests[i]);
@@ -211,9 +239,13 @@ main(int argc, char **argv)
} else
ret = last_test_result;
+end_of_cmd:
if (ret != 0)
break;
}
+ if (n_skip_tests > 0)
+ free(dpdk_test_skip);
+
cmdline_free(cl);
goto out;
} else {
diff --git a/app/test/test.h b/app/test/test.h
index a91ded76af..eede583cf9 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -12,6 +12,7 @@
#include <rte_hexdump.h>
#include <rte_common.h>
+#include <rte_os_shim.h>
#define TEST_SUCCESS EXIT_SUCCESS
#define TEST_FAILED -1
--
2.39.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4] app/test: add support for skipping tests
2023-09-14 15:16 ` [PATCH v4] " Bruce Richardson
@ 2023-09-25 16:26 ` Aaron Conole
2023-10-02 9:20 ` David Marchand
0 siblings, 1 reply; 15+ messages in thread
From: Aaron Conole @ 2023-09-25 16:26 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, Thomas Monjalon
Bruce Richardson <bruce.richardson@intel.com> writes:
> When called from automated tools, like meson test, it is often useful to
> skip tests in a test suite, without having to alter the test build. To
> do so, we add support for DPDK_TEST_SKIP environment variable, where one
> can provide a comma-separated list of tests. When the test binary is
> called to run one of the tests on the list via either cmdline parameter
> or environment variable (as done with meson test), the test will not
> actually be run, but will be reported skipped.
>
> Example run:
> $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> ...
> 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
>
> Ok: 7
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 2
> Timeout: 0
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>
> ---
Acked-by: Aaron Conole <aconole@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-09-25 16:26 ` Aaron Conole
@ 2023-10-02 9:20 ` David Marchand
2023-10-03 20:22 ` Patrick Robb
0 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2023-10-02 9:20 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, Thomas Monjalon, Aaron Conole
On Mon, Sep 25, 2023 at 6:27 PM Aaron Conole <aconole@redhat.com> wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
>
> > When called from automated tools, like meson test, it is often useful to
> > skip tests in a test suite, without having to alter the test build. To
> > do so, we add support for DPDK_TEST_SKIP environment variable, where one
> > can provide a comma-separated list of tests. When the test binary is
> > called to run one of the tests on the list via either cmdline parameter
> > or environment variable (as done with meson test), the test will not
> > actually be run, but will be reported skipped.
> >
> > Example run:
> > $ DPDK_TEST_SKIP=dump_devargs,dump_ring meson test --suite=debug-tests
> > ...
> > 1/9 DPDK:debug-tests / dump_devargs SKIP 1.11s
> > 2/9 DPDK:debug-tests / dump_log_types OK 1.06s
> > 3/9 DPDK:debug-tests / dump_malloc_heaps OK 1.11s
> > 4/9 DPDK:debug-tests / dump_malloc_stats OK 1.07s
> > 5/9 DPDK:debug-tests / dump_mempool OK 1.11s
> > 6/9 DPDK:debug-tests / dump_memzone OK 1.06s
> > 7/9 DPDK:debug-tests / dump_physmem OK 1.13s
> > 8/9 DPDK:debug-tests / dump_ring SKIP 1.04s
> > 9/9 DPDK:debug-tests / dump_struct_sizes OK 1.10s
> >
> > Ok: 7
> > Expected Fail: 0
> > Fail: 0
> > Unexpected Pass: 0
> > Skipped: 2
> > Timeout: 0
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Aaron Conole <aconole@redhat.com>
Applied, thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-10-02 9:20 ` David Marchand
@ 2023-10-03 20:22 ` Patrick Robb
2023-10-04 15:13 ` Aaron Conole
0 siblings, 1 reply; 15+ messages in thread
From: Patrick Robb @ 2023-10-03 20:22 UTC (permalink / raw)
To: David Marchand; +Cc: Bruce Richardson, dev, Thomas Monjalon, Aaron Conole
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
Thanks, this should help greatly going forward in the community lab.
As it relates to our arm64 unit testing, I will give it a few days (or
longer if needed) for next branches to rebase off of main and then
re-enable arm64 unit testing with the eal_flags_file_prefix_autotest added
to the skipped list. David explained to me on slack that this patch would
not likely be a candidate for backporting, so of course LTS will be
excluded.
[-- Attachment #2: Type: text/html, Size: 486 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-10-03 20:22 ` Patrick Robb
@ 2023-10-04 15:13 ` Aaron Conole
2023-10-09 20:03 ` Patrick Robb
0 siblings, 1 reply; 15+ messages in thread
From: Aaron Conole @ 2023-10-04 15:13 UTC (permalink / raw)
To: Patrick Robb
Cc: David Marchand, Bruce Richardson, dev, Thomas Monjalon, Kevin Traynor
Patrick Robb <probb@iol.unh.edu> writes:
> Thanks, this should help greatly going forward in the community lab.
>
> As it relates to our arm64 unit testing, I will give it a few days (or longer if needed) for next branches to rebase off of
> main and then re-enable arm64 unit testing with the eal_flags_file_prefix_autotest added to the skipped list. David
> explained to me on slack that this patch would not likely be a candidate for backporting, so of course LTS will be
> excluded.
This is in testing area, and maybe it can be considered as an exception
if it allows for improved LTS testing. CC'd Kevin.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-10-04 15:13 ` Aaron Conole
@ 2023-10-09 20:03 ` Patrick Robb
2023-10-20 15:02 ` Patrick Robb
0 siblings, 1 reply; 15+ messages in thread
From: Patrick Robb @ 2023-10-09 20:03 UTC (permalink / raw)
To: Aaron Conole
Cc: David Marchand, Bruce Richardson, dev, Thomas Monjalon,
Kevin Traynor, Lincoln Lavoie
[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]
On Wed, Oct 4, 2023 at 11:13 AM Aaron Conole <aconole@redhat.com> wrote:
> Patrick Robb <probb@iol.unh.edu> writes:
>
> > Thanks, this should help greatly going forward in the community lab.
> >
> > As it relates to our arm64 unit testing, I will give it a few days (or
> longer if needed) for next branches to rebase off of
> > main and then re-enable arm64 unit testing with the
> eal_flags_file_prefix_autotest added to the skipped list. David
> > explained to me on slack that this patch would not likely be a candidate
> for backporting, so of course LTS will be
> > excluded.
>
> This is in testing area, and maybe it can be considered as an exception
> if it allows for improved LTS testing. CC'd Kevin.
>
> Hello,
Yes, backporting would be ideal from a CI perspective because without it we
can't run arm64 testing on LTS tests. But I know there are other
considerations which also have to be weighed.
David also has a patch[1] which should resolve the underlying issue which
introduces the failures on the unit test we want to skip. If that patch is
accepted, and backported, fixing our original problem with unit testing on
our arm testbeds, that's another solution, at least for this specific unit
test issue.
It would still be nice to have this feature in case we need it otherwise.
[1]
https://patches.dpdk.org/project/dpdk/patch/20230821085806.3062613-4-david.marchand@redhat.com/
[-- Attachment #2: Type: text/html, Size: 2060 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-10-09 20:03 ` Patrick Robb
@ 2023-10-20 15:02 ` Patrick Robb
2023-10-20 15:08 ` Bruce Richardson
0 siblings, 1 reply; 15+ messages in thread
From: Patrick Robb @ 2023-10-20 15:02 UTC (permalink / raw)
To: Aaron Conole
Cc: David Marchand, Bruce Richardson, dev, Thomas Monjalon,
Kevin Traynor, Lincoln Lavoie
[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]
On Mon, Oct 9, 2023 at 4:03 PM Patrick Robb <probb@iol.unh.edu> wrote:
>
>> Hello,
>
> Yes, backporting would be ideal from a CI perspective because without it
> we can't run arm64 testing on LTS tests. But I know there are other
> considerations which also have to be weighed.
>
> David also has a patch[1] which should resolve the underlying issue which
> introduces the failures on the unit test we want to skip. If that patch is
> accepted, and backported, fixing our original problem with unit testing on
> our arm testbeds, that's another solution, at least for this specific unit
> test issue.
>
> It would still be nice to have this feature in case we need it otherwise.
>
> [1]
> https://patches.dpdk.org/project/dpdk/patch/20230821085806.3062613-4-david.marchand@redhat.com/
>
> Hi. just to close the loops on this, yes David's aforementioned patch did
resolve the unit test failure which was preventing us from running
fast-tests on our arm64 test beds. But, it is not (yet, at least)
backported for LTS releases.
Even if it were, having Bruce's patch here backported would mean the CI
testing approach could be common across releases in situations where
testcases have to be skipped.
Anyways, whether it's possible or "worth it" is ultimately down to the
community's bandwidth, but I didn't want to let the conversation lapse
without an update, and raising what the benefits would be.
In any case, thanks again Bruce for the rework, it's a great addition.
[-- Attachment #2: Type: text/html, Size: 2346 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] app/test: add support for skipping tests
2023-10-20 15:02 ` Patrick Robb
@ 2023-10-20 15:08 ` Bruce Richardson
0 siblings, 0 replies; 15+ messages in thread
From: Bruce Richardson @ 2023-10-20 15:08 UTC (permalink / raw)
To: Patrick Robb
Cc: Aaron Conole, David Marchand, dev, Thomas Monjalon,
Kevin Traynor, Lincoln Lavoie, stable
+stable on CC, to allow it be considered for possible backport. It's a
change to the unit test app, so not affecting any ABI or any end-user app.
On Fri, Oct 20, 2023 at 11:02:07AM -0400, Patrick Robb wrote:
> On Mon, Oct 9, 2023 at 4:03 PM Patrick Robb <[1]probb@iol.unh.edu>
> wrote:
>
> Hello,
> Yes, backporting would be ideal from a CI perspective because without
> it we can't run arm64 testing on LTS tests. But I know there are other
> considerations which also have to be weighed.
> David also has a patch[1] which should resolve the underlying issue
> which introduces the failures on the unit test we want to skip. If that
> patch is accepted, and backported, fixing our original problem with
> unit testing on our arm testbeds, that's another solution, at least for
> this specific unit test issue.
> It would still be nice to have this feature in case we need it
> otherwise.
> [1] [2]https://patches.dpdk.org/project/dpdk/patch/20230821085806.30626
> 13-4-david.marchand@redhat.com/
>
> Hi. just to close the loops on this, yes David's aforementioned patch
> did resolve the unit test failure which was preventing us from running
> fast-tests on our arm64 test beds. But, it is not (yet, at least)
> backported for LTS releases.
> Even if it were, having Bruce's patch here backported would mean the CI
> testing approach could be common across releases in situations where
> testcases have to be skipped.
> Anyways, whether it's possible or "worth it" is ultimately down to the
> community's bandwidth, but I didn't want to let the conversation lapse
> without an update, and raising what the benefits would be.
> In any case, thanks again Bruce for the rework, it's a great addition.
>
> References
>
> 1. mailto:probb@iol.unh.edu
> 2. https://patches.dpdk.org/project/dpdk/patch/20230821085806.3062613-4-david.marchand@redhat.com/
^ permalink raw reply [flat|nested] 15+ messages in thread