[PATCH 0/7] kselftest new exit codes and test changes to use them

This patch series adds a new header file for kselftest defines
for individual tests to report test results, and changes to
several tests to use the new header file and exit codes.

Shuah Khan (7):
  selftests: add header file for test exit code defines
  selftests/breakpoints: change breakpoints test to use kselftest exit
    codes
  selftests/ipc: change ipc test to use kselftest exit codes
  selftests/kcmp: change kcmp test to use kselftest exit codes
  selftests/mount: change mount test to use kselftest exit codes
  selftests/ptrace: change ptrace test to use kselftest exit codes
  selftests/timers: change timers test to use kselftest exit codes

 .../selftests/breakpoints/breakpoint_test.c        | 10 +++++----
 tools/testing/selftests/ipc/msgque.c               | 26 ++++++++++++----------
 tools/testing/selftests/kcmp/kcmp_test.c           | 16 +++++++------
 tools/testing/selftests/kselftest.h                | 20 +++++++++++++++++
 .../selftests/mount/unprivileged-remount-test.c    | 10 +++++----
 tools/testing/selftests/ptrace/peeksiginfo.c       | 21 ++++++++---------
 tools/testing/selftests/timers/posix_timers.c      | 19 +++++++++-------
 7 files changed, 77 insertions(+), 45 deletions(-)
 create mode 100644 tools/testing/selftests/kselftest.h

-- 
1.9.1

Change ptrace test to use kselftest exit codes in kselftest.h
to report test results.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/ptrace/peeksiginfo.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/ptrace/peeksiginfo.c b/tools/testing/selftests/ptrace/peeksiginfo.c
index c34cd8a..4d18964 100644
--- a/tools/testing/selftests/ptrace/peeksiginfo.c
+++ b/tools/testing/selftests/ptrace/peeksiginfo.c

@@ -10,6 +10,7 @@
 #include <sys/mman.h>
 
 #include "linux/ptrace.h"
+#include "../kselftest.h"
 
 static int sys_rt_sigqueueinfo(pid_t tgid, int sig, siginfo_t *uinfo)
 {

@@ -43,7 +44,7 @@ static int sys_ptrace(int request, pid_t pid, void *addr, void *data)
 static int check_error_paths(pid_t child)
 {
 	struct ptrace_peeksiginfo_args arg;
-	int ret, exit_code = -1;
+	int ret, exit_code = EXIT_FAIL;
 	void *addr_rw, *addr_ro;
 
 	/*

@@ -95,7 +96,7 @@ static int check_error_paths(pid_t child)
 		goto out;
 	}
 
-	exit_code = 0;
+	exit_code = EXIT_PASS;
 out:
 	munmap(addr_rw, 2 * PAGE_SIZE);
 	return exit_code;

@@ -104,7 +105,7 @@ out:
 int check_direct_path(pid_t child, int shared, int nr)
 {
 	struct ptrace_peeksiginfo_args arg = {.flags = 0, .nr = nr, .off = 0};
-	int i, j, ret, exit_code = -1;
+	int i, j, ret, exit_code = EXIT_FAIL;
 	siginfo_t siginfo[SIGNR];
 	int si_code;
 

@@ -143,7 +144,7 @@ int check_direct_path(pid_t child, int shared, int nr)
 		goto out;
 	}
 
-	exit_code = 0;
+	exit_code = EXIT_PASS;
 out:
 	return exit_code;
 }

@@ -151,7 +152,7 @@ out:
 int main(int argc, char *argv[])
 {
 	siginfo_t siginfo[SIGNR];
-	int i, exit_code = 1;
+	int i, exit_code = EXIT_FAIL;
 	sigset_t blockmask;
 	pid_t child;
 

@@ -162,7 +163,7 @@ int main(int argc, char *argv[])
 	child = fork();
 	if (child == -1) {
 		err("fork() failed: %m");
-		return 1;
+		return exit_code;
 	} else if (child == 0) {
 		pid_t ppid = getppid();
 		while (1) {

@@ -170,7 +171,7 @@ int main(int argc, char *argv[])
 				break;
 			sleep(1);
 		}
-		return 1;
+		return exit_code;
 	}
 
 	/* Send signals in process-wide and per-thread queues */

@@ -185,7 +186,7 @@ int main(int argc, char *argv[])
 	}
 
 	if (sys_ptrace(PTRACE_ATTACH, child, NULL, NULL) == -1)
-		return 1;
+		return exit_code;
 
 	waitpid(child, NULL, 0);
 

@@ -207,10 +208,10 @@ int main(int argc, char *argv[])
 		goto out;
 
 	printf("PASS\n");
-	exit_code = 0;
+	exit_code = EXIT_PASS;
 out:
 	if (sys_ptrace(PTRACE_KILL, child, NULL, NULL) == -1)
-		return 1;
+		return exit_code;
 
 	waitpid(child, NULL, 0);
 

-- 
1.9.1

Change mount test to use kselftest exit codes in kselftest.h
to report test results.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/mount/unprivileged-remount-test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c
index 1b3ff2f..d3c703f 100644
--- a/tools/testing/selftests/mount/unprivileged-remount-test.c
+++ b/tools/testing/selftests/mount/unprivileged-remount-test.c

@@ -13,6 +13,8 @@
 #include <stdbool.h>
 #include <stdarg.h>
 
+#include "../kselftest.h"
+
 #ifndef CLONE_NEWNS
 # define CLONE_NEWNS 0x00020000
 #endif

@@ -45,7 +47,7 @@ static void die(char *fmt, ...)
 	va_start(ap, fmt);
 	vfprintf(stderr, fmt, ap);
 	va_end(ap);
-	exit(EXIT_FAILURE);
+	exit(EXIT_FAIL);
 }
 
 static void write_file(char *filename, char *fmt, ...)

@@ -142,7 +144,7 @@ bool test_unpriv_remount(int mount_flags, int remount_flags, int invalid_flags)
 		if (!WIFEXITED(status)) {
 			die("child did not terminate cleanly\n");
 		}
-		return WEXITSTATUS(status) == EXIT_SUCCESS ? true : false;
+		return WEXITSTATUS(status) == EXIT_PASS ? true : false;
 	}
 
 	create_and_enter_userns();

@@ -176,7 +178,7 @@ bool test_unpriv_remount(int mount_flags, int remount_flags, int invalid_flags)
 		die("remount of /tmp with invalid flags "
 		    "succeeded unexpectedly\n");
 	}
-	exit(EXIT_SUCCESS);
+	exit(EXIT_PASS);
 }
 
 static bool test_unpriv_remount_simple(int mount_flags)

@@ -238,5 +240,5 @@ int main(int argc, char **argv)
 	{
 		die("Default atime malfunctions\n");
 	}
-	return EXIT_SUCCESS;
+	return EXIT_PASS;
 }

-- 
1.9.1

Change timers test to use kselftest exit codes in kselftest.h
to report test results.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/timers/posix_timers.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index 41bd855..7ae2c3b 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c

@@ -15,6 +15,8 @@
 #include <time.h>
 #include <pthread.h>
 
+#include "../kselftest.h"
+
 #define DELAY 2
 #define USECS_PER_SEC 1000000
 

@@ -188,22 +190,23 @@ static int check_timer_create(int which)
 
 int main(int argc, char **argv)
 {
-	int err;
+	int err = EXIT_FAIL;
 
-	printf("Testing posix timers. False negative may happen on CPU execution \n");
+	printf("Testing posix timers.\n"
+		"False negative may happen on CPU execution\n");
 	printf("based timers if other threads run on the CPU...\n");
 
 	if (check_itimer(ITIMER_VIRTUAL) < 0)
-		return -1;
+		return err;
 
 	if (check_itimer(ITIMER_PROF) < 0)
-		return -1;
+		return err;
 
 	if (check_itimer(ITIMER_REAL) < 0)
-		return -1;
+		return err;
 
 	if (check_timer_create(CLOCK_THREAD_CPUTIME_ID) < 0)
-		return -1;
+		return err;
 
 	/*
 	 * It's unfortunately hard to reliably test a timer expiration

@@ -215,7 +218,7 @@ int main(int argc, char **argv)
 	 * find a better solution.
 	 */
 	if (check_timer_create(CLOCK_PROCESS_CPUTIME_ID) < 0)
-		return -1;
+		return err;
 
-	return 0;
+	return EXIT_PASS;
 }

-- 
1.9.1

Change kcmp test to use kselftest exit codes in kselftest.h
to report test results.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/kcmp/kcmp_test.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kcmp/kcmp_test.c b/tools/testing/selftests/kcmp/kcmp_test.c
index dbba408..68bc70e 100644
--- a/tools/testing/selftests/kcmp/kcmp_test.c
+++ b/tools/testing/selftests/kcmp/kcmp_test.c

@@ -17,6 +17,8 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 
+#include "../kselftest.h"
+
 static long sys_kcmp(int pid1, int pid2, int type, int fd1, int fd2)
 {
 	return syscall(__NR_kcmp, pid1, pid2, type, fd1, fd2);

@@ -34,23 +36,23 @@ int main(int argc, char **argv)
 
 	if (fd1 < 0) {
 		perror("Can't create file");
-		exit(1);
+		exit(EXIT_FAIL);
 	}
 
 	pid2 = fork();
 	if (pid2 < 0) {
 		perror("fork failed");
-		exit(1);
+		exit(EXIT_FAIL);
 	}
 
 	if (!pid2) {
 		int pid2 = getpid();
-		int ret;
+		int ret = EXIT_PASS;
 
 		fd2 = open(kpath, O_RDWR, 0644);
 		if (fd2 < 0) {
 			perror("Can't open file");
-			exit(1);
+			exit(EXIT_FAIL);
 		}
 
 		/* An example of output and arguments */

@@ -74,7 +76,7 @@ int main(int argc, char **argv)
 		if (ret) {
 			printf("FAIL: 0 expected but %d returned (%s)\n",
 				ret, strerror(errno));
-			ret = -1;
+			ret = EXIT_FAIL;
 		} else
 			printf("PASS: 0 returned as expected\n");
 

@@ -83,7 +85,7 @@ int main(int argc, char **argv)
 		if (ret) {
 			printf("FAIL: 0 expected but %d returned (%s)\n",
 				ret, strerror(errno));
-			ret = -1;
+			ret = EXIT_FAIL;
 		} else
 			printf("PASS: 0 returned as expected\n");
 

@@ -92,5 +94,5 @@ int main(int argc, char **argv)
 
 	waitpid(pid2, &status, P_ALL);
 
-	return 0;
+	return EXIT_PASS;
 }

-- 
1.9.1

Change ipc test to use kselftest exit codes in kselftest.h
to report test results. With this change this test exits with
EXIT_FAIL instead of -errno. Changed print errno in test fail
messages to not loose that information.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/ipc/msgque.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/ipc/msgque.c b/tools/testing/selftests/ipc/msgque.c
index 552f081..2812017 100644
--- a/tools/testing/selftests/ipc/msgque.c
+++ b/tools/testing/selftests/ipc/msgque.c

@@ -5,6 +5,8 @@
 #include <linux/msg.h>
 #include <fcntl.h>
 
+#include "../kselftest.h"
+
 #define MAX_MSG_SIZE		32
 
 struct msg1 {

@@ -195,58 +197,58 @@ int main(int argc, char **argv)
 
 	if (getuid() != 0) {
 		printf("Please run the test as root - Exiting.\n");
-		exit(1);
+		exit(EXIT_FAIL);
 	}
 
 	msgque.key = ftok(argv[0], 822155650);
 	if (msgque.key == -1) {
-		printf("Can't make key\n");
-		return -errno;
+		printf("Can't make key: %d\n", -errno);
+		return EXIT_FAIL;
 	}
 
 	msgque.msq_id = msgget(msgque.key, IPC_CREAT | IPC_EXCL | 0666);
 	if (msgque.msq_id == -1) {
 		err = -errno;
-		printf("Can't create queue\n");
+		printf("Can't create queue: %d\n", err);
 		goto err_out;
 	}
 
 	err = fill_msgque(&msgque);
 	if (err) {
-		printf("Failed to fill queue\n");
+		printf("Failed to fill queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = dump_queue(&msgque);
 	if (err) {
-		printf("Failed to dump queue\n");
+		printf("Failed to dump queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = check_and_destroy_queue(&msgque);
 	if (err) {
-		printf("Failed to check and destroy queue\n");
+		printf("Failed to check and destroy queue: %d\n", err);
 		goto err_out;
 	}
 
 	err = restore_queue(&msgque);
 	if (err) {
-		printf("Failed to restore queue\n");
+		printf("Failed to restore queue: %d\n", err);
 		goto err_destroy;
 	}
 
 	err = check_and_destroy_queue(&msgque);
 	if (err) {
-		printf("Failed to test queue\n");
+		printf("Failed to test queue: %d\n", err);
 		goto err_out;
 	}
-	return 0;
+	return EXIT_PASS;
 
 err_destroy:
 	if (msgctl(msgque.msq_id, IPC_RMID, 0)) {
 		printf("Failed to destroy queue: %d\n", -errno);
-		return -errno;
+		return EXIT_FAIL;
 	}
 err_out:
-	return err;
+	return EXIT_FAIL;
 }

-- 
1.9.1

Change breakpoints test to use kselftest exit codes in kselftest.h
to report test results.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/breakpoints/breakpoint_test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/breakpoint_test.c b/tools/testing/selftests/breakpoints/breakpoint_test.c
index a0743f3..d4cc2bb 100644
--- a/tools/testing/selftests/breakpoints/breakpoint_test.c
+++ b/tools/testing/selftests/breakpoints/breakpoint_test.c

@@ -17,6 +17,8 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 
+#include "../kselftest.h"
+
 
 /* Breakpoint access modes */
 enum {

@@ -42,7 +44,7 @@ static void set_breakpoint_addr(void *addr, int n)
 		     offsetof(struct user, u_debugreg[n]), addr);
 	if (ret) {
 		perror("Can't set breakpoint addr\n");
-		exit(-1);
+		exit(EXIT_FAIL);
 	}
 }
 

@@ -105,7 +107,7 @@ static void toggle_breakpoint(int n, int type, int len,
 		     offsetof(struct user, u_debugreg[7]), dr7);
 	if (ret) {
 		perror("Can't set dr7");
-		exit(-1);
+		exit(EXIT_FAIL);
 	}
 }
 

@@ -275,7 +277,7 @@ static void check_success(const char *msg)
 			msg2 = "Ok";
 		if (ptrace(PTRACE_POKEDATA, child_pid, &trapped, 1)) {
 			perror("Can't poke\n");
-			exit(-1);
+			exit(EXIT_FAIL);
 		}
 	}
 

@@ -390,5 +392,5 @@ int main(int argc, char **argv)
 
 	wait(NULL);
 
-	return 0;
+	return EXIT_PASS;
 }

-- 
1.9.1

Add a new header file that defines exit codes for individual
tests to use to communicate test results. These defines are
intended to provide a common and uniform way for selftests
to report results. pass/fail/xfail/xpass/skip/unsupported
are defined.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/kselftest.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tools/testing/selftests/kselftest.h

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
new file mode 100644
index 0000000..1b1c9cb
--- /dev/null
+++ b/tools/testing/selftests/kselftest.h

@@ -0,0 +1,20 @@
+/*
+ * kselftest.h - kselftest framework return codes to include from
+ *		 selftests.
+ *
+ * Copyright (c) 2014 Shuah Khan <shuahkh@osg.samsung.com>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef __KSELFTEST_H
+#define __KSELFTEST_H
+
+#define	EXIT_PASS		0
+#define	EXIT_FAIL		1
+#define	EXIT_XFAIL		2
+#define	EXIT_XPASS		3
+#define	EXIT_SKIP		4
+#define	EXIT_UNSUPPORTED	EXIT_SKIP
+
+#endif /* __KSELFTEST_H */

-- 
1.9.1

On Mon, 2014-09-15 at 16:33 -0600, Shuah Khan wrote:

Looks to me like a potential name clashes here.

What's the difference between XFAIL/XPASS and regular FAIL/PASS (I don't
see the former used in patchset either, only PASS/FAIL)? What's the
purpose of EXIT_SKIP? I think overall these should be commented.

Also, in the bigger picture, I'm guessing you have a reason for not
recycling errno and inventing your own exit codes... How do you plan on
using these? In addition I'm seeing things like:

-       exit(EXIT_FAILURE);
+       exit(EXIT_FAIL);

which isn't a very good idea in general.

On 09/16/2014 10:04 AM, Davidlohr Bueso wrote:

On Mon, 2014-09-15 at 16:33 -0600, Shuah Khan wrote:

quoted

Add a new header file that defines exit codes for individual
tests to use to communicate test results. These defines are
intended to provide a common and uniform way for selftests
to report results. pass/fail/xfail/xpass/skip/unsupported
are defined.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/kselftest.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tools/testing/selftests/kselftest.h

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
new file mode 100644
index 0000000..1b1c9cb
--- /dev/null
+++ b/tools/testing/selftests/kselftest.h

@@ -0,0 +1,20 @@
+/*
+ * kselftest.h - kselftest framework return codes to include from
+ *		 selftests.
+ *
+ * Copyright (c) 2014 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef __KSELFTEST_H
+#define __KSELFTEST_H
+
+#define	EXIT_PASS		0
+#define	EXIT_FAIL		1
+#define	EXIT_XFAIL		2
+#define	EXIT_XPASS		3
+#define	EXIT_SKIP		4
+#define	EXIT_UNSUPPORTED	EXIT_SKIP

Looks to me like a potential name clashes here.

What's the difference between XFAIL/XPASS and regular FAIL/PASS (I don't
see the former used in patchset either, only PASS/FAIL)? What's the
purpose of EXIT_SKIP? I think overall these should be commented.

Yeah Comments would have been nice. :) I will add them.

Also, in the bigger picture, I'm guessing you have a reason for not
recycling errno and inventing your own exit codes... How do you plan on
using these? In addition I'm seeing things like:

-       exit(EXIT_FAILURE);
+       exit(EXIT_FAIL);

which isn't a very good idea in general.

EXIT_FAILURE happens to have the same value as EXIT_FAIL. That said,
I do understand what you are saying. EXIT_FAILURE and EXIT_SUCCESS
are defined in stdlib.h - I would have liked to simply use them,
however that won't meet the needs. More on this below.

At the moment there is no clear way to tell why a test failed. Some
tests exit with -1, some exit 1, and some with errno. One of the
requests/requirements that was discussed at the kernel summit kselftest
session was to enhance tests to report why an individual test failed.
Returning and/or exiting with -1, 1 and errno doesn't tell the user
anything other than that the test failed. Even without this request, it
will helps us developers if we have a uniform reporting in place for
all tests to use.

We have two kinds of users for these tests.

1. Developers that simply want to regression test their individual areas
   These are the users that don't care about the categories of failures.
2. Users that want to run them from their user-space test suites. These
   users care to know why a test failed, not just that it failed.

errno is useful in pin-pointing the failure for a developer, however it
is not very useful for somebody that is running sanity checks. We need
both, hence I changed some of the tests in this series to print errno.
Several tests print errno in their error legs and there a few places
that don't.

In either case, it would be good to report if a test failed because
a modules it needs isn't configured or it just failed.

There is also a need to report the following cases in addition a simple
pass/fail:

pass - test passed
fail - it failed
xfail - a test that expected to fail failed as expected (this is really
a pass case)
xpass - a test that is expected to fail passed.
xskip or xunsupported - test couldn't run because of unmet dependencies.

These types of decisions on why test failed, can only be made in the
individual tests.

I picked the POSIX conforming test codes that are used by various user
space test suites. POSIX right, I can't go wrong :)

I also want to avoid adding some test framework in kernel tree, hence
I simply defined these in a header file. Another goal is to not make it
hard for developers write these tests and think too much about the
reporting. We need some way to report these and hence the need for a
common defines so tests can simply use them.

I am trying to balance the needs of the two types of users and also
do minimal changes to existing test with a light weight framework.
Hope this helps explain this patch series better.

I am open to suggestions as always.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

On Tue, Sep 16, 2014 at 10:31 AM, Shuah Khan [off-list ref] wrote:

On 09/16/2014 10:04 AM, Davidlohr Bueso wrote:

quoted

On Mon, 2014-09-15 at 16:33 -0600, Shuah Khan wrote:

quoted

Add a new header file that defines exit codes for individual
tests to use to communicate test results. These defines are
intended to provide a common and uniform way for selftests
to report results. pass/fail/xfail/xpass/skip/unsupported
are defined.

Signed-off-by: Shuah Khan <redacted>
---
 tools/testing/selftests/kselftest.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tools/testing/selftests/kselftest.h

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
new file mode 100644
index 0000000..1b1c9cb
--- /dev/null
+++ b/tools/testing/selftests/kselftest.h

@@ -0,0 +1,20 @@
+/*
+ * kselftest.h - kselftest framework return codes to include from
+ *           selftests.
+ *
+ * Copyright (c) 2014 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This file is released under the GPLv2.
+ */
+#ifndef __KSELFTEST_H
+#define __KSELFTEST_H
+
+#define     EXIT_PASS               0
+#define     EXIT_FAIL               1
+#define     EXIT_XFAIL              2
+#define     EXIT_XPASS              3
+#define     EXIT_SKIP               4
+#define     EXIT_UNSUPPORTED        EXIT_SKIP

Looks to me like a potential name clashes here.

What's the difference between XFAIL/XPASS and regular FAIL/PASS (I don't
see the former used in patchset either, only PASS/FAIL)? What's the
purpose of EXIT_SKIP? I think overall these should be commented.

Yeah Comments would have been nice. :) I will add them.

quoted

Also, in the bigger picture, I'm guessing you have a reason for not
recycling errno and inventing your own exit codes... How do you plan on
using these? In addition I'm seeing things like:

-       exit(EXIT_FAILURE);
+       exit(EXIT_FAIL);

which isn't a very good idea in general.

EXIT_FAILURE happens to have the same value as EXIT_FAIL. That said,
I do understand what you are saying. EXIT_FAILURE and EXIT_SUCCESS
are defined in stdlib.h - I would have liked to simply use them,
however that won't meet the needs. More on this below.

At the moment there is no clear way to tell why a test failed. Some
tests exit with -1, some exit 1, and some with errno. One of the
requests/requirements that was discussed at the kernel summit kselftest
session was to enhance tests to report why an individual test failed.
Returning and/or exiting with -1, 1 and errno doesn't tell the user
anything other than that the test failed. Even without this request, it
will helps us developers if we have a uniform reporting in place for
all tests to use.

We have two kinds of users for these tests.

1. Developers that simply want to regression test their individual areas
   These are the users that don't care about the categories of failures.
2. Users that want to run them from their user-space test suites. These
   users care to know why a test failed, not just that it failed.

errno is useful in pin-pointing the failure for a developer, however it
is not very useful for somebody that is running sanity checks. We need
both, hence I changed some of the tests in this series to print errno.
Several tests print errno in their error legs and there a few places
that don't.

In either case, it would be good to report if a test failed because
a modules it needs isn't configured or it just failed.

There is also a need to report the following cases in addition a simple
pass/fail:

pass - test passed
fail - it failed
xfail - a test that expected to fail failed as expected (this is really
a pass case)
xpass - a test that is expected to fail passed.
xskip or xunsupported - test couldn't run because of unmet dependencies.

These types of decisions on why test failed, can only be made in the
individual tests.

I picked the POSIX conforming test codes that are used by various user
space test suites. POSIX right, I can't go wrong :)

I also want to avoid adding some test framework in kernel tree, hence
I simply defined these in a header file. Another goal is to not make it
hard for developers write these tests and think too much about the
reporting. We need some way to report these and hence the need for a
common defines so tests can simply use them.

I think we will want a framework in the tree, but it can be very
minimal.  But I also think that using exit(2) for this is wrong.  Why
not:

enum ktest_result {
  KTEST_PASS,
  ...,
};

void ktest_exit(enum ktest_result result);

With the possibility of further extensions for more than one test (and
associated result) per execution of the test binary.

--Andy

I am trying to balance the needs of the two types of users and also
do minimal changes to existing test with a light weight framework.
Hope this helps explain this patch series better.

I am open to suggestions as always.

thanks,
-- Shuah


--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Andy Lutomirski
AMA Capital Management, LLC

On 09/16/2014 11:40 AM, Andy Lutomirski wrote:

On Tue, Sep 16, 2014 at 10:31 AM, Shuah Khan [off-list ref] wrote:

I think we will want a framework in the tree, but it can be very
minimal.  But I also think that using exit(2) for this is wrong.  Why
not:

enum ktest_result {
  KTEST_PASS,
  ...,
};

void ktest_exit(enum ktest_result result);

With the possibility of further extensions for more than one test (and
associated result) per execution of the test binary.

Nothing wrong with the approach. It is one of the options I considered
and decided against it primarily because tests need more changes than
the ones needed for return codes. However, maybe it is worth while to
do so. I will play with this and see how extensive the changes are.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978