coverage report: lower log level to trace and drop all logs (#3065)

Signed-off-by: Christoph Pakulski <paker8848@gmail.com>

source/server/options_impl.cc

@@ -38,7 +38,6 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r
   }
   log_levels_string +=
       fmt::format("\nDefault is [{}]", spdlog::level::level_names[default_log_level]);
-  log_levels_string += "\n[trace] and [debug] are only available on debug builds";
 
   const std::string log_format_string =
       fmt::format("Log message format in spdlog syntax "

test/BUILD

@@ -18,6 +18,7 @@ envoy_cc_test_library(
         "//source/common/common:thread_lib",
         "//source/common/event:libevent_lib",
         "//test/test_common:environment_lib",
+        "//test/mocks/access_log:access_log_mocks",
         "//test/test_common:printers_lib",
     ] + select({
         "//bazel:disable_signal_trace": [],

test/common/compressor/zlib_compressor_impl_test.cc

@@ -39,9 +39,9 @@ class ZlibCompressorImplDeathTest : public ZlibCompressorImplTest {
  * compress before init.
  */
 TEST_F(ZlibCompressorImplDeathTest, CompressorTestDeath) {
-  EXPECT_DEATH(compressorBadInitTestHelper(100, 8), std::string{"assert failure: result >= 0"});
-  EXPECT_DEATH(compressorBadInitTestHelper(31, 10), std::string{"assert failure: result >= 0"});
-  EXPECT_DEATH(unitializedCompressorTestHelper(), std::string{"assert failure: result == Z_OK"});
+  EXPECT_DEATH_LOG_TO_STDERR(compressorBadInitTestHelper(100, 8), "assert failure: result >= 0");
+  EXPECT_DEATH_LOG_TO_STDERR(compressorBadInitTestHelper(31, 10), "assert failure: result >= 0");
+  EXPECT_DEATH_LOG_TO_STDERR(unitializedCompressorTestHelper(), "assert failure: result == Z_OK");
 }
 
 /**

test/common/decompressor/zlib_decompressor_impl_test.cc

@@ -39,8 +39,8 @@ class ZlibDecompressorImplDeathTest : public ZlibDecompressorImplTest {
  * decompress before init.
  */
 TEST_F(ZlibDecompressorImplDeathTest, DecompressorTestDeath) {
-  EXPECT_DEATH(decompressorBadInitTestHelper(100), std::string{"assert failure: result >= 0"});
-  EXPECT_DEATH(unitializedDecompressorTestHelper(), std::string{"assert failure: result == Z_OK"});
+  EXPECT_DEATH_LOG_TO_STDERR(decompressorBadInitTestHelper(100), "assert failure: result >= 0");
+  EXPECT_DEATH_LOG_TO_STDERR(unitializedDecompressorTestHelper(), "assert failure: result == Z_OK");
 }
 
 /**

test/common/network/address_impl_test.cc

@@ -332,9 +332,9 @@ TEST(AddressFromSockAddr, IPv4) {
   EXPECT_EQ(1, inet_pton(AF_INET, "1.2.3.4", &sin.sin_addr));
   sin.sin_port = htons(6502);
 
-  EXPECT_DEATH(addressFromSockAddr(ss, 1), "ss_len");
-  EXPECT_DEATH(addressFromSockAddr(ss, sizeof(sockaddr_in) - 1), "ss_len");
-  EXPECT_DEATH(addressFromSockAddr(ss, sizeof(sockaddr_in) + 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, sizeof(sockaddr_in) - 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, sizeof(sockaddr_in) + 1), "ss_len");
 
   EXPECT_EQ("1.2.3.4:6502", addressFromSockAddr(ss, sizeof(sockaddr_in))->asString());
 
@@ -351,9 +351,9 @@ TEST(AddressFromSockAddr, IPv6) {
   EXPECT_EQ(1, inet_pton(AF_INET6, "01:023::00Ef", &sin6.sin6_addr));
   sin6.sin6_port = htons(32000);
 
-  EXPECT_DEATH(addressFromSockAddr(ss, 1), "ss_len");
-  EXPECT_DEATH(addressFromSockAddr(ss, sizeof(sockaddr_in6) - 1), "ss_len");
-  EXPECT_DEATH(addressFromSockAddr(ss, sizeof(sockaddr_in6) + 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, sizeof(sockaddr_in6) - 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, sizeof(sockaddr_in6) + 1), "ss_len");
 
   EXPECT_EQ("[1:23::ef]:32000", addressFromSockAddr(ss, sizeof(sockaddr_in6))->asString());
 
@@ -374,8 +374,9 @@ TEST(AddressFromSockAddr, Pipe) {
 
   StringUtil::strlcpy(sun.sun_path, "/some/path", sizeof sun.sun_path);
 
-  EXPECT_DEATH(addressFromSockAddr(ss, 1), "ss_len");
-  EXPECT_DEATH(addressFromSockAddr(ss, offsetof(struct sockaddr_un, sun_path)), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, 1), "ss_len");
+  EXPECT_DEATH_LOG_TO_STDERR(addressFromSockAddr(ss, offsetof(struct sockaddr_un, sun_path)),
+                             "ss_len");
 
   socklen_t ss_len = offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sun.sun_path);
   EXPECT_EQ("/some/path", addressFromSockAddr(ss, ss_len)->asString());

test/common/network/connection_impl_test.cc

@@ -78,10 +78,10 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ConnectionImplDeathTest,
 
 TEST_P(ConnectionImplDeathTest, BadFd) {
   Event::DispatcherImpl dispatcher;
-  EXPECT_DEATH(ConnectionImpl(dispatcher,
-                              std::make_unique<ConnectionSocketImpl>(-1, nullptr, nullptr),
-                              Network::Test::createRawBufferSocket(), false),
-               ".*assert failure: fd\\(\\) != -1.*");
+  EXPECT_DEATH_LOG_TO_STDERR(
+      ConnectionImpl(dispatcher, std::make_unique<ConnectionSocketImpl>(-1, nullptr, nullptr),
+                     Network::Test::createRawBufferSocket(), false),
+      ".*assert failure: fd\\(\\) != -1.*");
 }
 
 class ConnectionImplTest : public testing::TestWithParam<Address::IpVersion> {

test/common/network/listener_impl_test.cc

@@ -59,7 +59,7 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ListenerImplDeathTest,
                         TestUtility::ipTestParamsToString);
 
 TEST_P(ListenerImplDeathTest, ErrorCallback) {
-  EXPECT_DEATH(errorCallbackTest(GetParam()), ".*listener accept failure.*");
+  EXPECT_DEATH_LOG_TO_STDERR(errorCallbackTest(GetParam()), ".*listener accept failure.*");
 }
 
 class TestListenerImpl : public ListenerImpl {

test/common/singleton/BUILD

@@ -13,6 +13,7 @@ envoy_cc_test(
     srcs = ["manager_impl_test.cc"],
     deps = [
         "//source/common/singleton:manager_impl_lib",
+        "//test/test_common:utility_lib",
     ],
 )
 

test/common/singleton/manager_impl_test.cc

@@ -2,6 +2,8 @@
 
 #include "common/singleton/manager_impl.h"
 
+#include "test/test_common/utility.h"
+
 #include "gmock/gmock.h"
 
 namespace Envoy {
@@ -14,7 +16,8 @@ static void deathTestWorker() {
 }
 
 TEST(SingletonManagerImplDeathTest, NotRegistered) {
-  EXPECT_DEATH(deathTestWorker(), "invalid singleton name 'foo'. Make sure it is registered.");
+  EXPECT_DEATH_LOG_TO_STDERR(deathTestWorker(),
+                             "invalid singleton name 'foo'. Make sure it is registered.");
 }
 
 static constexpr char test_singleton_name[] = "test_singleton";

test/exe/BUILD

@@ -37,5 +37,8 @@ envoy_cc_test(
     name = "signals_test",
     srcs = ["signals_test.cc"],
     tags = ["backtrace"],
-    deps = ["//source/exe:sigaction_lib"],
+    deps = [
+        "//source/exe:sigaction_lib",
+        "//test/test_common:utility_lib",
+    ],
 )

test/exe/signals_test.cc

@@ -3,6 +3,8 @@
 
 #include "exe/signal_action.h"
 
+#include "test/test_common/utility.h"
+
 #include "gtest/gtest.h"
 
 namespace Envoy {
@@ -23,7 +25,7 @@ namespace Envoy {
 #ifndef ASANITIZED
 TEST(Signals, InvalidAddressDeathTest) {
   SignalAction actions;
-  EXPECT_DEATH(
+  EXPECT_DEATH_LOG_TO_STDERR(
       []() -> void {
         // Oooooops!
         volatile int* nasty_ptr = reinterpret_cast<int*>(0x0);
@@ -34,7 +36,7 @@ TEST(Signals, InvalidAddressDeathTest) {
 
 TEST(Signals, BusDeathTest) {
   SignalAction actions;
-  EXPECT_DEATH(
+  EXPECT_DEATH_LOG_TO_STDERR(
       []() -> void {
         // Bus error is tricky. There's one way that can work on POSIX systems
         // described below but it depends on mmaping a file. Just make it easy and
@@ -50,7 +52,7 @@ TEST(Signals, BusDeathTest) {
 
 TEST(Signals, BadMathDeathTest) {
   SignalAction actions;
-  EXPECT_DEATH(
+  EXPECT_DEATH_LOG_TO_STDERR(
       []() -> void {
         // It turns out to be really hard to not have the optimizer get rid of a
         // division by zero. Just raise the signal for this test.
@@ -63,7 +65,7 @@ TEST(Signals, BadMathDeathTest) {
 // Unfortunately we don't have a reliable way to do this on other platforms
 TEST(Signals, IllegalInstructionDeathTest) {
   SignalAction actions;
-  EXPECT_DEATH(
+  EXPECT_DEATH_LOG_TO_STDERR(
       []() -> void {
         // Intel defines the "ud2" opcode to be an invalid instruction:
         __asm__("ud2");
@@ -74,7 +76,7 @@ TEST(Signals, IllegalInstructionDeathTest) {
 
 TEST(Signals, AbortDeathTest) {
   SignalAction actions;
-  EXPECT_DEATH([]() -> void { abort(); }(), "backtrace.*Abort(ed)?");
+  EXPECT_DEATH_LOG_TO_STDERR([]() -> void { abort(); }(), "backtrace.*Abort(ed)?");
 }
 
 TEST(Signals, RestoredPreviousHandlerDeathTest) {
@@ -86,7 +88,7 @@ TEST(Signals, RestoredPreviousHandlerDeathTest) {
     // goes out of scope, NOT the default.
   }
   // Outer SignalAction should be active again:
-  EXPECT_DEATH([]() -> void { abort(); }(), "backtrace.*Abort(ed)?");
+  EXPECT_DEATH_LOG_TO_STDERR([]() -> void { abort(); }(), "backtrace.*Abort(ed)?");
 }
 #endif
 

test/run_envoy_bazel_coverage.sh

@@ -36,7 +36,8 @@ BAZEL_TEST_OPTIONS="${BAZEL_TEST_OPTIONS} -c dbg --copt=-DNDEBUG"
 "${BAZEL_COVERAGE}" --batch test "${COVERAGE_TARGET}" ${BAZEL_TEST_OPTIONS} \
   --cache_test_results=no --cxxopt="--coverage" --cxxopt="-DENVOY_CONFIG_COVERAGE=1" \
   --linkopt="--coverage" --define ENVOY_CONFIG_COVERAGE=1 --test_output=streamed \
-  --strategy=Genrule=standalone --spawn_strategy=standalone --test_timeout=1000
+  --strategy=Genrule=standalone --spawn_strategy=standalone --test_timeout=1000 \
+  --test_arg="--log-path /dev/null" --test_arg="-l trace"
 
 # The Bazel build has a lot of whack in it, in particular generated files, headers from external
 # deps, etc. So, we exclude this from gcov to avoid false reporting of these files in the html and

test/test_common/utility.h

@@ -58,6 +58,22 @@ namespace Envoy {
     ADD_FAILURE() << "Unexpected exception: " << std::string(e.what());                            \
   }
 
+/*
+  Macro to use instead of EXPECT_DEATH when stderr is produced by a logger.
+  It temporarily installs stderr sink and restores the original logger sink after the test
+  completes and sdterr_sink object goes of of scope.
+  EXPECT_DEATH(statement, regex) test passes when statement causes crash and produces error message
+  matching regex. Test fails when statement does not crash or it crashes but message does not
+  match regex. If a message produced during crash is redirected away from strerr, the test fails.
+  By installing StderrSinkDelegate, the macro forces EXPECT_DEATH to send any output produced by
+  statement to stderr.
+*/
+#define EXPECT_DEATH_LOG_TO_STDERR(statement, message)                                             \
+  do {                                                                                             \
+    Logger::StderrSinkDelegate stderr_sink(Logger::Registry::getSink());                           \
+    EXPECT_DEATH(statement, message);                                                              \
+  } while (false)
+
 // Random number generator which logs its seed to stderr. To repeat a test run with a non-zero seed
 // one can run the test with --test_arg=--gtest_random_seed=[seed]
 class TestRandomGenerator {

test/test_runner.h

@@ -2,6 +2,7 @@
 #include "common/common/thread.h"
 #include "common/event/libevent.h"
 
+#include "test/mocks/access_log/mocks.h"
 #include "test/test_common/environment.h"
 
 #include "gmock/gmock.h"
@@ -31,6 +32,15 @@ class TestRunner {
     Logger::Registry::initialize(TestEnvironment::getOptions().logLevel(),
                                  TestEnvironment::getOptions().logFormat(), lock);
 
+    // Allocate fake log access manager.
+    testing::NiceMock<AccessLog::MockAccessLogManager> access_log_manager;
+    std::unique_ptr<Logger::FileSinkDelegate> file_logger;
+
+    // Redirect all logs to fake file when --log-path arg is specified in command line.
+    if (!TestEnvironment::getOptions().logPath().empty()) {
+      file_logger = std::make_unique<Logger::FileSinkDelegate>(
+          TestEnvironment::getOptions().logPath(), access_log_manager, Logger::Registry::getSink());
+    }
     return RUN_ALL_TESTS();
   }
 };