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

[Breno Leitao <leitao@debian.org>]

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