[PATCH testsuite v2 1/2] tests/file_contexts: refactor existing tests

["Thiébaud Weksteen" <tweek@google.com>]

Change the logging format when reporting an error to better capture the
location and context of the error.

Split tests into functions to better isolate the tested behaviour.

Signed-off-by: Thiébaud Weksteen <tweek@google.com>
---
 tests/file_contexts/internal.c       | 16 ++---
 tests/file_contexts/internal.h       | 20 +++++-
 tests/file_contexts/test_lookup.c    | 60 ++++++++++++------
 tests/file_contexts/test_open.c      | 94 +++++++++++++++++-----------
 tests/file_contexts/test_open_base.c | 46 ++++++++++----
 tests/file_contexts/test_validate.c  | 37 ++++++-----
 6 files changed, 178 insertions(+), 95 deletions(-)

diff --git a/tests/file_contexts/internal.c b/tests/file_contexts/internal.c
index 22b5790..ba45dd6 100644
--- a/tests/file_contexts/internal.c
+++ b/tests/file_contexts/internal.c
@@ -10,8 +10,8 @@
 
 #include "internal.h"
 
-void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
-			 size_t n)
+void assertContextsMatch(struct selabel_handle *hnd, const char *log_prefix,
+			 struct test_t *tests, size_t n)
 {
 	for (int i = 0; i < n; i++) {
 		char *context = NULL;
@@ -19,20 +19,22 @@ void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
 
 		if (selabel_lookup(hnd, &context, test.path, S_IFREG)) {
 			if (test.context) {
-				perror("file_contexts:selabel_lookup");
-				fprintf(stderr, "Lookup for %s failed\n", test.path);
+				log_errno("Lookup for %s from %s failed",
+					  test.path, log_prefix);
 				exit(2);
 			}
 			// Expected failure. Continue to the next test.
 			continue;
 		} else if (!test.context) {
-			fprintf(stderr, "Lookup for %s was supposed to failed\n", test.path);
+			log_err("Lookup for %s from %s was supposed to failed but got %s",
+				test.path, log_prefix, context);
 			exit(2);
 		}
 
 		if (strcmp(context, tests[i].context)) {
-			fprintf(stderr, "Lookup for %s returned %s, expected %s\n",
-				tests[i].path, context, tests[i].context);
+			log_err("Lookup for %s from %s returned %s, expected %s",
+				tests[i].path, log_prefix, context,
+				tests[i].context);
 			exit(2);
 		}
 
diff --git a/tests/file_contexts/internal.h b/tests/file_contexts/internal.h
index a07548f..c51bbc6 100644
--- a/tests/file_contexts/internal.h
+++ b/tests/file_contexts/internal.h
@@ -1,8 +1,21 @@
+#include <errno.h>
+#include <string.h>
+
 #include <selinux/label.h>
 #include <selinux/selinux.h>
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
+/* Log an error message with the file name, function name and line */
+#define log_err(fmt, ...)                                                    \
+	fprintf(stderr, "%s:%s:%d: " fmt "\n", __FILE__, __func__, __LINE__, \
+		##__VA_ARGS__)
+
+/* Log an error message as well as errno */
+#define log_errno(fmt, ...)                                            \
+	fprintf(stderr, "%s:%s:%d " fmt " (%s)\n", __FILE__, __func__, \
+		__LINE__, ##__VA_ARGS__, strerror(errno))
+
 /* Structure to capture a test. The |path| will be resolved and expected to
  * match the |context|. If |context| is NULL, the lookup is expected to fail. */
 struct test_t {
@@ -11,6 +24,7 @@ struct test_t {
 };
 
 /* Assert that all paths described in |tests| resolve appropriately. |hnd| must
- * be opened. |n| is the number of tests in |tests|. */
-void assertContextsMatch(struct selabel_handle *hnd, struct test_t *tests,
-			 size_t n);
+ * be opened. |n| is the number of tests in |tests|. |log_prefix| is added to
+ * any log message */
+void assertContextsMatch(struct selabel_handle *hnd, const char *log_prefix,
+			 struct test_t *tests, size_t n);
diff --git a/tests/file_contexts/test_lookup.c b/tests/file_contexts/test_lookup.c
index 9a54d1d..5957db4 100644
--- a/tests/file_contexts/test_lookup.c
+++ b/tests/file_contexts/test_lookup.c
@@ -9,38 +9,58 @@
 
 #include "internal.h"
 
-int main(int argc, char **argv)
+void test_lookup(const char *basedir)
 {
-	struct selabel_handle *hnd;
-
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
-
 	char *path;
-	asprintf(&path, "%s/f2.fc", argv[1]);
-	struct selinux_opt f2_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	asprintf(&path, "%s/f2.fc", basedir);
+	struct selinux_opt f2_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
-	hnd = selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
-
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f2_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	struct test_t tests[] = {
-		{ .path = "/base", .context = "system_u:object_r:test_base_t:s0" },
-		{ .path = "/base/unkown", .context = "system_u:object_r:test_base_wildcard_t:s0" },
-		{ .path = "/base/sub", .context = "system_u:object_r:test_base_sub_t:s0" },
-		{ .path = "/base/file.list", .context = "system_u:object_r:test_file_list_t:s0" },
-		{ .path = "/base/file_list", .context = "system_u:object_r:test_base_wildcard_t:s0" },
+		{
+			.path = "/base",
+			.context = "system_u:object_r:test_base_t:s0"
+		},
+		{
+			.path = "/base/unkown",
+			.context = "system_u:object_r:test_base_wildcard_t:s0"
+		},
+		{
+			.path = "/base/sub",
+			.context = "system_u:object_r:test_base_sub_t:s0"
+		},
+		{
+			.path = "/base/file.list",
+			.context = "system_u:object_r:test_file_list_t:s0"
+		},
+		{
+			.path = "/base/file_list",
+			.context = "system_u:object_r:test_base_wildcard_t:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
+
+	test_lookup(argv[1]);
 
 	return 0;
 }
diff --git a/tests/file_contexts/test_open.c b/tests/file_contexts/test_open.c
index eae5691..4dd3300 100644
--- a/tests/file_contexts/test_open.c
+++ b/tests/file_contexts/test_open.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -8,75 +7,96 @@
 #include <selinux/label.h>
 #include <selinux/selinux.h>
 
-int main(int argc, char **argv)
+#include "internal.h"
+
+void test_no_options(void)
 {
-	bool has_default_path = false;
-	struct selabel_handle *hnd;
 	struct stat default_stat;
+	if (stat(selinux_file_context_path(), &default_stat))
+		return;
 
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
+	/* Empty options */
+	struct selabel_handle *hnd = selabel_open(
+					     SELABEL_CTX_FILE, /* options= */ NULL, /* nopt= */ 0);
 
-	const char *default_path = selinux_file_context_path();
-	if (!stat(default_path, &default_stat)) {
-		has_default_path = true;
+	if (!hnd) {
+		log_errno("Unable to open default content file");
+		exit(2);
 	}
+	selabel_close(hnd);
+}
 
-	if (has_default_path) {
-		/* Empty options */
-		hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ NULL, /* nopt= */ 0);
+void test_null_path(void)
+{
+	struct stat default_stat;
+	if (stat(selinux_file_context_path(), &default_stat))
+		return;
 
-		if (!hnd) {
-			perror("file_context:no_options");
-			exit(2);
+	/* Default options */
+	struct selinux_opt null_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = NULL
 		}
-		selabel_close(hnd);
-
-		/* Default options */
-		struct selinux_opt null_opts[] = {
-			{ .type = SELABEL_OPT_PATH, .value = NULL }
-		};
+	};
 
-		hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ null_opts, /* nopt= */ 1);
+	struct selabel_handle *hnd = selabel_open(SELABEL_CTX_FILE, null_opts,
+						  ARRAY_SIZE(null_opts));
 
-		if (!hnd) {
-			perror("file_context:default_options");
-			exit(2);
-		}
-		selabel_close(hnd);
+	if (!hnd) {
+		log_errno("Unable to open default content file");
+		exit(2);
 	}
+	selabel_close(hnd);
+}
 
+void test_valid_path(const char *basedir)
+{
 	/* f1.fc file */
 	char *path;
-	asprintf(&path, "%s/f1.fc", argv[1]);
-	struct selinux_opt f1_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	asprintf(&path, "%s/f1.fc", basedir);
+	struct selinux_opt f1_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
-	hnd = selabel_open(SELABEL_CTX_FILE, /* options= */ f1_opts, /* nopt= */ 1);
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f1_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	char *context = NULL;
 	if (selabel_lookup(hnd, &context, "/", S_IFREG)) {
-		perror("file_contexts:f1_lookup");
+		log_errno("Unable to lookup \"/\"");
 		exit(2);
 	}
 
-	if (strcmp(context, "system_u:object_r:rootfs:s0")) {
-		perror("file_contexts:f1_strcmp");
+	const char *expected = "system_u:object_r:rootfs:s0";
+	if (strcmp(context, expected)) {
+		log_err("Incorrect context returned, expected %s got %s",
+			expected, context);
 		exit(2);
 	}
 
 	free(context);
 
 	selabel_close(hnd);
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
+
+	test_no_options();
+	test_null_path();
+	test_valid_path(argv[1]);
 
 	return 0;
 }
diff --git a/tests/file_contexts/test_open_base.c b/tests/file_contexts/test_open_base.c
index 33b0cac..3e4b4c6 100644
--- a/tests/file_contexts/test_open_base.c
+++ b/tests/file_contexts/test_open_base.c
@@ -22,26 +22,40 @@ void test_default_options(const char *basedir)
 
 	char *path;
 	asprintf(&path, "%s/f3.fc", basedir);
-	struct selinux_opt f3_opts[] = {
-		{ .type = SELABEL_OPT_PATH, .value = path }
+	struct selinux_opt f3_opts[] = { {
+			.type = SELABEL_OPT_PATH,
+			.value = path
+		}
 	};
 
 	hnd = selabel_open(SELABEL_CTX_FILE, f3_opts, ARRAY_SIZE(f3_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f3_default_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
 	struct test_t tests[] = {
 		{ .path = "/", .context = "system_u:object_r:rootfs:s0" },
-		{ .path = "/local", .context = "system_u:object_r:test_local:s0" },
-		{ .path = "/homedirs", .context = "system_u:object_r:test_homedirs:s0" },
-		{ .path = "/sub", .context = "system_u:object_r:test_subbed:s0" },
-		{ .path = "/sub_dist", .context = "system_u:object_r:test_subbed:s0" },
+		{
+			.path = "/local",
+			.context = "system_u:object_r:test_local:s0"
+		},
+		{
+			.path = "/homedirs",
+			.context = "system_u:object_r:test_homedirs:s0"
+		},
+		{
+			.path = "/sub",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
+		{
+			.path = "/sub_dist",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
 
 	selabel_close(hnd);
 }
@@ -62,7 +76,7 @@ void test_base_only_option(const char *basedir)
 	free(path);
 
 	if (!hnd) {
-		perror("file_context:f3_base_only_options");
+		log_errno("Unable to open file backend");
 		exit(2);
 	}
 
@@ -70,10 +84,16 @@ void test_base_only_option(const char *basedir)
 		{ .path = "/", .context = "system_u:object_r:rootfs:s0" },
 		{ .path = "/local", .context = NULL },
 		{ .path = "/homedirs", .context = NULL },
-		{ .path = "/sub", .context = "system_u:object_r:test_subbed:s0" },
-		{ .path = "/sub_dist", .context = "system_u:object_r:test_subbed:s0" },
+		{
+			.path = "/sub",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
+		{
+			.path = "/sub_dist",
+			.context = "system_u:object_r:test_subbed:s0"
+		},
 	};
-	assertContextsMatch(hnd, tests, ARRAY_SIZE(tests));
+	assertContextsMatch(hnd, __func__, tests, ARRAY_SIZE(tests));
 
 	selabel_close(hnd);
 }
@@ -81,7 +101,7 @@ void test_base_only_option(const char *basedir)
 int main(int argc, char **argv)
 {
 	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
+		log_err("basedir not provided");
 		exit(1);
 	}
 
diff --git a/tests/file_contexts/test_validate.c b/tests/file_contexts/test_validate.c
index 8abefed..aa71c22 100644
--- a/tests/file_contexts/test_validate.c
+++ b/tests/file_contexts/test_validate.c
@@ -1,6 +1,5 @@
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -10,47 +9,55 @@
 
 #include "internal.h"
 
-int main(int argc, char **argv)
+void test_validate_unknown_fails(const char *basedir)
 {
 	char *path;
-	struct selabel_handle *hnd;
-
-	if (argc != 2) {
-		fprintf(stderr, "basedir not provided\n");
-		exit(1);
-	}
-
-	asprintf(&path, "%s/f1.fc", argv[1]);
+	asprintf(&path, "%s/f1.fc", basedir);
 	struct selinux_opt f1_opts[] = {
 		{ .type = SELABEL_OPT_PATH, .value = path },
 		{ .type = SELABEL_OPT_VALIDATE, .value = "1" }
 	};
 
 	/* f1 types are not defined in the test policy. */
-	hnd = selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f1_opts, ARRAY_SIZE(f1_opts));
 	free(path);
 
 	if (hnd) {
-		fprintf(stderr, "The validation of f1 should have failed.\n");
+		log_err("The validation of f1 should have failed");
+		selabel_close(hnd);
 		exit(2);
 	}
+}
 
-	asprintf(&path, "%s/f2.fc", argv[1]);
+void test_validate_known_succeeds(const char *basedir)
+{
+	char *path;
+	asprintf(&path, "%s/f2.fc", basedir);
 	struct selinux_opt f2_opts[] = {
 		{ .type = SELABEL_OPT_PATH, .value = path },
 		{ .type = SELABEL_OPT_VALIDATE, .value = "1" }
 	};
 
 	/* f2 types are defined in the test policy. */
-	hnd = selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
+	struct selabel_handle *hnd =
+		selabel_open(SELABEL_CTX_FILE, f2_opts, ARRAY_SIZE(f2_opts));
 	free(path);
 
 	if (!hnd) {
-		perror("Unable to read valid file_contexts");
+		log_errno("Unable to read valid file_contexts");
 		exit(2);
 	}
 
 	selabel_close(hnd);
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 2) {
+		log_err("basedir not provided");
+		exit(1);
+	}
 
 	return 0;
 }
-- 
2.55.0.229.g6434b31f56-goog