diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index 6d6e549f5c..7a1977d804 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -780,3 +780,13 @@ SPDX-License-Identifier: LGPL-2.1-or-later good idea where it might end up running inside of libsystemd.so or similar. Hence, use TLS (i.e. `thread_local`) where appropriate, and maybe the occasional `pthread_once()`. + +## Tests + +- Use the assertion macros from `tests.h` (`ASSERT_GE()`, `ASSERT_OK()`, ...) to + make sure a descriptive error is logged when an assertion fails. If no assertion + macro exists for your specific use case, please add a new assertion macro in a + separate commit. + +- When modifying existing tests, please convert the test to use the new assertion + macros from `tests.h` if it is not already using those. diff --git a/src/shared/tests.h b/src/shared/tests.h index 1572ca7102..784748545c 100644 --- a/src/shared/tests.h +++ b/src/shared/tests.h @@ -210,12 +210,22 @@ static inline int run_test_table(void) { ({ \ typeof(expr) _result = (expr); \ if (_result < 0) { \ - log_error_errno(_result, "%s:%i: Assertion failed: %s: %m", \ + log_error_errno(_result, "%s:%i: Assertion failed: expected \"%s\" to succeed but got the following error: %m", \ PROJECT_FILE, __LINE__, #expr); \ abort(); \ } \ }) +#define ASSERT_OK_ERRNO(expr) \ + ({ \ + typeof(expr) _result = (expr); \ + if (_result < 0) { \ + log_error_errno(errno, "%s:%i: Assertion failed: expected \"%s\" to succeed but got the following error: %m", \ + PROJECT_FILE, __LINE__, #expr); \ + abort(); \ + } \ + }) + #define ASSERT_TRUE(expr) \ ({ \ if (!(expr)) { \ @@ -236,9 +246,10 @@ static inline int run_test_table(void) { #define ASSERT_NULL(expr) \ ({ \ - if ((expr) != NULL) { \ - log_error("%s:%i: Assertion failed: expected \"%s\" to be NULL", \ - PROJECT_FILE, __LINE__, #expr); \ + typeof(expr) _result = (expr); \ + if (_result != NULL) { \ + log_error("%s:%i: Assertion failed: expected \"%s\" to be NULL, but \"%p\" != NULL", \ + PROJECT_FILE, __LINE__, #expr, _result); \ abort(); \ } \ }) diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c index 3cb106a93f..0cc9afcf34 100644 --- a/src/test/test-acl-util.c +++ b/src/test/test-acl-util.c @@ -95,7 +95,7 @@ TEST_RET(fd_acl_make_read_only) { /* make it more exciting */ (void) fd_add_uid_acl_permission(fd, 1, ACL_READ|ACL_WRITE|ACL_EXECUTE); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se(FLAGS_SET(st.st_mode, 0200)); cmd = strjoina("getfacl -p ", fn); @@ -107,7 +107,7 @@ TEST_RET(fd_acl_make_read_only) { log_info("read-only"); assert_se(fd_acl_make_read_only(fd)); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se((st.st_mode & 0222) == 0000); cmd = strjoina("getfacl -p ", fn); @@ -119,7 +119,7 @@ TEST_RET(fd_acl_make_read_only) { log_info("writable"); assert_se(fd_acl_make_writable(fd)); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se((st.st_mode & 0222) == 0200); cmd = strjoina("getfacl -p ", fn); @@ -131,7 +131,7 @@ TEST_RET(fd_acl_make_read_only) { log_info("read-only"); assert_se(fd_acl_make_read_only(fd)); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se((st.st_mode & 0222) == 0000); cmd = strjoina("getfacl -p ", fn); diff --git a/src/test/test-copy.c b/src/test/test-copy.c index ea5d67ff48..46519fdc01 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -444,7 +444,7 @@ TEST_RET(copy_holes) { return log_tests_skipped("Filesystem doesn't support hole punching"); assert_se(r >= 0); - assert_se(fstat(fd, &stat) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &stat)); blksz = stat.st_blksize; buf = alloca_safe(blksz); memset(buf, 1, blksz); @@ -469,7 +469,7 @@ TEST_RET(copy_holes) { assert_se(lseek(fd_copy, 2 * blksz, SEEK_DATA) < 0 && errno == ENXIO); /* Test that the copied file has the correct size. */ - assert_se(fstat(fd_copy, &stat) >= 0); + ASSERT_OK_ERRNO(fstat(fd_copy, &stat)); assert_se(stat.st_size == 3 * blksz); close(fd); @@ -490,7 +490,7 @@ TEST_RET(copy_holes_with_gaps) { assert_se((fd = openat(tfd, "src", O_CREAT | O_RDWR, 0600)) >= 0); assert_se((fd_copy = openat(tfd, "dst", O_CREAT | O_WRONLY, 0600)) >= 0); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); blksz = st.st_blksize; buf = alloca_safe(blksz); memset(buf, 1, blksz); @@ -519,7 +519,7 @@ TEST_RET(copy_holes_with_gaps) { /* Copy to the start of the second hole */ assert_se(copy_bytes(fd, fd_copy, 3 * blksz, COPY_HOLES) >= 0); - assert_se(fstat(fd_copy, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd_copy, &st)); assert_se(st.st_size == 3 * blksz); /* Copy to the middle of the second hole */ @@ -527,7 +527,7 @@ TEST_RET(copy_holes_with_gaps) { assert_se(lseek(fd_copy, 0, SEEK_SET) >= 0); assert_se(ftruncate(fd_copy, 0) >= 0); assert_se(copy_bytes(fd, fd_copy, 4 * blksz, COPY_HOLES) >= 0); - assert_se(fstat(fd_copy, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd_copy, &st)); assert_se(st.st_size == 4 * blksz); /* Copy to the end of the second hole */ @@ -535,7 +535,7 @@ TEST_RET(copy_holes_with_gaps) { assert_se(lseek(fd_copy, 0, SEEK_SET) >= 0); assert_se(ftruncate(fd_copy, 0) >= 0); assert_se(copy_bytes(fd, fd_copy, 5 * blksz, COPY_HOLES) >= 0); - assert_se(fstat(fd_copy, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd_copy, &st)); assert_se(st.st_size == 5 * blksz); /* Copy everything */ @@ -543,7 +543,7 @@ TEST_RET(copy_holes_with_gaps) { assert_se(lseek(fd_copy, 0, SEEK_SET) >= 0); assert_se(ftruncate(fd_copy, 0) >= 0); assert_se(copy_bytes(fd, fd_copy, UINT64_MAX, COPY_HOLES) >= 0); - assert_se(fstat(fd_copy, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd_copy, &st)); assert_se(st.st_size == 6 * blksz); return 0; diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index f05cbbee6e..b10dfa8ed2 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -413,7 +413,7 @@ TEST(fd_reopen) { fd1 = open("/proc", O_DIRECTORY|O_PATH|O_CLOEXEC); assert_se(fd1 >= 0); - assert_se(fstat(fd1, &st1) >= 0); + ASSERT_OK_ERRNO(fstat(fd1, &st1)); assert_se(S_ISDIR(st1.st_mode)); fl = fcntl(fd1, F_GETFL); @@ -428,7 +428,7 @@ TEST(fd_reopen) { fd2 = fd_reopen(fd1, O_RDONLY|O_DIRECTORY|O_CLOEXEC); /* drop the O_PATH */ assert_se(fd2 >= 0); - assert_se(fstat(fd2, &st2) >= 0); + ASSERT_OK_ERRNO(fstat(fd2, &st2)); assert_se(S_ISDIR(st2.st_mode)); assert_se(stat_inode_same(&st1, &st2)); @@ -442,7 +442,7 @@ TEST(fd_reopen) { fd1 = fd_reopen(fd2, O_DIRECTORY|O_PATH|O_CLOEXEC); /* reacquire the O_PATH */ assert_se(fd1 >= 0); - assert_se(fstat(fd1, &st1) >= 0); + ASSERT_OK_ERRNO(fstat(fd1, &st1)); assert_se(S_ISDIR(st1.st_mode)); assert_se(stat_inode_same(&st1, &st2)); @@ -457,7 +457,7 @@ TEST(fd_reopen) { fd1 = open("/proc/version", O_PATH|O_CLOEXEC); assert_se(fd1 >= 0); - assert_se(fstat(fd1, &st1) >= 0); + ASSERT_OK_ERRNO(fstat(fd1, &st1)); assert_se(S_ISREG(st1.st_mode)); fl = fcntl(fd1, F_GETFL); @@ -469,7 +469,7 @@ TEST(fd_reopen) { fd2 = fd_reopen(fd1, O_RDONLY|O_CLOEXEC); /* drop the O_PATH */ assert_se(fd2 >= 0); - assert_se(fstat(fd2, &st2) >= 0); + ASSERT_OK_ERRNO(fstat(fd2, &st2)); assert_se(S_ISREG(st2.st_mode)); assert_se(stat_inode_same(&st1, &st2)); @@ -484,7 +484,7 @@ TEST(fd_reopen) { fd1 = fd_reopen(fd2, O_PATH|O_CLOEXEC); /* reacquire the O_PATH */ assert_se(fd1 >= 0); - assert_se(fstat(fd1, &st1) >= 0); + ASSERT_OK_ERRNO(fstat(fd1, &st1)); assert_se(S_ISREG(st1.st_mode)); assert_se(stat_inode_same(&st1, &st2)); @@ -501,12 +501,12 @@ TEST(fd_reopen) { /* Validate what happens if we reopen a symlink */ fd1 = open("/proc/self", O_PATH|O_CLOEXEC|O_NOFOLLOW); assert_se(fd1 >= 0); - assert_se(fstat(fd1, &st1) >= 0); + ASSERT_OK_ERRNO(fstat(fd1, &st1)); assert_se(S_ISLNK(st1.st_mode)); fd2 = fd_reopen(fd1, O_PATH|O_CLOEXEC); assert_se(fd2 >= 0); - assert_se(fstat(fd2, &st2) >= 0); + ASSERT_OK_ERRNO(fstat(fd2, &st2)); assert_se(S_ISLNK(st2.st_mode)); assert_se(stat_inode_same(&st1, &st2)); fd2 = safe_close(fd2); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index b27c79fdc9..d1f555fafe 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -275,14 +275,14 @@ TEST(unlinkat_deallocate) { assert_se(write(fd, "hallo\n", 6) == 6); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se(st.st_size == 6); assert_se(st.st_blocks > 0); assert_se(st.st_nlink == 1); assert_se(unlinkat_deallocate(AT_FDCWD, p, UNLINK_ERASE) >= 0); - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se(IN_SET(st.st_size, 0, 6)); /* depending on whether hole punching worked the size will be 6 (it worked) or 0 (we had to resort to truncation) */ assert_se(st.st_blocks == 0); @@ -557,13 +557,13 @@ TEST(open_mkdir_at) { fd = open_mkdir_at(AT_FDCWD, "/", O_CLOEXEC, 0); assert_se(fd >= 0); assert_se(stat("/", &sta) >= 0); - assert_se(fstat(fd, &stb) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &stb)); assert_se(stat_inode_same(&sta, &stb)); fd = safe_close(fd); fd = open_mkdir_at(AT_FDCWD, ".", O_CLOEXEC, 0); assert_se(stat(".", &sta) >= 0); - assert_se(fstat(fd, &stb) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &stb)); assert_se(stat_inode_same(&sta, &stb)); fd = safe_close(fd); diff --git a/src/test/test-macro.c b/src/test/test-macro.c index 6a6e7a0962..05e15c8bdc 100644 --- a/src/test/test-macro.c +++ b/src/test/test-macro.c @@ -1114,6 +1114,12 @@ TEST(ASSERT) { ASSERT_SIGNAL(ASSERT_OK(-1), SIGABRT); ASSERT_SIGNAL(ASSERT_OK(-ENOANO), SIGABRT); + ASSERT_OK_ERRNO(0 >= 0); + ASSERT_OK_ERRNO(255 >= 0); + ASSERT_OK_ERRNO(printf("Hello world\n")); + ASSERT_SIGNAL(ASSERT_OK_ERRNO(-1), SIGABRT); + ASSERT_SIGNAL(ASSERT_OK_ERRNO(-ENOANO), SIGABRT); + ASSERT_TRUE(true); ASSERT_TRUE(255); ASSERT_TRUE(getpid()); diff --git a/src/test/test-stat-util.c b/src/test/test-stat-util.c index df37dcb528..a69f6f06e0 100644 --- a/src/test/test-stat-util.c +++ b/src/test/test-stat-util.c @@ -205,7 +205,7 @@ TEST(anonymous_inode) { /* Verify that we handle anonymous inodes correctly, i.e. those which have no file type */ struct stat st; - assert_se(fstat(fd, &st) >= 0); + ASSERT_OK_ERRNO(fstat(fd, &st)); assert_se((st.st_mode & S_IFMT) == 0); assert_se(!inode_type_to_string(st.st_mode)); diff --git a/src/test/test-terminal-util.c b/src/test/test-terminal-util.c index 0b46cad982..dbd7654991 100644 --- a/src/test/test-terminal-util.c +++ b/src/test/test-terminal-util.c @@ -155,7 +155,7 @@ TEST(get_ctty) { } /* In almost all cases STDIN will match our controlling TTY. Let's verify that and then compare paths */ - assert_se(fstat(STDIN_FILENO, &st) >= 0); + ASSERT_OK_ERRNO(fstat(STDIN_FILENO, &st)); if (S_ISCHR(st.st_mode) && st.st_rdev == devnr) { _cleanup_free_ char *stdin_name = NULL;