Re: [PATCH] rtla: add tests for tracefs instance cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] rtla: subcase SKIP treated as group skip in run_cleanup_cases()
- [Medium] rtla: tests: invalid --version check in check-cleanup.sh skips all tests
--

commit 39ad3784406cff716e91c58adc60f6311f089ea0
Author: Chaosheng Qu <chaosheng.qu.linux@gmail.com>

rtla: add tests for tracefs instance cleanup

This commit adds a SIGTERM handler to rtla so tracefs instances are cleaned
up on termination, along with a new test suite verifying tracefs instance
cleanup across abnormal exit paths.

> diff --git a/tools/tracing/rtla/tests/cleanup.t b/tools/tracing/rtla/tests/cleanup.t
> new file mode 100644
> index 0000000000000..f3b495b0aee4e
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/cleanup.t
> @@ -0,0 +1,71 @@

[ ... ]

> +run_cleanup_cases() {

[ ... ]

> +	out=$($timeout_cmd "$helper" "$RTLA" 2>&1)
> +	rc=$?
> +	if [ $rc -eq 0 ]; then
> +		# The helper may skip individual cases; a group-level SKIP means it
> +		# could not run at all.
> +		skip=$(printf '%s\n' "$out" | sed -n 's/^SKIP: //p' | head -n 1)
> +		if [ -n "$skip" ]; then
> +			echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure # SKIP $skip"
> +			return
> +		fi
> +		printf '%s\n' "$out" | grep '^SKIP: ' | while read -r line; do
> +			echo "# ${line}"
> +		done
> +		echo "ok $ctr - instances cleaned up on SIGINT, timeout and action failure"

[Severity: Medium]
In run_cleanup_cases(), sed -n 's/^SKIP: //p' matches any line in $out
starting with "SKIP: ". When the action-failure subtest in
tools/tracing/rtla/tests/scripts/check-cleanup.sh does not cross the
stop-total threshold on an idle system, it logs:

	if [ "$action_rc" -eq 0 ]; then
		log "SKIP: action-failure: stop-total threshold was not crossed"

Won't $skip be populated from that subtest skip even when all other subtests
ran and passed, causing run_cleanup_cases() to report the entire TAP test as
skipped and leaving the grep '^SKIP: ' loop right below it unreachable?

[ ... ]

> diff --git a/tools/tracing/rtla/tests/scripts/check-cleanup.sh b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
> new file mode 100755
> index 0000000000000..1b4397123708e
> --- /dev/null
> +++ b/tools/tracing/rtla/tests/scripts/check-cleanup.sh
> @@ -0,0 +1,197 @@

[ ... ]

> +# Leave nothing behind from the test before it starts, and make sure the
> +# command under test is actually usable.
> +cleanup_all
> +if ! "$RTLA" --version >/dev/null 2>&1; then
> +	log "SKIP: $RTLA is not usable"
> +	exit 0
> +fi

[Severity: Medium]
Does rtla support the --version flag? Looking at main() in
tools/tracing/rtla/src/cli.c:

	if (strcmp(argv[1], "-h") == 0)
		rtla_usage(129);
	else if (strcmp(argv[1], "--help") == 0)
		rtla_usage(129);

	retval = run_tool_command(argc, argv, 1);
	if (retval)
		exit(0);

usage:
	rtla_usage(129);

It looks like passing --version falls through to rtla_usage(129) and exits
with status 129. Won't this check always fail and cause check-cleanup.sh to
skip the entire test suite on every run?

[ ... ]

────
To understand more about how the conclusions weredrawn,Normally:
- Stage 1 (goal): VF1
- Stage 2 (implementation): PVF1
────

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179229581980.42222.4083144248804063295@gmail.com?part=1