From 68ae78f5a9a00adaf958cb4439abebb0980ccb27 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Sun, 10 Jan 2016 00:16:23 -0800 Subject: [PATCH] Make NamedEventTest deterministic. It turns out that unit tests in NamedEventTest (e.g. NamedEventBasicTest) were written with an assumption that Util::Sleep() is reliable and the test environment has enough CPU cores. Unfortunately, this kind of tests are quite flaky on VM with a few virtual cores. To make those tests deterministic and reliable, this CL only checks if NamedEventNotifier::Notify() happens before NamedEventListener::Wait() returns true. This CL also does: - Use TEST_F to make sure that user profile directory is isolated from the real user environment. - Use std::unique_ptr when possible. - Remove non-essential loops from IsOwnerTest and NamedEventBasicTest. We can already rely on gtest's options when we need those kind of stress tests hence hard-coding such a stress-test does not make much sense. BUG= TEST=unittest REF_BUG= REF_CL=105736130 --- src/ipc/named_event_test.cc | 151 ++++++++++++++++++++++------------ src/mozc_version_template.txt | 2 +- 2 files changed, 99 insertions(+), 54 deletions(-) diff --git a/src/ipc/named_event_test.cc b/src/ipc/named_event_test.cc index b70a0c66a..91f9ba04b 100644 --- a/src/ipc/named_event_test.cc +++ b/src/ipc/named_event_test.cc @@ -29,8 +29,11 @@ #include "ipc/named_event.h" +#include +#include #include +#include "base/clock.h" #include "base/port.h" #include "base/system_util.h" #include "base/thread.h" @@ -40,48 +43,83 @@ DECLARE_string(test_tmpdir); namespace mozc { - namespace { + const char kName[] = "named_event_test"; -const int kNumRequests = 5; -class NamedEventNotifierThread: public Thread { +class NamedEventListenerThread : public Thread { public: - void Run() { - Util::Sleep(100); - NamedEventNotifier n(kName); - EXPECT_TRUE(n.IsAvailable()); - Util::Sleep(500); - EXPECT_TRUE(n.Notify()); + NamedEventListenerThread(const string &name, + uint32 initial_wait_msec, + uint32 wait_msec, + size_t max_num_wait) + : listener_(name.c_str()), + initial_wait_msec_(initial_wait_msec), + wait_msec_(wait_msec), + max_num_wait_(max_num_wait), + first_triggered_ticks_(0) { + EXPECT_TRUE(listener_.IsAvailable()); } -}; -class NamedEventListenerThread: public Thread { - public: - void Run() { - NamedEventListener n(kName); - EXPECT_TRUE(n.IsAvailable()); - EXPECT_FALSE(n.Wait(500)); - EXPECT_TRUE(n.Wait(2000)); + void Run() override { + Util::Sleep(initial_wait_msec_); + for (size_t i = 0; i < max_num_wait_; ++i) { + const bool result = listener_.Wait(wait_msec_); + const uint64 ticks = Clock::GetTicks(); + if (result) { + first_triggered_ticks_ = ticks; + return; + } + } } + + uint64 first_triggered_ticks() const { + return first_triggered_ticks_; + } + + bool IsTriggered() const { + return first_triggered_ticks() > 0; + } + + private: + NamedEventListener listener_; + const uint32 initial_wait_msec_; + const uint32 wait_msec_; + const size_t max_num_wait_; + std::atomic first_triggered_ticks_; }; -TEST(NamedEventTest, NamedEventBasicTest) { - SystemUtil::SetUserProfileDirectory(FLAGS_test_tmpdir); +class NamedEventTest : public testing::Test { + void SetUp() override { + original_user_profile_directory_ = SystemUtil::GetUserProfileDirectory(); + SystemUtil::SetUserProfileDirectory(FLAGS_test_tmpdir); + } - for (int i = 0; i < kNumRequests; ++i) { - NamedEventNotifierThread t; - NamedEventListener l(kName); - EXPECT_TRUE(l.IsAvailable()); - t.Start(); - EXPECT_FALSE(l.Wait(200)); - Util::Sleep(100); - EXPECT_TRUE(l.Wait(500)); - t.Join(); + void TearDown() override { + SystemUtil::SetUserProfileDirectory(original_user_profile_directory_); + } + + private: + string original_user_profile_directory_; +}; + +TEST_F(NamedEventTest, NamedEventBasicTest) { + NamedEventListenerThread listner(kName, 0, 50, 100); + listner.Start(); + Util::Sleep(200); + NamedEventNotifier notifier(kName); + ASSERT_TRUE(notifier.IsAvailable()); + const uint64 notify_ticks = Clock::GetTicks(); + notifier.Notify(); + listner.Join(); + + // There is a chance that |listner| is not triggered. + if (listner.IsTriggered()) { + EXPECT_LT(notify_ticks, listner.first_triggered_ticks()); } } -TEST(NamedEventTest, IsAvailableTest) { +TEST_F(NamedEventTest, IsAvailableTest) { { NamedEventListener l(kName); EXPECT_TRUE(l.IsAvailable()); @@ -96,47 +134,54 @@ TEST(NamedEventTest, IsAvailableTest) { } } -TEST(NamedEventTest, IsOwnerTest) { - for (int i = 0; i < kNumRequests; ++i) { - NamedEventListener l1(kName); - EXPECT_TRUE(l1.IsOwner()); - EXPECT_TRUE(l1.IsAvailable()); - NamedEventListener l2(kName); - EXPECT_FALSE(l2.IsOwner()); // the instance is owneded by l1 - EXPECT_TRUE(l2.IsAvailable()); - } +TEST_F(NamedEventTest, IsOwnerTest) { + NamedEventListener l1(kName); + EXPECT_TRUE(l1.IsOwner()); + EXPECT_TRUE(l1.IsAvailable()); + NamedEventListener l2(kName); + EXPECT_FALSE(l2.IsOwner()); // the instance is owneded by l1 + EXPECT_TRUE(l2.IsAvailable()); } -TEST(NamedEventTest, NamedEventMultipleListenerTest) { - SystemUtil::SetUserProfileDirectory(FLAGS_test_tmpdir); +TEST_F(NamedEventTest, NamedEventMultipleListenerTest) { + const size_t kNumRequests = 4; // mozc::Thread is not designed as value-semantics. // So here we use pointers to maintain these instances. - vector t(kNumRequests); - for (int i = 0; i < kNumRequests; ++i) { - t[i] = new NamedEventListenerThread; - t[i]->Start(); + vector> listeners(kNumRequests); + for (size_t i = 0; i < kNumRequests; ++i) { + listeners[i].reset(new NamedEventListenerThread(kName, 33 * i, 50, 100)); + listeners[i]->Start(); } + Util::Sleep(200); + // all |kNumRequests| listener events should be raised // at once with single notifier event - Util::Sleep(1000); - NamedEventNotifier n(kName); - EXPECT_TRUE(n.Notify()); - - for (int i = 0; i < kNumRequests; ++i) { - t[i]->Join(); - delete t[i]; - t[i] = NULL; + NamedEventNotifier notifier(kName); + ASSERT_TRUE(notifier.IsAvailable()); + const uint64 notify_ticks = Clock::GetTicks(); + ASSERT_TRUE(notifier.Notify()); + + for (size_t i = 0; i < kNumRequests; ++i) { + listeners[i]->Join(); + } + + for (size_t i = 0; i < kNumRequests; ++i) { + // There is a chance that |listeners[i]| is not triggered. + if (listeners[i]->IsTriggered()) { + EXPECT_LT(notify_ticks, listeners[i]->first_triggered_ticks()); + } } } -TEST(NamedEventTest, NamedEventPathLengthTest) { +TEST_F(NamedEventTest, NamedEventPathLengthTest) { #ifndef OS_WIN const string name_path = NamedEventUtil::GetEventPath(kName); // length should be less than 14 not includeing terminating null. EXPECT_EQ(13, strlen(name_path.c_str())); #endif // OS_WIN } + } // namespace } // namespace mozc diff --git a/src/mozc_version_template.txt b/src/mozc_version_template.txt index 062eb289d..98fa3d892 100644 --- a/src/mozc_version_template.txt +++ b/src/mozc_version_template.txt @@ -1,6 +1,6 @@ MAJOR=2 MINOR=17 -BUILD=2310 +BUILD=2311 REVISION=102 # NACL_DICTIONARY_VERSION is the target version of the system dictionary to be # downloaded by NaCl Mozc.