From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Vorel Date: Mon, 29 Mar 2021 20:43:40 +0200 Subject: [LTP] [PATCH 2/2] pec: Fix multiple event test In-Reply-To: <20210315092844.991073-2-lkml@jv-coder.de> References: <20210315092844.991073-1-lkml@jv-coder.de> <20210315092844.991073-2-lkml@jv-coder.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it Hi Joerg, > +free_fd() > +{ > + # Find a free file handle > + local found > + for fd in $(seq 200); do > + rco="$(true 2>/dev/null >&${fd}; echo $?)" > + rci="$(true 2>/dev/null <&${fd}; echo $?)" > + [[ "${rco}${rci}" = "11" ]] && found=${fd} && break [[ .. ]] is a bashism, single brackets are enough. > + done > + echo $found > +} NIT: adding a comment to before function and space separating section with local helps readability. # Find a free file handle free_fd() { local found for fd in $(seq 200); do ... > + > setup() > { > if ! grep -q cn_proc /proc/net/connector; then > @@ -32,35 +64,75 @@ setup() > test() > { > local event=$2 > + > + tst_res TINFO "Testing $2 event (nevents=$num_events)" > + > pec_listener >lis_$event.log 2>lis_$event.err & > - pid=$! > + lis_pid=$! This change is unnecessary, if you prefer $lis_pid, it should have been in previous patch where you added $pid (no need to repost, I can fix it before merge). > # Wait for pec_listener to start listening > tst_sleep 100ms > # Run with absolute path, so the generator can exec itself > generator="$(command -v event_generator)" > - "$generator" -n $NUM_EVENTS -e $event >gen_$event.log 2>gen_$event.err > + "$generator" -n $num_events -e $event >gen_$event.log 2>gen_$event.err > gen_rc=$? > - # Sleep until pec_listener has seen and handled all of the generated events > - tst_sleep 100ms > - kill -s SIGINT $pid 2> /dev/null > - wait $pid > + kill -s SIGINT $lis_pid 2> /dev/null > + wait $lis_pid > lis_rc=$? > if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then > - tst_brk TBROK "failed to generate process events" > + tst_brk TBROK "failed to generate process events: $(cat gen_$event.err)" > fi > if [ $lis_rc -ne 0 ]; then > tst_brk TBROK "failed to execute the listener: $(cat lis_$event.err)" > fi > - expected_events="$(cat gen_$event.log)" > - if grep -q "$expected_events" lis_$event.log; then > - tst_res TPASS "$event detected by listener" > + # The listener writes the same messages as the generator, but it can > + # also see more events (e.g. for testing exit, a fork is generated). > + # So: The events generated by the generator have to be in the same order > + # as the events printed by the listener, but my interleaved with other > + # messages. To correctly compare them, we have to open both logs > + # and iterate over both of them at the same time, skipping messages > + # in the listener log, that are not of interest. > + # Because some messages may be multiple times in the listener log, > + # we have to open it only once! > + # This however does not check, if the listener sees more messages, > + # than expected. > + > + fd_act=$(free_fd) > + [ -z "$fd_act" ] && tst_brk TBROK "No free filehandle found" > + eval "exec ${fd_act} + > + failed=0 > + act_nevents=0 Again two missing local. > + while read -r exp; do > + local found=0 > + act_nevents=$((act_nevents + 1)) > + while read -r <&${fd_act} act; do <& is a bashism. Isn't it using just stdin enough? while read -r < $fd_act act; do > + if [ "$exp" = "$act" ]; then > + found=1 > + break > + fi > + done > + if [ $found -ne 1 ]; then > + failed=1 > + tst_res TFAIL "Event was not detected by the event listener: $exp" > + break > + fi > + done + > + eval "exec ${fd_act}<&-" > + > + if [ $failed -eq 0 ]; then > + if [ $act_nevents -ne $num_events ]; then > + tst_res TBROK "Expected $num_events, but $act_nevents generated" > + else > + tst_res TPASS "All events detected" > + fi > else > - tst_res TFAIL "$event not detected by listener" > + cat lis_$event.log Why removing tst_res TFAIL? If "cat lis_$event.log" needed, why not having it in previous commit? > fi Also whole section would be probably more readable written as: if [ $failed -eq 0 ]; then tst_res TFAIL "$event not detected by listener" cat lis_$event.log return fi if [ $act_nevents -ne $num_events ]; then tst_brk TBROK "Expected $num_events, but $act_nevents generated" fi tst_res TPASS "All events detected" > } All changes suggested for shell: (FYI in: https://github.com/pevik/ltp/commits/joerg/connectors.v1.fixes) Kind regards, Petr diff --git testcases/kernel/connectors/pec/cn_pec.sh testcases/kernel/connectors/pec/cn_pec.sh index 8bbfe3a19..e0821a8ef 100755 --- testcases/kernel/connectors/pec/cn_pec.sh +++ testcases/kernel/connectors/pec/cn_pec.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (c) 2008 FUJITSU LIMITED @@ -39,14 +39,18 @@ parse_args() esac } +# Find a free file handle free_fd() { - # Find a free file handle - local found + local fd found rci rco + for fd in $(seq 200); do - rco="$(true 2>/dev/null >&${fd}; echo $?)" - rci="$(true 2>/dev/null <&${fd}; echo $?)" - [[ "${rco}${rci}" = "11" ]] && found=${fd} && break + rco="$(true 2>/dev/null >&$fd; echo $?)" + rci="$(true 2>/dev/null <&$fd; echo $?)" + if [ "${rco}${rci}" = "11" ]; then + found="$fd" + break + fi done echo $found } @@ -55,7 +59,6 @@ setup() { if ! grep -q cn_proc /proc/net/connector; then tst_brk TCONF "Process Event Connector is not supported or kernel is below 2.6.26" - exit 0; fi tst_res TINFO "Test process events connector" @@ -64,24 +67,24 @@ setup() test() { local event=$2 + local expected_events lis_rc pid tst_res TINFO "Testing $2 event (nevents=$num_events)" - pec_listener >lis_$event.log 2>lis_$event.err & - lis_pid=$! + ROD pec_listener \>lis_$event.log 2\>lis_$event.err & + pid=$! # Wait for pec_listener to start listening tst_sleep 100ms - # Run with absolute path, so the generator can exec itself - generator="$(command -v event_generator)" - "$generator" -n $num_events -e $event >gen_$event.log 2>gen_$event.err + # generator must be in PATH + ROD event_generator -n $num_events -e $event \>gen_$event.log 2\>gen_$event.err gen_rc=$? kill -s SIGINT $lis_pid 2> /dev/null wait $lis_pid lis_rc=$? - if [ $gen_rc -ne 0 -o ! -s gen_$event.log ]; then + if [ ! -s gen_$event.log ]; then tst_brk TBROK "failed to generate process events: $(cat gen_$event.err)" fi @@ -103,14 +106,14 @@ test() fd_act=$(free_fd) [ -z "$fd_act" ] && tst_brk TBROK "No free filehandle found" - eval "exec ${fd_act}