[PATCH 0/2] kselftest/tty: Improve integration with automated systems

[PATCH 0/2] kselftest/tty: Improve integration with automated systems

[Mark Brown]

This cleans up the output of the tty_tstamp_update selftest to play a
bit more nicely with automated systems parsing the test output.

To do this I've also added a new helper ksft_test_result() which takes a
KSFT_ code as a report, this is something I've wanted on other occasions
but restructured things to avoid needing it.  This time I figured I'd
just add it since it keeps coming up.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Mark Brown (2):
      kselftest: Add mechanism for reporting a KSFT_ result code
      kselftest/tty: Report a consistent test name for the one test we run

 tools/testing/selftests/kselftest.h             | 22 ++++++++++++
 tools/testing/selftests/tty/tty_tstamp_update.c | 48 +++++++++++++++++--------
 2 files changed, 55 insertions(+), 15 deletions(-)
---
base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
change-id: 20240305-kselftest-tty-tname-5411444ce037

Best regards,
-- 
Mark Brown <broonie@kernel.org>

[PATCH 1/2] kselftest: Add mechanism for reporting a KSFT_ result code

[Mark Brown]

Currently there's no helper which a test can use to report it's result as
a KSFT_ result code, we can report a boolean pass/fail but not a skip. This
is sometimes a useful idiom so let's add a helper ksft_test_result_report()
which translates into the relevant report types.

Due to the use of va_args in the result reporting functions this is done as
a macro rather than an inline function as one might expect, none of the
alternatives looked particularly great.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/kselftest.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index a781e6311810..9bc130c269d7 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -20,6 +20,7 @@
  * and finally report the pass/fail/skip/xfail state of the test with one of:
  *
  *     ksft_test_result(condition, fmt, ...);
+ *     ksft_test_result_report(result, fmt, ...);
  *     ksft_test_result_pass(fmt, ...);
  *     ksft_test_result_fail(fmt, ...);
  *     ksft_test_result_skip(fmt, ...);
@@ -254,6 +255,27 @@ static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...)
 	va_end(args);
 }
 
+/**
+ * ksft_test_result() - Report test success based on truth of condition
+ *
+ * @condition: if true, report test success, otherwise failure.
+ */
+#define ksft_test_result_report(result, fmt, ...) do {		\
+	switch (result) {					\
+	case KSFT_PASS:						\
+		ksft_test_result_pass(fmt, ##__VA_ARGS__);	\
+		break;						\
+	case KSFT_FAIL:						\
+		ksft_test_result_fail(fmt, ##__VA_ARGS__);	\
+		break;						\
+	case KSFT_XFAIL:					\
+		ksft_test_result_xfail(fmt, ##__VA_ARGS__);	\
+		break;						\
+	case KSFT_SKIP:						\
+		ksft_test_result_skip(fmt, ##__VA_ARGS__);	\
+		break;						\
+	} } while (0)
+
 static inline int ksft_exit_pass(void)
 {
 	ksft_print_cnts();

-- 
2.30.2

[PATCH 2/2] kselftest/tty: Report a consistent test name for the one test we run

[Mark Brown]

Currently the tty_tstamp_update test reports a different exit message
for every path it can exit via. This can be confusing for automated systems
as the string that gets logged is interpreted as a test name so if the test
status changes they can't tell that it's the same test case that was run,
they can see that the overall status of the test program is a failure but
it's not clear that it was running the same test.

Change all the messages that are logged to be diagnostic prints and log the
name of the program as the test name.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/tty/tty_tstamp_update.c | 48 +++++++++++++++++--------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/tty/tty_tstamp_update.c b/tools/testing/selftests/tty/tty_tstamp_update.c
index 0ee97943dccc..9e1a40f5db17 100644
--- a/tools/testing/selftests/tty/tty_tstamp_update.c
+++ b/tools/testing/selftests/tty/tty_tstamp_update.c
@@ -47,42 +47,60 @@ int main(int argc, char **argv)
 	int r;
 	char tty[PATH_MAX] = {};
 	struct stat st1, st2;
+	int result = KSFT_FAIL;
 
 	ksft_print_header();
 	ksft_set_plan(1);
 
 	r = readlink("/proc/self/fd/0", tty, PATH_MAX);
-	if (r < 0)
-		ksft_exit_fail_msg("readlink on /proc/self/fd/0 failed: %m\n");
+	if (r < 0) {
+		ksft_print_msg("readlink on /proc/self/fd/0 failed: %m\n");
+		goto out;
+	}
+
+	if (!tty_valid(tty)) {
+		ksft_print_msg("invalid tty path '%s'\n", tty);
+		result = KSFT_SKIP;
+		goto out;
 
-	if (!tty_valid(tty))
-		ksft_exit_skip("invalid tty path '%s'\n", tty);
+	}
 
 	r = stat(tty, &st1);
-	if (r < 0)
-		ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty);
+	if (r < 0) {
+		ksft_print_msg("stat failed on tty path '%s': %m\n", tty);
+		goto out;
+	}
 
 	/* We need to wait at least 8 seconds in order to observe timestamp change */
 	/* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fbf47635315ab308c9b58a1ea0906e711a9228de */
 	sleep(10);
 
 	r = write_dev_tty();
-	if (r < 0)
-		ksft_exit_fail_msg("failed to write to /dev/tty: %s\n",
-				   strerror(-r));
+	if (r < 0) {
+		ksft_print_msg("failed to write to /dev/tty: %s\n",
+			       strerror(-r));
+		goto out;
+	}
 
 	r = stat(tty, &st2);
-	if (r < 0)
-		ksft_exit_fail_msg("stat failed on tty path '%s': %m\n", tty);
+	if (r < 0) {
+		ksft_print_msg("stat failed on tty path '%s': %m\n", tty);
+		goto out;
+	}
 
 	/* We wrote to the terminal so timestamps should have been updated */
 	if (st1.st_atim.tv_sec == st2.st_atim.tv_sec &&
 	    st1.st_mtim.tv_sec == st2.st_mtim.tv_sec) {
-		ksft_test_result_fail("tty timestamps not updated\n");
-		ksft_exit_fail();
+		ksft_print_msg("tty timestamps not updated\n");
+		goto out;
 	}
 
-	ksft_test_result_pass(
+	ksft_print_msg(
 		"timestamps of terminal '%s' updated after write to /dev/tty\n", tty);
-	return EXIT_SUCCESS;
+	result = KSFT_PASS;
+
+out:
+	ksft_test_result_report(result, "tty_tstamp_update\n");
+
+	ksft_finished();
 }

-- 
2.30.2

Re: [PATCH 0/2] kselftest/tty: Improve integration with automated systems

[Greg Kroah-Hartman]

On Wed, Mar 06, 2024 at 07:21:24PM +0000, Mark Brown wrote:
> This cleans up the output of the tty_tstamp_update selftest to play a
> bit more nicely with automated systems parsing the test output.
> 
> To do this I've also added a new helper ksft_test_result() which takes a
> KSFT_ code as a report, this is something I've wanted on other occasions
> but restructured things to avoid needing it.  This time I figured I'd
> just add it since it keeps coming up.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 0/2] kselftest/tty: Improve integration with automated systems

[Shuah Khan]

On 3/6/24 15:51, Greg Kroah-Hartman wrote:
> On Wed, Mar 06, 2024 at 07:21:24PM +0000, Mark Brown wrote:
>> This cleans up the output of the tty_tstamp_update selftest to play a
>> bit more nicely with automated systems parsing the test output.
>>
>> To do this I've also added a new helper ksft_test_result() which takes a
>> KSFT_ code as a report, this is something I've wanted on other occasions
>> but restructured things to avoid needing it.  This time I figured I'd
>> just add it since it keeps coming up.
>>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


Thank you. Applied to linux-kselftest next for 6.10-rc1.

thanks,
-- Shuah

Re: [PATCH 0/2] kselftest/tty: Improve integration with automated systems

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On Tue, Mar 26, 2024 at 01:49:11PM -0600, Shuah Khan wrote:
> On 3/6/24 15:51, Greg Kroah-Hartman wrote:
> > On Wed, Mar 06, 2024 at 07:21:24PM +0000, Mark Brown wrote:

> > > This cleans up the output of the tty_tstamp_update selftest to play a
> > > bit more nicely with automated systems parsing the test output.
> > > 
> > > To do this I've also added a new helper ksft_test_result() which takes a
> > > KSFT_ code as a report, this is something I've wanted on other occasions
> > > but restructured things to avoid needing it.  This time I figured I'd
> > > just add it since it keeps coming up.

> > > Signed-off-by: Mark Brown <broonie@kernel.org>

> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> Thank you. Applied to linux-kselftest next for 6.10-rc1.

Thanks, but note that I sent a v2 based on v6.9-rc1 which drops the
first patch in favour of using the newly added ksft_test_result_code()
which does something similar (though with a slightly less idiomatic
API).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]