RE: [PATCH v2 02/10] test/telemetry: fix test calling all commands

[Marat Khalili <marat.khalili@huawei.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday 3 July 2025 15:10
> 
> 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.

BTW what would be the behaviour of the new code for the empty input?

> - 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...

I guess it's a balance between effort and gain, and we are not making it worse (it will still need to pass CI where we can double check the behaviour)... I now support this change, thanks for detailed explanations and research.

Acked-by: Marat Khalili <marat.khalili@huawei.com>