DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Aaron Conole <aconole@redhat.com>
Subject: Re: [dpdk-dev] [PATCH] test: allow taking test names from commandline
Date: Wed, 14 Apr 2021 11:02:09 +0100	[thread overview]
Message-ID: <20210414100209.GB500@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <CAJFAV8yVXpZRxCkHK3EdEYXS_incP0RPKo=9dpWC=S7jLHjEXg@mail.gmail.com>

On Wed, Apr 14, 2021 at 08:12:50AM +0200, David Marchand wrote:
> On Tue, Apr 13, 2021 at 6:49 PM Bruce Richardson
> <bruce.richardson@intel.com> 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
> > > > <bruce.richardson@intel.com> 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

      reply	other threads:[~2021-04-14 10:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27 17:42 Bruce Richardson
2021-01-29 20:22 ` Aaron Conole
2021-04-14 13:25   ` David Marchand
2021-04-09 13:27 ` David Marchand
2021-04-09 13:41   ` Bruce Richardson
2021-04-13 16:48     ` Bruce Richardson
2021-04-14  6:12       ` David Marchand
2021-04-14 10:02         ` Bruce Richardson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210414100209.GB500@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=aconole@redhat.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).