DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Marat Khalili <marat.khalili@huawei.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>,
	 Bruce Richardson <bruce.richardson@intel.com>,
	Robin Jarry <rjarry@redhat.com>
Subject: Re: [PATCH v2 02/10] test/telemetry: fix test calling all commands
Date: Thu, 3 Jul 2025 16:09:43 +0200	[thread overview]
Message-ID: <CAJFAV8xryLsxkLJMCDT5-o4s2-fMU5G2bcX066WEXaT=aXQG6Q@mail.gmail.com> (raw)
In-Reply-To: <afe059471e604b5c9319cb7312e69c23@huawei.com>

On Thu, Jun 26, 2025 at 11:53 AM Marat Khalili <marat.khalili@huawei.com> wrote:
> > @@ -15,7 +15,7 @@ call_all_telemetry() {
> >      telemetry_script=$rootdir/usertools/dpdk-telemetry.py
> >      echo >$tmpoutput
> >      echo "Telemetry commands log:" >>$tmpoutput
> > -    for cmd in $(echo / | $telemetry_script | jq -r '.["/"][]')
> > +    echo / | $telemetry_script | jq -r '.["/"][]' | while read cmd
> >      do
> >          for input in $cmd $cmd,0 $cmd,z
> >          do
> > @@ -25,4 +25,5 @@ call_all_telemetry() {
> >      done
> >  }
> >
> > -(sleep 1 && call_all_telemetry && echo quit) | $@
> > +! set -o | grep -q pipefail || set -o pipefail
> > +(set -e; ! set -o | grep -q pipefail || set -o pipefail; sleep 1 &&
> > call_all_telemetry && echo quit) | $@
>
> I 100% agree with the idea, but sadly I'm not familiar with shell scripting enough to suggest or review this diff. Is `for cmd in` always equivalent to `while read cmd`? Is CI ever executing it in bash for our attempt to set pipefail to be justified? Is it idiomatic? I hope someone else here can help with this.

- From my experiment, the difference between 'for cmd in $(xxx)' and
'xxx | while read cmd' is that an error is not propagated in the
former case.

I suppose it has to do with the for loop, as described in POSIX:
"""
Exit Status

If there is at least one item in the list of items, the exit status of
a for command shall be the exit status of the last compound-list
executed. If there are no items, the exit status shall be zero.
"""

On error of the command, there is no item in the list of the for loop,
so the loop is overall evaluated as a success.


- As far as the CI is concened, the unit tests are run on various
distributions, including Fedora at UNH.
Since the default shell for Fedora is bash, then an error would be caught there.


>
> Perhaps this should just be re-written in Python. It depends on a Python script anyway.

Maybe.
Though if we go that way, we would need some refactoring of the
telemetry client script, defining some python class etc...


-- 
David Marchand


  reply	other threads:[~2025-07-03 14:10 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-19  7:10 [PATCH 00/10] Run with UBSan in GHA David Marchand
2025-06-19  7:10 ` [PATCH 01/10] ci: save ccache on failure David Marchand
2025-06-25 12:16   ` Aaron Conole
2025-06-19  7:10 ` [PATCH 02/10] test/telemetry: fix test calling all commands David Marchand
2025-06-20  9:16   ` Bruce Richardson
2025-06-23  9:54   ` David Marchand
2025-06-19  7:10 ` [PATCH 03/10] test/mempool: fix test without stack driver David Marchand
2025-06-20  8:54   ` Andrew Rybchenko
2025-06-19  7:10 ` [PATCH 04/10] eal: fix plugin dir walk David Marchand
2025-06-20  9:19   ` Bruce Richardson
2025-06-23  9:41     ` David Marchand
2025-06-19  7:10 ` [PATCH 05/10] cmdline: fix port list parsing David Marchand
2025-06-20  9:58   ` Bruce Richardson
2025-06-23  9:40     ` David Marchand
2025-06-23 10:41       ` Bruce Richardson
2025-06-19  7:10 ` [PATCH 06/10] cmdline: fix highest bit " David Marchand
2025-06-20  9:21   ` Bruce Richardson
2025-06-23  9:32     ` David Marchand
2025-06-19  7:10 ` [PATCH 07/10] tailq: fix cast macro for null pointer David Marchand
2025-06-20  9:23   ` Bruce Richardson
2025-06-19  7:10 ` [PATCH 08/10] hash: fix unaligned access in predictable RSS David Marchand
2025-06-19  7:10 ` [PATCH 09/10] stack: fix unaligned accesses on 128-bit David Marchand
2025-06-19  7:10 ` [PATCH 10/10] build: support Undefined Behavior Sanitizer David Marchand
2025-06-25 12:17   ` Aaron Conole
2025-06-23 13:52 ` [PATCH v2 00/10] Run with UBSan in GHA David Marchand
2025-06-23 13:52   ` [PATCH v2 01/10] ci: save ccache on failure David Marchand
2025-06-23 13:52   ` [PATCH v2 02/10] test/telemetry: fix test calling all commands David Marchand
2025-06-24 15:59     ` Marat Khalili
2025-06-26  8:32       ` David Marchand
2025-06-26  9:51         ` Marat Khalili
2025-07-03 14:09           ` David Marchand [this message]
2025-07-03 15:08             ` Marat Khalili
2025-06-23 13:52   ` [PATCH v2 03/10] test/mempool: fix test without stack driver David Marchand
2025-06-24 16:21     ` Marat Khalili
2025-06-23 13:52   ` [PATCH v2 04/10] eal: fix plugin dir walk David Marchand
2025-06-25  8:43     ` Marat Khalili
2025-07-03 14:27       ` David Marchand
2025-06-23 13:52   ` [PATCH v2 05/10] cmdline: fix port list parsing David Marchand
2025-06-23 14:00     ` Bruce Richardson
2025-06-26  9:32     ` Marat Khalili
2025-06-23 13:52   ` [PATCH v2 06/10] cmdline: fix highest bit " David Marchand
2025-06-30 15:25     ` Marat Khalili
2025-06-23 13:52   ` [PATCH v2 07/10] tailq: fix cast macro for null pointer David Marchand
2025-06-30 16:06     ` Marat Khalili
2025-06-23 13:52   ` [PATCH v2 08/10] hash: fix unaligned access in predictable RSS David Marchand
2025-06-30 15:32     ` Bruce Richardson
2025-07-01  8:36     ` Konstantin Ananyev
2025-06-23 13:52   ` [PATCH v2 09/10] stack: fix unaligned accesses on 128-bit David Marchand
2025-06-30 15:33     ` Bruce Richardson
2025-06-23 13:52   ` [PATCH v2 10/10] build: support Undefined Behavior Sanitizer David Marchand

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='CAJFAV8xryLsxkLJMCDT5-o4s2-fMU5G2bcX066WEXaT=aXQG6Q@mail.gmail.com' \
    --to=david.marchand@redhat.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=marat.khalili@huawei.com \
    --cc=rjarry@redhat.com \
    --cc=stable@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).