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 3A5E3A00C2; Fri, 17 Jun 2022 04:42:58 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C704440698; Fri, 17 Jun 2022 04:42:57 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 87FDC40689 for ; Fri, 17 Jun 2022 04:42:55 +0200 (CEST) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4LPNb42xyZzjXbx; Fri, 17 Jun 2022 10:41:20 +0800 (CST) Received: from [127.0.0.1] (10.67.100.224) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Fri, 17 Jun 2022 10:42:19 +0800 Subject: Re: [PATCH v2 3/3] test: support trace-autotest when enable trace To: David Marchand , Aaron Conole CC: Thomas Monjalon , dev , "Burakov, Anatoly" , Jerin Jacob Kollanukkaran , Bruce Richardson References: <20220607120014.49823-1-fengchengwen@huawei.com> <20220614055900.22848-1-fengchengwen@huawei.com> <20220614055900.22848-4-fengchengwen@huawei.com> From: fengchengwen Message-ID: <382eda43-9a03-60b9-ea8c-316e44a29d1f@huawei.com> Date: Fri, 17 Jun 2022 10:42:19 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected 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 Hi David, thanks for your review. On 2022/6/15 22:00, David Marchand wrote: > On Tue, Jun 14, 2022 at 8:06 AM Chengwen Feng wrote: >> >> There are a bug[1] when exit application while enable tracing, this >> bug has not been discovered for a long time, to quickly detect such >> bugs, this patch was introduced. >> >> This patch adds a testcase with trace enabling, it also depends on >> patch[2] because it has a long file-prefix. >> >> [1] eal: fix segment fault when exit trace >> [2] eal: fix trace init fail with long file-prefix > > This commitlog feels more like a cover letter. > > Please describe what the impact of the patch is, like mention that the > trace_autotest unit test is being called twice, once with traces > disabled, and once with traces enabled. > And that the traces file is being written in a directory part of the > build directory. > > >> >> Signed-off-by: Chengwen Feng >> --- >> app/test/meson.build | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/app/test/meson.build b/app/test/meson.build >> index 7fe261cae8..eb37aa632a 100644 >> --- a/app/test/meson.build >> +++ b/app/test/meson.build >> @@ -509,6 +509,17 @@ foreach arg : fast_tests >> is_parallel : false, >> suite : 'fast-tests') >> endif >> + >> + if run_test and arg[0] == 'trace_autotest' and (not is_windows) > > Calling this test on Windows should not be an issue. > Is it not the case? I notice that eal_parse_common_option still keep windows macro for trace parameter parse: #ifndef RTE_EXEC_ENV_WINDOWS case OPT_TRACE_NUM: { ... case OPT_TRACE_MODE_NUM: { if (eal_trace_mode_args_save(optarg) < 0) { RTE_LOG(ERR, EAL, "invalid parameters for --" OPT_TRACE_MODE "\n"); return -1; } break; } #endif /* !RTE_EXEC_ENV_WINDOWS */ Maybe trace already support in windows, but I havn't the windows enviorment to verify it. So I think we should keep '(not is_windows)' currently. > > >> + test_args += ['--trace=.*'] >> + test_args += ['--trace-dir=@0@'.format(meson.current_build_dir())] >> + test(arg[0], dpdk_test, > > 58/68 DPDK:fast-tests / trace_autotest OK 0.16s > 59/68 DPDK:fast-tests / trace_autotest OK 0.16s > > By using the same test name, it's hard to tell what the difference is > between those two lines. > And practically, as a developer reproducing/troubleshooting a test > failure, you can't call only one of the test case. > > I am not sure what could be done to enhance this.., how about using a > dedicated test name? > Like below snippet, replacing the whole proposed patch: Good idea, already fix in v3 > > @@ -508,6 +508,16 @@ foreach arg : fast_tests > timeout : timeout_seconds_fast, > is_parallel : false, > suite : 'fast-tests') > + if arg[0] == 'trace_autotest' > + test_args += ['--trace=.*'] > + test_args += ['--trace-dir=@0@'.format(meson.current_build_dir())] > + test(arg[0] + '_with_traces', dpdk_test, > + env : ['DPDK_TEST=' + arg[0]], > + args : test_args, > + timeout : timeout_seconds_fast, > + is_parallel : false, > + suite : 'fast-tests') > + endif > endif > endforeach > >