Re: [PATCH net-next v2] net: netconsole: selftests: Create a new netconsole selftest

On Wed, Aug 14, 2024 at 12:24:46PM +0200, Petr Machata wrote:

Breno Leitao leitao@debian.org writes:

...

• fi
• if ! grep -q "${MSG}" "${TMPFILENAME}"; then

•    echo "FAIL: ${MSG} not found in ${TMPFILENAME}" >&2
  

•    cat "${TMPFILENAME}" >&2
  

•    return ${ksft_fail}
  

• fi
• # Delete the file once it is validated, otherwise keep it
• # for debugging purposes
• rm "${TMPFILENAME}"

Seeing the removal within the validation function is odd, I would expect it to be part of cleanup().

Thanks for all the other feedbacks, all of them make sense.

Regarding this one, I kept like this, because I only remove the file if the test succeed, otherwise I keep the file here for debugging purposes, as described in the comment above.

If that is not a good practice, I am more than happy to move this to cleanup.

Thanks for the detailed review, --breno