PM: Changes in unit tests.

* Adds macros for checking null-ness of pointers in tests. This makes it
  safer and less error-prone: gtest seems to be inconsistent in its
  treatment of pointer values, so having our own wrappers and making
  them perform uniformly is good.

* Regulates the use of ASSERT vs EXPECT in unit tests: the former must
  be used if subsequent test logic depends on the condition tested (for
  example, a pointer being dereferenced later is not null, or a provider
  module initialized correctly). The latter will fail the test but would
  allow it to keep executing (aka non-critical failure).

* General revision of existing unit test code. Minor changes in
  random_provider.h.

BUG=None
TEST=Builds and passes unit tests.

Change-Id: I2ada0fbd96db4cb0c0d631c2e350873853116fc2
Reviewed-on: https://chromium-review.googlesource.com/184449
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/policy_manager/generic_variables_unittest.cc b/policy_manager/generic_variables_unittest.cc
index 15ea3d7..e14dc50 100644
--- a/policy_manager/generic_variables_unittest.cc
+++ b/policy_manager/generic_variables_unittest.cc
@@ -2,61 +2,70 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <base/memory/scoped_ptr.h>
 #include <gtest/gtest.h>
+
 #include "policy_manager/generic_variables.h"
+#include "policy_manager/pmtest_utils.h"
 
 using base::TimeDelta;
-using std::string;
 
 namespace chromeos_policy_manager {
 
-TEST(PmCopyVariableTest, SimpleTest) {
-  int obj_int = 5;
+class PmCopyVariableTest : public ::testing::Test {
+ protected:
+  virtual void SetUp() {
+    default_timeout_ = TimeDelta::FromSeconds(1);
+  }
 
-  CopyVariable<int> var("var", obj_int);
+  TimeDelta default_timeout_;
+};
 
-  string errmsg = "Nope";
 
-  const int* res_1 = var.GetValue(TimeDelta::FromSeconds(1), &errmsg);
-  EXPECT_NE(res_1, static_cast<void*>(NULL));
-  EXPECT_EQ(5, *res_1);
+TEST_F(PmCopyVariableTest, SimpleTest) {
+  // Tests that copies are generated as intended.
+  int source = 5;
+  CopyVariable<int> var("var", source);
 
-  obj_int = 42;
+  // Generate and validate a copy.
+  scoped_ptr<const int> copy_1(var.GetValue(default_timeout_, NULL));
+  PMTEST_ASSERT_NOT_NULL(copy_1.get());
+  EXPECT_EQ(5, *copy_1);
 
-  // Check the result in res_1 is actually a new copy.
-  EXPECT_EQ(5, *res_1);
+  // Assign a different value to the source variable.
+  source = 42;
 
-  const int* res_2 = var.GetValue(TimeDelta::FromSeconds(1), &errmsg);
-  EXPECT_NE(res_2, static_cast<void*>(NULL));
-  EXPECT_EQ(42, *res_2);
+  // Check that the content of the copy was not affected (distinct instance).
+  EXPECT_EQ(5, *copy_1);
 
-  delete res_1;
-  delete res_2;
+  // Generate and validate a second copy.
+  scoped_ptr<const int> copy_2(var.GetValue(default_timeout_, NULL));
+  PMTEST_ASSERT_NOT_NULL(copy_2.get());
+  EXPECT_EQ(42, *copy_2);
 }
 
-class ConstructorTestClass {
- public:
-  ConstructorTestClass(void) : copied_(false) {}
 
-  ConstructorTestClass(const ConstructorTestClass& /* ignored */)
+class CopyConstructorTestClass {
+ public:
+  CopyConstructorTestClass(void) : copied_(false) {}
+  CopyConstructorTestClass(const CopyConstructorTestClass& /* ignored */)
       : copied_(true) {}
 
   // Tells if the instance was constructed using the copy-constructor.
   bool copied_;
 };
 
-TEST(PmCopyVariableTest, UseCopyConstructorTest) {
-  ConstructorTestClass obj;
-  ASSERT_FALSE(obj.copied_);
 
-  string errmsg;
-  CopyVariable<ConstructorTestClass> var("var", obj);
-  const ConstructorTestClass* value =
-      var.GetValue(TimeDelta::FromSeconds(1), &errmsg);
-  EXPECT_NE(value, static_cast<void*>(NULL));
-  EXPECT_TRUE(value->copied_);
+TEST_F(PmCopyVariableTest, UseCopyConstructorTest) {
+  // Ensures that CopyVariables indeed uses the copy contructor.
+  const CopyConstructorTestClass source;
+  ASSERT_FALSE(source.copied_);
 
-  delete value;
+  CopyVariable<CopyConstructorTestClass> var("var", source);
+  scoped_ptr<const CopyConstructorTestClass> copy(
+      var.GetValue(default_timeout_, NULL));
+  PMTEST_ASSERT_NOT_NULL(copy.get());
+  EXPECT_TRUE(copy->copied_);
 }
 
 }  // namespace chromeos_policy_manager