[chrome_elf] blacklist flaky tests

chrome_elf/blacklist/test/blacklist_test.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 
 #include <memory>
 
 #include "base/environment.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/i18n/case_conversion.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/scoped_native_library.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_reg_util_win.h"
+#include "base/test/test_timeouts.h"
 #include "base/win/registry.h"
 #include "chrome/common/chrome_version.h"
 #include "chrome/install_static/install_util.h"
 #include "chrome_elf/blacklist/blacklist.h"
 #include "chrome_elf/chrome_elf_constants.h"
 #include "chrome_elf/nt_registry/nt_registry.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 const wchar_t kTestDllName1[] = L"blacklist_test_dll_1.dll";
 const wchar_t kTestDllName2[] = L"blacklist_test_dll_2.dll";
 const wchar_t kTestDllName3[] = L"blacklist_test_dll_3.dll";
 
 const wchar_t kDll2Beacon[] = L"{F70A0100-2889-4629-9B44-610FE5C73231}";
 const wchar_t kDll3Beacon[] = L"{9E056AEC-169E-400c-B2D0-5A07E3ACE2EB}";
 
 extern const wchar_t* kEnvVars[];
 
 namespace {
 
+// Timeouts for synchronization.
+#define event_timeout \

[robertshield, 2017/05/05 13:04:43]

nit, mildly prefer: kEventTimeout or EVENT_TIMEOUT

[grt (UTC plus 2), 2017/05/05 13:11:47]

if this can't be a true constant owing to TestTimeouts, then please use a
function -- macros are strongly discouraged:

DWORD SychronizationTimeout() {
  return static_cast<DWORD>(
      TestTimeouts::action_timeout().InMillisecondsRoundedUp());
}

or something along those lines.

+  static_cast<DWORD>((TestTimeouts::action_timeout()).InMillisecondsRoundedUp())
+
+const wchar_t* g_blacklist_test_mutex = L"ChromeElfBlacklistTestMutex";

[grt (UTC plus 2), 2017/05/05 13:11:47]

also from the nit factory:

constexpr wchar_t g_blacklist_test_mutex[] = ...;

is the preferred way to spell this since it ensures that the constant is in
read-only data and it retains the string length at compile in case anyone cares
to know about it.

+
 // Functions we need from blacklist_test_main_dll.dll
 typedef bool (*TestDll_AddDllToBlacklistFunction)(const wchar_t* dll_name);
 typedef int (*TestDll_BlacklistSizeFunction)();
 typedef void (*TestDll_BlockedDllFunction)(size_t blocked_index);
 typedef int (*TestDll_GetBlacklistIndexFunction)(const wchar_t* dll_name);
 typedef bool (*TestDll_IsBlacklistInitializedFunction)();
 typedef bool (*TestDll_RemoveDllFromBlacklistFunction)(const wchar_t* dll_name);
 typedef bool (*TestDll_SuccessfullyBlockedFunction)(
     const wchar_t** blocked_dlls,
     int* size);
@@ skipping 16 matching lines @@
     {kTestDllName3, kDll3Beacon}
 };
 
 class BlacklistTest : public testing::Test {
  protected:
   BlacklistTest() : override_manager_(), num_initially_blocked_(0) {
   }
 
   void CheckBlacklistedDllsNotLoaded() {
     base::FilePath current_dir;
-    ASSERT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
+    EXPECT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
 
     for (size_t i = 0; i < arraysize(test_data); ++i) {
       // Ensure that the dll has not been loaded both by inspecting the handle
       // returned by LoadLibrary and by looking for an environment variable that
       // is set when the DLL's entry point is called.
       base::ScopedNativeLibrary dll_blacklisted(
           current_dir.Append(test_data[i].dll_name));
       EXPECT_FALSE(dll_blacklisted.is_valid());
       EXPECT_EQ(0u, ::GetEnvironmentVariable(test_data[i].dll_beacon, NULL, 0));
       dll_blacklisted.Reset(NULL);
@@ skipping 42 matching lines @@
   // The number of dlls initially blocked by the blacklist.
   int num_initially_blocked_;
 
  private:
   // This function puts registry-key redirection paths into
   // process-specific environment variables, for our test DLLs to access.
   // This will only work as long as the IPC is within the same process.
   void IpcOverrides() {
     base::string16 temp = nt::GetTestingOverride(nt::HKCU);
     if (!temp.empty())
-      ASSERT_TRUE(::SetEnvironmentVariableW(L"hkcu_override", temp.c_str()));
+      EXPECT_TRUE(::SetEnvironmentVariableW(L"hkcu_override", temp.c_str()));
     temp = nt::GetTestingOverride(nt::HKLM);
     if (!temp.empty())
-      ASSERT_TRUE(::SetEnvironmentVariableW(L"hklm_override", temp.c_str()));
+      EXPECT_TRUE(::SetEnvironmentVariableW(L"hklm_override", temp.c_str()));
   }
 
   void SetUp() override {
     base::string16 temp;
-    ASSERT_NO_FATAL_FAILURE(
+    EXPECT_NO_FATAL_FAILURE(
         override_manager_.OverrideRegistry(HKEY_CURRENT_USER, &temp));
-    ASSERT_TRUE(nt::SetTestingOverride(nt::HKCU, temp));
+    EXPECT_TRUE(nt::SetTestingOverride(nt::HKCU, temp));
 
     // Make the override path available to our test DLL.
     IpcOverrides();
 
     // Load the main test Dll now.
     // Note: this has to happen after we set up the registry overrides.
     HMODULE dll = nullptr;
     dll = ::LoadLibraryW(L"blacklist_test_main_dll.dll");
     if (!dll)
       return;
@@ skipping 39 matching lines @@
 
     // Find out how many dlls were blocked before the test starts.
     TestDll_SuccessfullyBlocked(NULL, &num_initially_blocked_);
   }
 
   void TearDown() override {
     TestDll_RemoveDllFromBlacklist(kTestDllName1);
     TestDll_RemoveDllFromBlacklist(kTestDllName2);
     TestDll_RemoveDllFromBlacklist(kTestDllName3);
 
-    ASSERT_TRUE(nt::SetTestingOverride(nt::HKCU, base::string16()));
+    EXPECT_TRUE(nt::SetTestingOverride(nt::HKCU, base::string16()));
   }
 };
 
 TEST_F(BlacklistTest, Beacon) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   // Ensure that the beacon state starts off 'running' for this version.
   LONG result = blacklist_registry_key_->WriteValue(
       blacklist::kBeaconState, blacklist::BLACKLIST_SETUP_RUNNING);
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   result = blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
                                                TEXT(CHROME_VERSION_STRING));
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   // First call should find the beacon and reset it.
   EXPECT_TRUE(blacklist::ResetBeacon());
 
   // First call should succeed as the beacon is enabled.
   EXPECT_TRUE(blacklist::LeaveSetupBeacon());
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 TEST_F(BlacklistTest, AddAndRemoveModules) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   EXPECT_TRUE(TestDll_AddDllToBlacklist(L"foo.dll"));
   // Adding the same item twice should be idempotent.
   EXPECT_TRUE(TestDll_AddDllToBlacklist(L"foo.dll"));
   EXPECT_TRUE(TestDll_RemoveDllFromBlacklist(L"foo.dll"));
   EXPECT_FALSE(TestDll_RemoveDllFromBlacklist(L"foo.dll"));
 
   // Increase the blacklist size by 1 to include the NULL pointer
   // that marks the end.
   int empty_spaces =
       blacklist::kTroublesomeDllsMaxCount - (TestDll_BlacklistSize() + 1);
   std::vector<base::string16> added_dlls;
   added_dlls.reserve(empty_spaces);
   for (int i = 0; i < empty_spaces; ++i) {
     added_dlls.push_back(base::IntToString16(i) + L".dll");
     EXPECT_TRUE(TestDll_AddDllToBlacklist(added_dlls[i].c_str())) << i;
   }
   EXPECT_FALSE(TestDll_AddDllToBlacklist(L"overflow.dll"));
   for (int i = 0; i < empty_spaces; ++i) {
     EXPECT_TRUE(TestDll_RemoveDllFromBlacklist(added_dlls[i].c_str())) << i;
   }
   EXPECT_FALSE(TestDll_RemoveDllFromBlacklist(added_dlls[0].c_str()));
   EXPECT_FALSE(
       TestDll_RemoveDllFromBlacklist(added_dlls[empty_spaces - 1].c_str()));
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 TEST_F(BlacklistTest, SuccessfullyBlocked) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   // Add 5 news dlls to blacklist.
   const int kDesiredBlacklistSize = 1;
   std::vector<base::string16> dlls_to_block;
   for (int i = 0; i < kDesiredBlacklistSize; ++i) {
     dlls_to_block.push_back(base::IntToString16(i) + L".dll");
-    ASSERT_TRUE(TestDll_AddDllToBlacklist(dlls_to_block[i].c_str()));
+    EXPECT_TRUE(TestDll_AddDllToBlacklist(dlls_to_block[i].c_str()));
   }
 
   // Block the dlls, one at a time, and ensure SuccesfullyBlocked correctly
   // passes the list of blocked dlls.
   for (int i = 0; i < kDesiredBlacklistSize; ++i) {
     TestDll_BlockedDll(TestDll_GetBlacklistIndex(dlls_to_block[i].c_str()));
 
     int size = 0;
     TestDll_SuccessfullyBlocked(NULL, &size);
-    ASSERT_EQ(num_initially_blocked_ + i + 1, size);
+    EXPECT_EQ(num_initially_blocked_ + i + 1, size);
 
     std::vector<const wchar_t*> blocked_dlls(size);
     TestDll_SuccessfullyBlocked(&(blocked_dlls[0]), &size);
-    ASSERT_EQ(num_initially_blocked_ + i + 1, size);
+    EXPECT_EQ(num_initially_blocked_ + i + 1, size);
 
     for (int j = 0; j <= i; ++j) {
       EXPECT_STREQ(blocked_dlls[num_initially_blocked_ + j],
                    dlls_to_block[j].c_str());
     }
   }
 
   // Remove the dlls from the blacklist now that we are done.
   for (const auto& dll : dlls_to_block) {
     EXPECT_TRUE(TestDll_RemoveDllFromBlacklist(dll.c_str()));
   }
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
-// Disabled due to flakiness.  https://crbug.com/711651.
-TEST_F(BlacklistTest, DISABLED_LoadBlacklistedLibrary) {
+TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   base::FilePath current_dir;
-  ASSERT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
+  EXPECT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
 
   // Ensure that the blacklist is loaded.
-  ASSERT_TRUE(TestDll_IsBlacklistInitialized());
+  EXPECT_TRUE(TestDll_IsBlacklistInitialized());
 
   // Test that an un-blacklisted DLL can load correctly.
   base::ScopedNativeLibrary dll1(current_dir.Append(kTestDllName1));
   EXPECT_TRUE(dll1.is_valid());
   dll1.Reset(NULL);
 
   int num_blocked_dlls = 0;
   TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
   EXPECT_EQ(num_initially_blocked_, num_blocked_dlls);
 
   // Add all DLLs to the blacklist then check they are blocked.
   for (size_t i = 0; i < arraysize(test_data); ++i) {
     EXPECT_TRUE(TestDll_AddDllToBlacklist(test_data[i].dll_name));
   }
   CheckBlacklistedDllsNotLoaded();
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 void TestResetBeacon(std::unique_ptr<base::win::RegKey>& key,
                      DWORD input_state,
                      DWORD expected_output_state) {
   LONG result = key->WriteValue(blacklist::kBeaconState, input_state);
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   EXPECT_TRUE(blacklist::ResetBeacon());
   DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
   result = key->ReadValueDW(blacklist::kBeaconState, &blacklist_state);
   EXPECT_EQ(ERROR_SUCCESS, result);
   EXPECT_EQ(expected_output_state, blacklist_state);
 }
 
 TEST_F(BlacklistTest, ResetBeacon) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   // Ensure that ResetBeacon resets properly on successful runs and not on
   // failed or disabled runs.
   TestResetBeacon(blacklist_registry_key_,
                   blacklist::BLACKLIST_SETUP_RUNNING,
                   blacklist::BLACKLIST_ENABLED);
 
   TestResetBeacon(blacklist_registry_key_,
                   blacklist::BLACKLIST_SETUP_FAILED,
                   blacklist::BLACKLIST_SETUP_FAILED);
 
   TestResetBeacon(blacklist_registry_key_,
                   blacklist::BLACKLIST_DISABLED,
                   blacklist::BLACKLIST_DISABLED);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 TEST_F(BlacklistTest, SetupFailed) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   // Ensure that when the number of failed tries reaches the maximum allowed,
   // the blacklist state is set to failed.
   LONG result = blacklist_registry_key_->WriteValue(
       blacklist::kBeaconState, blacklist::BLACKLIST_SETUP_RUNNING);
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   // Set the attempt count so that on the next failure the blacklist is
   // disabled.
   result = blacklist_registry_key_->WriteValue(
       blacklist::kBeaconAttemptCount, blacklist::kBeaconMaxAttempts - 1);
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   EXPECT_FALSE(blacklist::LeaveSetupBeacon());
 
   DWORD attempt_count = 0;
   blacklist_registry_key_->ReadValueDW(blacklist::kBeaconAttemptCount,
                                        &attempt_count);
   EXPECT_EQ(attempt_count, blacklist::kBeaconMaxAttempts);
 
   DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
   result = blacklist_registry_key_->ReadValueDW(blacklist::kBeaconState,
                                                 &blacklist_state);
   EXPECT_EQ(ERROR_SUCCESS, result);
   EXPECT_EQ(blacklist_state, blacklist::BLACKLIST_SETUP_FAILED);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 TEST_F(BlacklistTest, SetupSucceeded) {
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, g_blacklist_test_mutex);
+  EXPECT_TRUE(mutex != NULL);
+  DWORD timeout = ::IsDebuggerPresent() ? INFINITE : event_timeout;
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, timeout));
+
   // Starting with the enabled beacon should result in the setup running state
   // and the attempt counter reset to zero.
   LONG result = blacklist_registry_key_->WriteValue(
       blacklist::kBeaconState, blacklist::BLACKLIST_ENABLED);
   EXPECT_EQ(ERROR_SUCCESS, result);
   result = blacklist_registry_key_->WriteValue(blacklist::kBeaconAttemptCount,
                                                blacklist::kBeaconMaxAttempts);
   EXPECT_EQ(ERROR_SUCCESS, result);
 
   EXPECT_TRUE(blacklist::LeaveSetupBeacon());
 
   DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
   blacklist_registry_key_->ReadValueDW(blacklist::kBeaconState,
                                        &blacklist_state);
   EXPECT_EQ(blacklist_state, blacklist::BLACKLIST_SETUP_RUNNING);
 
   DWORD attempt_count = blacklist::kBeaconMaxAttempts;
   blacklist_registry_key_->ReadValueDW(blacklist::kBeaconAttemptCount,
                                        &attempt_count);
   EXPECT_EQ(static_cast<DWORD>(0), attempt_count);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
 }
 
 }  // namespace