Re: [PATCH net-next, v3] selftest: Add selftest for multicast address notifications

[Yuyang Huang]

Thanks for the suggestion.

>Perhaps move these to lib.sh as a separate commit?
>They seem generic.

I am looking at the existing test cases, and it seems that each case
is doing its own way of handling the end_test()/run_cmd(). It's
non-trivial to unify everything into lib.sh, and it seems to be a huge
refactor if we want to do it this way. I can also imagine each test
case might want to customize the behavior a little bit differently.

On the other hand, it seems some of the helper functions I copied over
can be simplified. I will refactor the code a little bit to reduce the
duplication.

Thanks,
Yuyang


On Sat, Jun 14, 2025 at 9:29 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> On Thu, 12 Jun 2025 11:05:14 +0900 Yuyang Huang wrote:
> > +VERBOSE=0
> > +PAUSE=no
> > +PAUSE_ON_FAIL=no
> > +
> > +source lib.sh
> > +
> > +# set global exit status, but never reset nonzero one.
> > +check_err()
> > +{
> > +     if [ $ret -eq 0 ]; then
> > +             ret=$1
> > +     fi
> > +     [ -n "$2" ] && echo "$2"
> > +}
> > +
> > +run_cmd_common()
> > +{
> > +     local cmd="$*"
> > +     local out
> > +     if [ "$VERBOSE" = "1" ]; then
> > +             echo "COMMAND: ${cmd}"
> > +     fi
> > +     out=$($cmd 2>&1)
> > +     rc=$?
> > +     if [ "$VERBOSE" = "1" -a -n "$out" ]; then
> > +             echo "    $out"
> > +     fi
> > +     return $rc
> > +}
> > +
> > +run_cmd() {
> > +     run_cmd_common "$@"
> > +     rc=$?
> > +     check_err $rc
> > +     return $rc
> > +}
> > +
> > +end_test()
> > +{
> > +     echo "$*"
> > +     [ "${VERBOSE}" = "1" ] && echo
> > +
> > +     if [[ $ret -ne 0 ]] && [[ "${PAUSE_ON_FAIL}" = "yes" ]]; then
> > +             echo "Hit enter to continue"
> > +             read a
> > +     fi;
> > +
> > +     if [ "${PAUSE}" = "yes" ]; then
> > +             echo "Hit enter to continue"
> > +             read a
> > +     fi
> > +
> > +}
>
> Perhaps move these to lib.sh as a separate commit?
> They seem generic.
>
> Please fix the shellcheck warnings. The "info"-level messages
> are up you, SC2317 can be ignored.
> --
> pw-bot: cr