From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 189F841F55; Tue, 29 Aug 2023 15:27:31 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9F61940279; Tue, 29 Aug 2023 15:27:30 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id A9CE64026E for ; Tue, 29 Aug 2023 15:27:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1693315647; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=qsPiVZ7/F3IdF/hJMqRwPvYEne8c1vAXlYQQYSup4Is=; b=B7vF+H8zG3UbNG0iOb193gh7buMyGH4cvMtXmcEkq4CJzHFpDQCGIL6+1V1p32H5amLiNp btNIJXNWzWtQoIWqyn4BQDuX4FX1BfmPh+KQH28xKKRzcpNZsj5u74drKUb/wheUut9r8F SHLZ3zM4KwUC8ezEtIu9nmYJRmarg9g= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-204-jud20GAmMD6XL-YORfhxpg-1; Tue, 29 Aug 2023 09:27:25 -0400 X-MC-Unique: jud20GAmMD6XL-YORfhxpg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D311A81173A; Tue, 29 Aug 2023 13:27:24 +0000 (UTC) Received: from RHTPC1VM0NT (unknown [10.22.32.194]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5C3791121319; Tue, 29 Aug 2023 13:27:24 +0000 (UTC) From: Aaron Conole To: Bruce Richardson Cc: dev@dpdk.org, david.marchand@redhat.com, probb@iol.unh.edu Subject: Re: [RFC PATCH] app/test: add support for skipping tests References: <20230817105851.501947-1-bruce.richardson@intel.com> Date: Tue, 29 Aug 2023 09:27:23 -0400 In-Reply-To: <20230817105851.501947-1-bruce.richardson@intel.com> (Bruce Richardson's message of "Thu, 17 Aug 2023 11:58:51 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Bruce Richardson 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 > --- 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 {