Permalink
Browse files

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
  • Loading branch information...
1 parent a2c1d40 commit 68ae78f5a9a00adaf958cb4439abebb0980ccb27 @yukawa yukawa committed Jan 10, 2016
Showing with 99 additions and 54 deletions.
  1. +98 −53 src/ipc/named_event_test.cc
  2. +1 −1 src/mozc_version_template.txt
@@ -29,8 +29,11 @@
#include "ipc/named_event.h"
+#include <atomic>
+#include <memory>
#include <string>
+#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<uint64> 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<NamedEventListenerThread *> t(kNumRequests);
- for (int i = 0; i < kNumRequests; ++i) {
- t[i] = new NamedEventListenerThread;
- t[i]->Start();
+ vector<std::unique_ptr<NamedEventListenerThread>> 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
@@ -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.

0 comments on commit 68ae78f

Please sign in to comment.