From 5f4c79f510f66b85c2f9a35b173fc3471c68f55e Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 10 Apr 2020 13:48:05 -0700 Subject: [PATCH] liblog: cleanup TODOs in tests 1) log_msg.msg() will never be nullptr, unless logd sends an invalid response, so it's the right idea to ASSERT() that this is true instead of just checking and continuing. 2) Even though liblog.too_big_payload is tautological, there's no reason not to test it, in case the assumptions that make it tautological change. 3) We're not too worried about the return value of logging functions or that liblog prevents them from being written (anyone can write their own values to logd after all). Test: liblog-unit-tests Change-Id: I144cc7cf45c164ea5f04e0786ff0e298fd626f07 --- liblog/tests/liblog_test.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp index a031531cb..048bf610f 100644 --- a/liblog/tests/liblog_test.cpp +++ b/liblog/tests/liblog_test.cpp @@ -97,10 +97,7 @@ static void RunLogTests(log_id_t log_buffer, FWrite write_messages, FCheck check ASSERT_EQ(log_buffer, log_msg.id()); ASSERT_EQ(pid, log_msg.entry.pid); - // TODO: Should this be an assert? - if (log_msg.msg() == nullptr) { - continue; - } + ASSERT_NE(nullptr, log_msg.msg()); check_message(log_msg, &found); } @@ -121,10 +118,7 @@ static void RunLogTests(log_id_t log_buffer, FWrite write_messages, FCheck check ASSERT_EQ(log_buffer, log_msg.id()); ASSERT_EQ(pid, log_msg.entry.pid); - // TODO: Should this be an assert? - if (log_msg.msg() == nullptr) { - continue; - } + ASSERT_NE(nullptr, log_msg.msg()); found = false; check_message(log_msg, &found); @@ -1031,7 +1025,7 @@ TEST(liblog, __android_log_buf_print__maxtag) { #endif } -// TODO: This test is tautological. android_logger_list_read() calls recv() with +// Note: This test is tautological. android_logger_list_read() calls recv() with // LOGGER_ENTRY_MAX_PAYLOAD as its size argument, so it's not possible for this test to read a // payload larger than that size. TEST(liblog, too_big_payload) { @@ -1982,7 +1976,6 @@ TEST(liblog, #endif } -// TODO: Do we need to check that we didn't actually write anything if we return a failure here? TEST(liblog, android_errorWriteWithInfoLog__android_logger_list_read__null_data) { #ifdef __ANDROID__