From c1ffd337f14788d596f2c3b869e20b3cca5b1e8a Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Fri, 22 Mar 2013 17:13:23 +0000 Subject: [PATCH] Add trace printouts to all unit tests. Unfortunately, this requires splitting system_wrappers_unittests out of system_wrappers.gyp to avoid a cyclic dependency. TESTED=ran a few unit tests and observed printouts Review URL: https://webrtc-codereview.appspot.com/1221006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3711 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc.gyp | 3 +- webrtc/system_wrappers/interface/trace.h | 7 +++ .../source/logging_unittest.cc | 8 +-- .../source/system_wrappers.gyp | 54 ----------------- .../source/system_wrappers_tests.gyp | 59 +++++++++++++++++++ webrtc/system_wrappers/source/trace_impl.cc | 26 ++++---- .../source/unittest_utilities.h | 1 + webrtc/test/test.gyp | 1 + webrtc/test/test_suite.cc | 39 ++++++++++-- webrtc/test/test_suite.h | 8 +++ 10 files changed, 129 insertions(+), 77 deletions(-) create mode 100644 webrtc/system_wrappers/source/system_wrappers_tests.gyp diff --git a/webrtc.gyp b/webrtc.gyp index 46e87bbfea..3c5742576d 100644 --- a/webrtc.gyp +++ b/webrtc.gyp @@ -29,9 +29,10 @@ 'conditions': [ ['include_tests==1', { 'dependencies': [ + 'webrtc/system_wrappers/source/system_wrappers_tests.gyp:*', + 'webrtc/test/channel_transport.gyp:*', 'webrtc/test/metrics.gyp:*', 'webrtc/test/test.gyp:*', - 'webrtc/test/channel_transport.gyp:*', 'webrtc/tools/tools.gyp:*', 'tools/e2e_quality/e2e_quality.gyp:*', ], diff --git a/webrtc/system_wrappers/interface/trace.h b/webrtc/system_wrappers/interface/trace.h index 92bacf312d..4cbda45ed5 100644 --- a/webrtc/system_wrappers/interface/trace.h +++ b/webrtc/system_wrappers/interface/trace.h @@ -29,6 +29,13 @@ namespace webrtc { class Trace { public: + // The length of the trace text preceeding the log message. + static const int kBoilerplateLength; + // The position of the timestamp text within a trace. + static const int kTimestampPosition; + // The length of the timestamp (without "delta" field). + static const int kTimestampLength; + // Increments the reference count to the trace. static void CreateTrace(); // Decrements the reference count to the trace. diff --git a/webrtc/system_wrappers/source/logging_unittest.cc b/webrtc/system_wrappers/source/logging_unittest.cc index 8d8a580f78..de790a5422 100644 --- a/webrtc/system_wrappers/source/logging_unittest.cc +++ b/webrtc/system_wrappers/source/logging_unittest.cc @@ -20,18 +20,16 @@ namespace webrtc { namespace { -const size_t kBoilerplateLength = 71; - class LoggingTest : public ::testing::Test, public TraceCallback { public: virtual void Print(TraceLevel level, const char* msg, int length) { CriticalSectionScoped cs(crit_.get()); // We test the length here to ensure (with high likelihood) that only our // traces will be tested. - if (level_ != kTraceNone && - expected_log_.str().size() == length - kBoilerplateLength - 1) { + if (level_ != kTraceNone && static_cast(expected_log_.str().size()) == + length - Trace::kBoilerplateLength - 1) { EXPECT_EQ(level_, level); - EXPECT_EQ(expected_log_.str(), &msg[kBoilerplateLength]); + EXPECT_EQ(expected_log_.str(), &msg[Trace::kBoilerplateLength]); level_ = kTraceNone; cv_->Wake(); } diff --git a/webrtc/system_wrappers/source/system_wrappers.gyp b/webrtc/system_wrappers/source/system_wrappers.gyp index c931b177a0..fca9bf7e0c 100644 --- a/webrtc/system_wrappers/source/system_wrappers.gyp +++ b/webrtc/system_wrappers/source/system_wrappers.gyp @@ -233,60 +233,6 @@ }, ], }], - ['include_tests==1', { - 'targets': [ - { - 'target_name': 'system_wrappers_unittests', - 'type': 'executable', - 'dependencies': [ - 'system_wrappers', - '<(DEPTH)/testing/gtest.gyp:gtest', - '<(webrtc_root)/test/test.gyp:test_support_main', - ], - 'sources': [ - 'aligned_malloc_unittest.cc', - 'condition_variable_unittest.cc', - 'cpu_wrapper_unittest.cc', - 'cpu_measurement_harness.h', - 'cpu_measurement_harness.cc', - 'critical_section_unittest.cc', - 'event_tracer_unittest.cc', - 'list_unittest.cc', - 'logging_unittest.cc', - 'map_unittest.cc', - 'data_log_unittest.cc', - 'data_log_unittest_disabled.cc', - 'data_log_helpers_unittest.cc', - 'data_log_c_helpers_unittest.c', - 'data_log_c_helpers_unittest.h', - 'stringize_macros_unittest.cc', - 'thread_unittest.cc', - 'thread_posix_unittest.cc', - 'trace_unittest.cc', - 'unittest_utilities_unittest.cc', - ], - 'conditions': [ - ['enable_data_logging==1', { - 'sources!': [ 'data_log_unittest_disabled.cc', ], - }, { - 'sources!': [ 'data_log_unittest.cc', ], - }], - ['os_posix==0', { - 'sources!': [ 'thread_posix_unittest.cc', ], - }], - ], - # Disable warnings to enable Win64 build, issue 1323. - 'msvs_disabled_warnings': [ - 4267, # size_t to int truncation. - ], - }, - ], # targets - }], # include_tests ], # conditions } -# Local Variables: -# tab-width:2 -# indent-tabs-mode:nil -# End: -# vim: set expandtab tabstop=2 shiftwidth=2: diff --git a/webrtc/system_wrappers/source/system_wrappers_tests.gyp b/webrtc/system_wrappers/source/system_wrappers_tests.gyp new file mode 100644 index 0000000000..66939e99c4 --- /dev/null +++ b/webrtc/system_wrappers/source/system_wrappers_tests.gyp @@ -0,0 +1,59 @@ +# Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +{ + 'includes': ['../../build/common.gypi',], + 'targets': [ + { + 'target_name': 'system_wrappers_unittests', + 'type': 'executable', + 'dependencies': [ + '<(DEPTH)/testing/gtest.gyp:gtest', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', + '<(webrtc_root)/test/test.gyp:test_support_main', + ], + 'sources': [ + 'aligned_malloc_unittest.cc', + 'condition_variable_unittest.cc', + 'cpu_wrapper_unittest.cc', + 'cpu_measurement_harness.h', + 'cpu_measurement_harness.cc', + 'critical_section_unittest.cc', + 'event_tracer_unittest.cc', + 'list_unittest.cc', + 'logging_unittest.cc', + 'map_unittest.cc', + 'data_log_unittest.cc', + 'data_log_unittest_disabled.cc', + 'data_log_helpers_unittest.cc', + 'data_log_c_helpers_unittest.c', + 'data_log_c_helpers_unittest.h', + 'stringize_macros_unittest.cc', + 'thread_unittest.cc', + 'thread_posix_unittest.cc', + 'trace_unittest.cc', + 'unittest_utilities_unittest.cc', + ], + 'conditions': [ + ['enable_data_logging==1', { + 'sources!': [ 'data_log_unittest_disabled.cc', ], + }, { + 'sources!': [ 'data_log_unittest.cc', ], + }], + ['os_posix==0', { + 'sources!': [ 'thread_posix_unittest.cc', ], + }], + ], + # Disable warnings to enable Win64 build, issue 1323. + 'msvs_disabled_warnings': [ + 4267, # size_t to int truncation. + ], + }, + ], +} + diff --git a/webrtc/system_wrappers/source/trace_impl.cc b/webrtc/system_wrappers/source/trace_impl.cc index 2f65d06b60..de7c58c439 100644 --- a/webrtc/system_wrappers/source/trace_impl.cc +++ b/webrtc/system_wrappers/source/trace_impl.cc @@ -31,6 +31,10 @@ namespace webrtc { +const int Trace::kBoilerplateLength = 71; +const int Trace::kTimestampPosition = 13; +const int Trace::kTimestampLength = 12; + static WebRtc_UWord32 level_filter = kTraceDefault; // Construct On First Use idiom. Avoids "static initialization order fiasco". @@ -408,16 +412,9 @@ WebRtc_Word32 TraceImpl::AddMessage( } void TraceImpl::AddMessageToList( - const char trace_message[WEBRTC_TRACE_MAX_MESSAGE_SIZE], - const WebRtc_UWord16 length, - const TraceLevel level) { -#ifdef WEBRTC_DIRECT_TRACE - if (callback_) { - callback_->Print(level, trace_message, length); - } - return; -#endif - + const char trace_message[WEBRTC_TRACE_MAX_MESSAGE_SIZE], + const WebRtc_UWord16 length, + const TraceLevel level) { CriticalSectionScoped lock(critsect_array_); if (next_free_idx_[active_queue_] >= WEBRTC_TRACE_MAX_QUEUE) { @@ -469,11 +466,16 @@ bool TraceImpl::Run(void* obj) { bool TraceImpl::Process() { if (event_.Wait(1000) == kEventSignaled) { - if (trace_file_.Open() || callback_) { - // File mode (not callback mode). + // This slightly odd construction is to avoid locking |critsect_interface_| + // while calling WriteToFile() since it's locked inside the function. + critsect_interface_->Enter(); + bool write_to_file = trace_file_.Open() || callback_; + critsect_interface_->Leave(); + if (write_to_file) { WriteToFile(); } } else { + CriticalSectionScoped lock(critsect_interface_); trace_file_.Flush(); } return true; diff --git a/webrtc/system_wrappers/source/unittest_utilities.h b/webrtc/system_wrappers/source/unittest_utilities.h index cfc5392a6b..32eabfb813 100644 --- a/webrtc/system_wrappers/source/unittest_utilities.h +++ b/webrtc/system_wrappers/source/unittest_utilities.h @@ -67,6 +67,7 @@ class ScopedTracing { void StopTrace() { if (logging_) { + Trace::SetTraceCallback(NULL); Trace::ReturnTrace(); } } diff --git a/webrtc/test/test.gyp b/webrtc/test/test.gyp index 304e532052..ff83ec53c9 100644 --- a/webrtc/test/test.gyp +++ b/webrtc/test/test.gyp @@ -22,6 +22,7 @@ 'dependencies': [ '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/testing/gmock.gyp:gmock', + '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers', ], 'all_dependent_settings': { 'include_dirs': [ diff --git a/webrtc/test/test_suite.cc b/webrtc/test/test_suite.cc index 81913fd7ae..e0208e2186 100644 --- a/webrtc/test/test_suite.cc +++ b/webrtc/test/test_suite.cc @@ -9,15 +9,39 @@ */ #include "test/test_suite.h" -#include "test/testsupport/fileutils.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" +#include + +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "test/testsupport/fileutils.h" +#include "webrtc/system_wrappers/interface/trace.h" namespace webrtc { namespace test { -TestSuite::TestSuite(int argc, char** argv) { +const int kLevelFilter = kTraceError | kTraceWarning | kTraceTerseInfo; + +class TraceCallbackImpl : public TraceCallback { + public: + TraceCallbackImpl() { } + virtual ~TraceCallbackImpl() { } + + virtual void Print(TraceLevel level, const char* msg_array, int length) { + if (level & kLevelFilter) { + ASSERT_GT(length, Trace::kBoilerplateLength); + std::string msg = msg_array; + std::string msg_time = msg.substr(Trace::kTimestampPosition, + Trace::kTimestampLength); + std::string msg_log = msg.substr(Trace::kBoilerplateLength); + fprintf(stderr, "%s %s\n", msg_time.c_str(), msg_log.c_str()); + fflush(stderr); + } + } +}; + +TestSuite::TestSuite(int argc, char** argv) + : trace_callback_(new TraceCallbackImpl) { SetExecutablePath(argv[0]); testing::InitGoogleMock(&argc, argv); // Runs InitGoogleTest() internally. } @@ -33,10 +57,15 @@ int TestSuite::Run() { } void TestSuite::Initialize() { - // TODO(andrew): initialize singletons here (e.g. Trace). + Trace::CreateTrace(); + Trace::SetTraceCallback(trace_callback_.get()); + Trace::SetLevelFilter(kLevelFilter); } void TestSuite::Shutdown() { + Trace::SetTraceCallback(NULL); + Trace::ReturnTrace(); } + } // namespace test } // namespace webrtc diff --git a/webrtc/test/test_suite.h b/webrtc/test/test_suite.h index 595c5a98ea..340c2151fc 100644 --- a/webrtc/test/test_suite.h +++ b/webrtc/test/test_suite.h @@ -18,9 +18,13 @@ // any gtest based tests that are linked into your executable. #include "system_wrappers/interface/constructor_magic.h" +#include "system_wrappers/interface/scoped_ptr.h" namespace webrtc { namespace test { + +class TraceCallbackImpl; + class TestSuite { public: TestSuite(int argc, char** argv); @@ -35,7 +39,11 @@ class TestSuite { virtual void Shutdown(); DISALLOW_COPY_AND_ASSIGN(TestSuite); + + private: + scoped_ptr trace_callback_; }; + } // namespace test } // namespace webrtc