Re: [PATCH v2 02/15] tools/rv: Skip empty pid error in selftest if command failed

[bot+bpf-ci]

> tools/rv: Skip empty pid error in selftest if command failed

TL;DR: Commit message only: the parenthetical "(exit code not what was
expected)" looks inverted relative to the code, which reports the empty pid
only when the exit code matches the expected one. No code bug.

>     tools/rv: Skip empty pid error in selftest if command failed
>
>     Some rv userspace selftests rely on the pid of the rv process to match
>     with the output and fail prematurely in case the pid was not found.
>     If the rv process failed, however, it's likely it will terminate before
>     catching the pid, and the test reports empty pid ignoring the error that
>     caused the process to fail.
>
>     Report empty pid only when the rv process succeeds (exit code not what
>     was expected) and continue showing exit code and output otherwise.
>
>     Signed-off-by: Gabriele Monaco <gmonaco@xxxxxxxxxx>

This isn't a bug in the code, but the parenthetical in the commit message
looks inverted.  Is "(exit code not what was expected)" meant to read
"(exit code is what was expected)"?

The patch adds this condition to the empty-pid check in _check():

    if [ -z "$pid" ] && grep -q "\$pid" <<< "$patterns" \
            && [ $exitcode -eq "$expected_exitcode" ]; then
            result=''
            failure "# Empty pid for $command"
            return 1
    fi

so "# Empty pid for $command" is reported only when the exit code equals the
expected exit code.  When the exit code differs, the check is skipped and the
"# Expected exit code" failure is still recorded along with the output.

As written, the message describes the pre-patch behaviour that the change
removes.  Something like this would match the code:

    Report empty pid only when the rv process succeeds (exit code is what
    was expected) and continue showing exit code and output otherwise.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36944413541