On Tue, Jun 24, 2025 at 6:00 PM Marat Khalili <marat.khal...@huawei.com> wrote:
>
> Reviewed-by: Marat Khalili <marat.khal...@huawei.com>
>
> Just an idea, in case you have another iteration: could we maybe add a small 
> check that $telemetry_script actually exists and executable to avoid similar 
> situations in the future? Can be as simple as:
>
> test -f "$telemetry_script"
> test -x "$telemetry_script"
>
> Due to -e in the first line it should fail the script of any of these tests 
> fail.

The problem lies in the use of subshell and pipes and that a failure
is not propagated.
Adding a test only the the telemetry script would not catch other
errors (like for example, if the jq command starts to spew errors).

The most elegant would be to use errtrace and pipefail options, but
the errtrace is a bashism (iow not POSIX), and pipefail is POSIX only
since 2022 and many shell (like dash in Ubuntu 22.04/24.04) don't
implement it.

We could try something like:

diff --git a/app/test/suites/test_telemetry.sh
b/app/test/suites/test_telemetry.sh
index ca6abe266e..a81b4add90 100755
--- a/app/test/suites/test_telemetry.sh
+++ b/app/test/suites/test_telemetry.sh
@@ -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) | $@



-- 
David Marchand

Reply via email to