[v4] app/test: add support for skipping tests
Checks
Commit Message
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(+)
Comments
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>
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.
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.
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.
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/
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.
+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/
@@ -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 {
@@ -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