Expand the functionality of the BudgetDatabase.

chrome/browser/budget_service/budget_database_unittest.cc

Index: chrome/browser/budget_service/budget_database_unittest.cc
diff --git a/chrome/browser/budget_service/budget_database_unittest.cc b/chrome/browser/budget_service/budget_database_unittest.cc
index 37d5e34a8a2aaedd86faf9aea26a8fca2ee2c50f..a722db672e034b0f2f52f865aafd3f9643fc0b56 100644
--- a/chrome/browser/budget_service/budget_database_unittest.cc
+++ b/chrome/browser/budget_service/budget_database_unittest.cc
@@ -42,11 +42,11 @@ class BudgetDatabaseTest : public ::testing::Test {
 
     // Create two chunks and give them default values.
     budget_service::BudgetChunk* budget_chunk = budget.add_budget();
-    budget_chunk->set_budget_amount(kDefaultBudget1);
-    budget_chunk->set_expiration_timestamp(kDefaultExpiration);
+    budget_chunk->set_amount(kDefaultBudget1);
+    budget_chunk->set_expiration(kDefaultExpiration);
     budget_chunk = budget.add_budget();
-    budget_chunk->set_budget_amount(kDefaultBudget2);
-    budget_chunk->set_expiration_timestamp(kDefaultExpiration + 1);
+    budget_chunk->set_amount(kDefaultBudget2);
+    budget_chunk->set_expiration(kDefaultExpiration + 1);
 
     base::RunLoop run_loop;
     db_.SetValue(GURL(kTestOrigin), budget,
@@ -75,7 +75,31 @@ class BudgetDatabaseTest : public ::testing::Test {
     return std::move(budget_);
   }
 
+  void GetBudgetDetailsComplete(
+      base::Closure run_loop_closure,
+      bool success,
+      double debt,
+      const BudgetDatabase::BudgetExpectation& expectation) {
+    success_ = success;
+    debt_ = debt;
+    expectation_ = expectation;
+    run_loop_closure.Run();
+  }
+
+  void GetBudgetDetails() {
+    base::RunLoop run_loop;
+    db_.GetBudgetDetails(
+        GURL(kTestOrigin),
+        base::Bind(&BudgetDatabaseTest::GetBudgetDetailsComplete,
+                   base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+    return;

[Peter Beverloo, 2016/07/26 17:26:21]

nit: drop (control flow ends after this anyway)

[harkness, 2016/07/27 10:59:03]

Done.

+  }
+
   Profile* profile() { return &profile_; }
+  const BudgetDatabase::BudgetExpectation& expectation() {
+    return expectation_;
+  }
 
  protected:
   bool success_;
@@ -85,6 +109,8 @@ class BudgetDatabaseTest : public ::testing::Test {
   std::unique_ptr<budget_service::Budget> budget_;
   TestingProfile profile_;
   BudgetDatabase db_;
+  double debt_;

[Peter Beverloo, 2016/07/26 17:26:21]

init (double debt_ = 0;)

[harkness, 2016/07/27 10:59:03]

Done.

+  BudgetDatabase::BudgetExpectation expectation_;
 };
 
 TEST_F(BudgetDatabaseTest, ReadAndWriteTest) {
@@ -93,9 +119,38 @@ TEST_F(BudgetDatabaseTest, ReadAndWriteTest) {
 
   ASSERT_TRUE(success_);
   EXPECT_EQ(kDefaultDebt, b->debt());
-  EXPECT_EQ(kDefaultBudget1, b->budget(0).budget_amount());
-  EXPECT_EQ(kDefaultBudget2, b->budget(1).budget_amount());
-  EXPECT_EQ(kDefaultExpiration, b->budget(0).expiration_timestamp());
+  EXPECT_EQ(kDefaultBudget1, b->budget(0).amount());
+  EXPECT_EQ(kDefaultBudget2, b->budget(1).amount());
+  EXPECT_EQ(kDefaultExpiration, b->budget(0).expiration());
+}
+
+TEST_F(BudgetDatabaseTest, BudgetDetailsTest) {
+  ASSERT_TRUE(SetBudgetWithDefaultValues());
+  GetBudgetDetails();
+
+  // Get the expectation and validate it.
+  const BudgetDatabase::BudgetExpectation exp = expectation();

[Peter Beverloo, 2016/07/26 17:26:21]

nit 2: perfectly fine to use const auto& here. Try to avoid shorter names such
as "exp" where possible.

[Peter Beverloo, 2016/07/26 17:26:21]

nit 1: expectation() returns a reference, so no need to make a copy.

[harkness, 2016/07/27 10:59:03]

Done.

[harkness, 2016/07/27 10:59:03]

Done.

+  ASSERT_TRUE(success_);
+
+  // Make sure that the correct data is returned.
+  BudgetDatabase::BudgetExpectation::const_iterator iter = exp.begin();

[Peter Beverloo, 2016/07/26 17:26:21]

nit: const auto& iter = ...?

[harkness, 2016/07/27 10:59:03]

I did some reading on const_iterator and auto, and it doesn't seem like there's
a good way to do this. Let's talk about it in person.

[Peter Beverloo, 2016/07/27 11:21:40]

Ugh ok. Could still use `auto` (const-ness doesn't really matter here), or leave
as-is. Up to you.

+  ASSERT_EQ(exp.size(), (const unsigned long)3);

[Peter Beverloo, 2016/07/26 17:26:21]

nit 1: This ASSERT should be under line 133, it's unrelated to line 136.

[Peter Beverloo, 2016/07/26 17:26:21]

nit 3: We disallow c-style casts in Chromium. In this case you can use the "U"
suffix:  3U

[Peter Beverloo, 2016/07/26 17:26:21]

nit 2: Canonical use of the {ASSERT,EXPECT}_EQ macros is (expected, actual),
which affects the error messages GTest prints. Dito further down.

[harkness, 2016/07/27 10:59:03]

Done.

[harkness, 2016/07/27 10:59:03]

Done.

[harkness, 2016/07/27 10:59:03]

Done.

+
+  // First value should be [total_budget, now]
+  EXPECT_EQ(iter->first, kDefaultBudget1 + kDefaultBudget2);
+  // TODO(harkness): This will be "now" in the final version. For now, it's
+  // just 0.0.
+  EXPECT_EQ(iter->second, 0.0);

[Peter Beverloo, 2016/07/26 17:26:21]

nit: s/0.0/0/ (dito on line 152)

[harkness, 2016/07/27 10:59:03]

Done.

+
+  // The next value should be the budget after the first chunk expires.
+  iter++;
+  EXPECT_EQ(iter->first, kDefaultBudget2);
+  EXPECT_EQ(iter->second, kDefaultExpiration);
+
+  // The final value gives the budget of 0.0 after the second chunk expires.
+  iter++;
+  EXPECT_EQ(iter->first, 0.0);
+  EXPECT_EQ(iter->second, kDefaultExpiration + 1);
 }
 
 }  // namespace