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 3374AA0562; Wed, 14 Apr 2021 12:02:17 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1CFD916192B; Wed, 14 Apr 2021 12:02:17 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 3BA1E161936 for ; Wed, 14 Apr 2021 12:02:15 +0200 (CEST) IronPort-SDR: R4IxXZhs4+/3ivz/FdsSyygWHuiPamx7RBKmI4jAHQ7JyFrt/tmSJNvMKxp8ttrXqp1Eo/403X uIecSLc9RRzg== X-IronPort-AV: E=McAfee;i="6200,9189,9953"; a="194718737" X-IronPort-AV: E=Sophos;i="5.82,221,1613462400"; d="scan'208";a="194718737" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2021 03:02:14 -0700 IronPort-SDR: brj93b1G0dfvpL4B1cnwMVoIIPfA9czP60ftvNYNIYkj6RgCNkRhdCF79+3ESU6ULHBkT8jrJp cWs19vPwnthA== X-IronPort-AV: E=Sophos;i="5.82,221,1613462400"; d="scan'208";a="424661824" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.7.48]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 14 Apr 2021 03:02:13 -0700 Date: Wed, 14 Apr 2021 11:02:09 +0100 From: Bruce Richardson To: David Marchand Cc: dev , Aaron Conole Message-ID: <20210414100209.GB500@bricha3-MOBL.ger.corp.intel.com> References: <20210127174255.1671738-1-bruce.richardson@intel.com> <20210409134111.GB1381@bricha3-MOBL.ger.corp.intel.com> <20210413164837.GB1185@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH] test: allow taking test names from commandline 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 Sender: "dev" On Wed, Apr 14, 2021 at 08:12:50AM +0200, David Marchand wrote: > On Tue, Apr 13, 2021 at 6:49 PM Bruce Richardson > wrote: > > > > On Fri, Apr 09, 2021 at 02:41:11PM +0100, Bruce Richardson wrote: > > > On Fri, Apr 09, 2021 at 03:27:17PM +0200, David Marchand wrote: > > > > On Wed, Jan 27, 2021 at 6:43 PM Bruce Richardson > > > > wrote: > > > > > > > > > > While having the ability to run a test based off the DPDK_TEST environment > > > > > variable is useful, it's often easier to specify the test name as a > > > > > commandline parameter to a test binary. This also allows the test runs to > > > > > be saved as part of the shell cmdline history. > > > > > > > > I don't get the argument about history: > > > > > > > > $ history |grep DPDK_TEST > > > > 10615 2021-03-24 10:42:11 ninja-build -C build -j4 && > > > > DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512 > > > > --log-level=lib.eal:debug > > > > 10636 2021-03-24 10:51:09 ninja-build -C build -j4 && > > > > DPDK_TEST=logs_autotest ./build/app/test/dpdk-test --no-huge -m 512 > > > > --log-level=lib.eal:debug > > > > 10653 2021-03-24 11:17:01 ninja-build -C build -j4 && > > > > DPDK_TEST=kvargs_autotest ./build/app/test/dpdk-test --no-huge -m 512 > > > > --log-level=lib.eal:debug > > > > 10794 2021-03-25 18:37:48 history |grep DPDK_TEST > > > > > > > > > > Sure, if you always specify the test name explicitly for each command, > > > rather than running the one test multiple times having set it separately in > > > the environment. > > > Overall, though I take the point that from a history saving point of view > > > it's a minor saving. > > > > > > > > > > > > > > > > > This patch adds support for checking all parameters after the EAL ones, and > > > > > running all valid autotests requested - either from DPDK_TEST or on the > > > > > commandline. This also allows multiple tests to be run in a single > > > > > automated session, which is useful for working with components which have > > > > > multiple test suites. > > > > > > > > The same could be achieved splitting DPDK_TEST content with spaces, > > > > since test names don't contain one. > > > > > > > > > > Yep, that's a useful enhancement too, but I still thing it's better to just > > > have the list of tests appended to the test binary command rather than have > > > to be worrying about properly quoting a specific environment variable at > > > the start of each command. > > > > > Ping on this. > > I have been using the DPDK_TEST= method for quite some time. > But I don't think I ever had a need to run multiple tests since I > usually track regressions or hard to reproduce failures. > When developing a new test, idem, I used DPDK_TEST=. > > > > > > Other instances where using cmdline is preferred over environment > > > > * Calling tests using sudo. > > I guess calling a shell with the same command would work, something like: > $ sudo sh -c 'DPDK_TEST=debug_autotest ..../build/app/test/dpdk-test > --no-huge -m 512' Not exactly the simplest command, compared to just "sudo /path/to/dpdk-test --no-huge -m512 -- debug_autotest" Using sudo -E is what I tend to prefer in this case. :-) > > > * Calling tests as an execute action when doing a git rebase > > I am pretty sure I used DPDK_TEST= in rebase execute actions in the past too. > This probably works too: > $ git rebase -i HEAD^ -x 'DPDK_TEST=debug_autotest > build/app/test/dpdk-test --no-huge -m 512' > > > > > > Yes, again in both cases, other workarounds are generally available (e.g. > > sudo -E, and exporting to environemtn before rebase), but also generally > > less convenient. > > You find it less convenient when you want to run multiple tests. > I don't have that use case but I don't mind taking it. > Can you simply adjust the commitlog, or propose a new wording I can > take when applying? > Would just replacing the first paragraph as below be suitable? Since it's only a single sentence, the second paragraph can also be merged straight in with it to leave a 1-paragraph commit log message. "While having the ability to run a test based off the DPDK_TEST environment variable is useful, it's sometimes more convenient to specify the test name as a commandline parameter to a test binary." /Bruce