test: Remove TEST_CHECK macro

The TEST_CHECK macro returns a value (from the calling function) on
either success or failure. This differs from all other TEST_* macros in
test_util.h, which only return on failure. Returning on failure is
appropriate and allows short circuiting a test on the first failure but
returning on success results in short circuiting after the first
successful check and bypassing subsequent checks. This behavior
is somewhat confusing and easy to miss during review.

BRANCH=none
BUG=b:238120333
TEST=make runhosttests
TEST=./util/compare_build.sh -b all -j 120
      => MATCH
Signed-off-by: Bobby Casey <bobbycasey@google.com>
Change-Id: I3777b427aa5e20a91689f86fc37daadffacf27f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3748830
Reviewed-by: Tom Hughes <tomhughes@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>

8 files changed, 80 insertions, 64 deletions

diff --git a/docs/zephyr/ztest.md b/docs/zephyr/ztest.md
index 6bb121045d..d782631cc3 100644
--- a/docs/zephyr/ztest.md
+++ b/docs/zephyr/ztest.md
@@ -142,7 +142,6 @@ find-and-replace.
 *   `TEST_BITS_CLEARED(a, bits)` to `zassert_true(a & (int)bits == 0, "%u, 0", a
     & (int)bits)`
 *   `TEST_ASSERT_ARRAY_EQ(s, d, n)` to `zassert_mem_equal(s, d, b, NULL)`
-*   `TEST_CHECK(n)` to `zassert_true(n, NULL)`
 *   `TEST_NEAR(a, b, epsilon, fmt)` to `zassert_within(a, b, epsilon, fmt, a)`
     *   Currently, every usage of `TEST_NEAR` involves floating point values
 *   `TEST_ASSERT_ABS_LESS(n, t)` to `zassert_true(abs(n) < t, "%d, %d", n, t)`
diff --git a/include/test_util.h b/include/test_util.h
index 03058fdaaf..66513f6513 100644
--- a/include/test_util.h
+++ b/include/test_util.h
@@ -107,14 +107,6 @@
 			}                                                  \
 	} while (0)
 
-#define TEST_CHECK(n)                            \
-	do {                                     \
-		if (n)                           \
-			return EC_SUCCESS;       \
-		else                             \
-			return EC_ERROR_UNKNOWN; \
-	} while (0)
-
 /* Mutlistep test states */
 enum test_state_t {
 	TEST_STATE_STEP_1 = 0,
diff --git a/test/console_edit.c b/test/console_edit.c
index e949073f16..fb49e9c41c 100644
--- a/test/console_edit.c
+++ b/test/console_edit.c
@@ -98,7 +98,8 @@ static int test_backspace(void)
 	cmd_1_call_cnt = 0;
 	UART_INJECT("testx\b1\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_insert_char(void)
@@ -108,7 +109,8 @@ static int test_insert_char(void)
 	arrow_key(ARROW_LEFT, 2);
 	UART_INJECT("s\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_delete_char(void)
@@ -118,7 +120,8 @@ static int test_delete_char(void)
 	arrow_key(ARROW_LEFT, 1);
 	UART_INJECT("\b\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_insert_delete_char(void)
@@ -130,7 +133,8 @@ static int test_insert_delete_char(void)
 	arrow_key(ARROW_RIGHT, 1);
 	UART_INJECT("s\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_home_end_key(void)
@@ -142,7 +146,8 @@ static int test_home_end_key(void)
 	end_key();
 	UART_INJECT("1\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_ctrl_k(void)
@@ -153,7 +158,8 @@ static int test_ctrl_k(void)
 	ctrl_key('K');
 	UART_INJECT("\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_history_up(void)
@@ -164,7 +170,8 @@ static int test_history_up(void)
 	arrow_key(ARROW_UP, 1);
 	UART_INJECT("\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 2);
+	TEST_ASSERT(cmd_1_call_cnt == 2);
+	return EC_SUCCESS;
 }
 
 static int test_history_up_up(void)
@@ -178,7 +185,8 @@ static int test_history_up_up(void)
 	arrow_key(ARROW_UP, 2);
 	UART_INJECT("\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 2 && cmd_2_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 2 && cmd_2_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_history_up_up_down(void)
@@ -193,7 +201,8 @@ static int test_history_up_up_down(void)
 	arrow_key(ARROW_DOWN, 1);
 	UART_INJECT("\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 2);
+	TEST_ASSERT(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 2);
+	return EC_SUCCESS;
 }
 
 static int test_history_edit(void)
@@ -205,7 +214,8 @@ static int test_history_edit(void)
 	arrow_key(ARROW_UP, 1);
 	UART_INJECT("\b2\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_history_stash(void)
@@ -219,7 +229,8 @@ static int test_history_stash(void)
 	arrow_key(ARROW_DOWN, 1);
 	UART_INJECT("2\n");
 	msleep(30);
-	TEST_CHECK(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 1);
+	TEST_ASSERT(cmd_1_call_cnt == 1 && cmd_2_call_cnt == 1);
+	return EC_SUCCESS;
 }
 
 static int test_history_list(void)
diff --git a/test/flash.c b/test/flash.c
index a5f25fb164..c571c4e05f 100644
--- a/test/flash.c
+++ b/test/flash.c
@@ -351,10 +351,11 @@ static int test_flash_info(void)
 	TEST_ASSERT(test_send_host_command(EC_CMD_FLASH_INFO, 1, NULL, 0, &resp,
 					   sizeof(resp)) == EC_RES_SUCCESS);
 
-	TEST_CHECK((resp.flash_size == CONFIG_FLASH_SIZE_BYTES) &&
-		   (resp.write_block_size == CONFIG_FLASH_WRITE_SIZE) &&
-		   (resp.erase_block_size == CONFIG_FLASH_ERASE_SIZE) &&
-		   (resp.protect_block_size == CONFIG_FLASH_BANK_SIZE));
+	TEST_ASSERT(resp.flash_size == CONFIG_FLASH_SIZE_BYTES);
+	TEST_ASSERT(resp.write_block_size == CONFIG_FLASH_WRITE_SIZE);
+	TEST_ASSERT(resp.erase_block_size == CONFIG_FLASH_ERASE_SIZE);
+	TEST_ASSERT(resp.protect_block_size == CONFIG_FLASH_BANK_SIZE);
+	return EC_SUCCESS;
 }
 
 static int test_region_info(void)
diff --git a/test/kb_8042.c b/test/kb_8042.c
index 0bd0c273c4..1a3681aa28 100644
--- a/test/kb_8042.c
+++ b/test/kb_8042.c
@@ -114,7 +114,8 @@ static int __verify_no_char(void)
 {
 	lpc_char_cnt = 0;
 	msleep(30);
-	TEST_CHECK(lpc_char_cnt == 0);
+	TEST_ASSERT(lpc_char_cnt == 0);
+	return EC_SUCCESS;
 }
 
 #define VERIFY_NO_CHAR() TEST_ASSERT(__verify_no_char() == EC_SUCCESS)
diff --git a/test/kb_scan.c b/test/kb_scan.c
index b36f868b8e..57d16bc73d 100644
--- a/test/kb_scan.c
+++ b/test/kb_scan.c
@@ -544,12 +544,14 @@ static int lid_test(void)
 
 static int test_check_boot_esc(void)
 {
-	TEST_CHECK(keyboard_scan_get_boot_keys() == BOOT_KEY_ESC);
+	TEST_ASSERT(keyboard_scan_get_boot_keys() == BOOT_KEY_ESC);
+	return EC_SUCCESS;
 }
 
 static int test_check_boot_down(void)
 {
-	TEST_CHECK(keyboard_scan_get_boot_keys() == BOOT_KEY_DOWN_ARROW);
+	TEST_ASSERT(keyboard_scan_get_boot_keys() == BOOT_KEY_DOWN_ARROW);
+	return EC_SUCCESS;
 }
 
 void test_init(void)
diff --git a/test/stdlib.c b/test/stdlib.c
index cd018af09b..89ba90b54d 100644
--- a/test/stdlib.c
+++ b/test/stdlib.c
@@ -16,28 +16,30 @@
 
 static int test_isalpha(void)
 {
-	TEST_CHECK(isalpha('a'));
-	TEST_CHECK(isalpha('z'));
-	TEST_CHECK(isalpha('A'));
-	TEST_CHECK(isalpha('Z'));
-	TEST_CHECK(!isalpha('0'));
-	TEST_CHECK(!isalpha('~'));
-	TEST_CHECK(!isalpha(' '));
-	TEST_CHECK(!isalpha('\0'));
-	TEST_CHECK(!isalpha('\n'));
+	TEST_ASSERT(isalpha('a'));
+	TEST_ASSERT(isalpha('z'));
+	TEST_ASSERT(isalpha('A'));
+	TEST_ASSERT(isalpha('Z'));
+	TEST_ASSERT(!isalpha('0'));
+	TEST_ASSERT(!isalpha('~'));
+	TEST_ASSERT(!isalpha(' '));
+	TEST_ASSERT(!isalpha('\0'));
+	TEST_ASSERT(!isalpha('\n'));
+	return EC_SUCCESS;
 }
 
 static int test_isprint(void)
 {
-	TEST_CHECK(isprint('a'));
-	TEST_CHECK(isprint('z'));
-	TEST_CHECK(isprint('A'));
-	TEST_CHECK(isprint('Z'));
-	TEST_CHECK(isprint('0'));
-	TEST_CHECK(isprint('~'));
-	TEST_CHECK(isprint(' '));
-	TEST_CHECK(!isprint('\0'));
-	TEST_CHECK(!isprint('\n'));
+	TEST_ASSERT(isprint('a'));
+	TEST_ASSERT(isprint('z'));
+	TEST_ASSERT(isprint('A'));
+	TEST_ASSERT(isprint('Z'));
+	TEST_ASSERT(isprint('0'));
+	TEST_ASSERT(isprint('~'));
+	TEST_ASSERT(isprint(' '));
+	TEST_ASSERT(!isprint('\0'));
+	TEST_ASSERT(!isprint('\n'));
+	return EC_SUCCESS;
 }
 
 static int test_strstr(void)
@@ -145,7 +147,8 @@ static int test_strncmp(void)
 
 static int test_strlen(void)
 {
-	TEST_CHECK(strlen("this is a string") == 16);
+	TEST_ASSERT(strlen("this is a string") == 16);
+	return EC_SUCCESS;
 }
 
 static int test_strnlen(void)
@@ -159,35 +162,39 @@ static int test_strnlen(void)
 
 static int test_strcasecmp(void)
 {
-	TEST_CHECK(strcasecmp("test string", "TEST strIng") == 0);
-	TEST_CHECK(strcasecmp("test123!@#", "TesT123!@#") == 0);
-	TEST_CHECK(strcasecmp("lower", "UPPER") != 0);
+	TEST_ASSERT(strcasecmp("test string", "TEST strIng") == 0);
+	TEST_ASSERT(strcasecmp("test123!@#", "TesT123!@#") == 0);
+	TEST_ASSERT(strcasecmp("lower", "UPPER") != 0);
+	return EC_SUCCESS;
 }
 
 static int test_strncasecmp(void)
 {
-	TEST_CHECK(strncasecmp("test string", "TEST str", 4) == 0);
-	TEST_CHECK(strncasecmp("test string", "TEST str", 8) == 0);
-	TEST_CHECK(strncasecmp("test123!@#", "TesT321!@#", 5) != 0);
-	TEST_CHECK(strncasecmp("test123!@#", "TesT321!@#", 4) == 0);
-	TEST_CHECK(strncasecmp("1test123!@#", "1TesT321!@#", 5) == 0);
-	TEST_CHECK(strncasecmp("1test123", "teststr", 0) == 0);
+	TEST_ASSERT(strncasecmp("test string", "TEST str", 4) == 0);
+	TEST_ASSERT(strncasecmp("test string", "TEST str", 8) == 0);
+	TEST_ASSERT(strncasecmp("test123!@#", "TesT321!@#", 5) != 0);
+	TEST_ASSERT(strncasecmp("test123!@#", "TesT321!@#", 4) == 0);
+	TEST_ASSERT(strncasecmp("1test123!@#", "1TesT321!@#", 5) == 0);
+	TEST_ASSERT(strncasecmp("1test123", "teststr", 0) == 0);
+	return EC_SUCCESS;
 }
 
 static int test_atoi(void)
 {
-	TEST_CHECK(atoi("  901") == 901);
-	TEST_CHECK(atoi("-12c") == -12);
-	TEST_CHECK(atoi("   0  ") == 0);
-	TEST_CHECK(atoi("\t111") == 111);
+	TEST_ASSERT(atoi("  901") == 901);
+	TEST_ASSERT(atoi("-12c") == -12);
+	TEST_ASSERT(atoi("   0  ") == 0);
+	TEST_ASSERT(atoi("\t111") == 111);
+	return EC_SUCCESS;
 }
 
 static int test_snprintf(void)
 {
 	char buffer[32];
 
-	TEST_CHECK(snprintf(buffer, sizeof(buffer), "%u", 1234) == 4);
-	TEST_CHECK(strncmp(buffer, "1234", sizeof(buffer)));
+	TEST_ASSERT(snprintf(buffer, sizeof(buffer), "%u", 1234) == 4);
+	TEST_ASSERT(strncmp(buffer, "1234", sizeof(buffer)) == 0);
+	return EC_SUCCESS;
 }
 
 static int test_strcspn(void)
diff --git a/test/utils.c b/test/utils.c
index 11298a40c9..d5bdcf6b4e 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -21,7 +21,8 @@ static int test_uint64divmod_0(void)
 	int d = 54870071;
 	int r = uint64divmod(&n, d);
 
-	TEST_CHECK(r == 5991285 && n == 156134415ULL);
+	TEST_ASSERT(r == 5991285 && n == 156134415ULL);
+	return EC_SUCCESS;
 }
 
 static int test_uint64divmod_1(void)
@@ -30,7 +31,8 @@ static int test_uint64divmod_1(void)
 	int d = 2;
 	int r = uint64divmod(&n, d);
 
-	TEST_CHECK(r == 0 && n == 4283553221292375ULL);
+	TEST_ASSERT(r == 0 && n == 4283553221292375ULL);
+	return EC_SUCCESS;
 }
 
 static int test_uint64divmod_2(void)
@@ -39,7 +41,8 @@ static int test_uint64divmod_2(void)
 	int d = 0;
 	int r = uint64divmod(&n, d);
 
-	TEST_CHECK(r == 0 && n == 0ULL);
+	TEST_ASSERT(r == 0 && n == 0ULL);
+	return EC_SUCCESS;
 }
 
 static int test_get_next_bit(void)