All of lore.kernel.org
 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 [dpdk-dev] [PATCH] test: allow taking test names from commandline 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.