Reading List iOS: Use external clock in ReadingListEntry.

components/reading_list/ios/reading_list_store_unittest.cc

 // Copyright 2016 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 "components/reading_list/ios/reading_list_store.h"
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#import "base/test/ios/wait_util.h"
+#include "base/test/simple_test_clock.h"
 #include "components/reading_list/ios/reading_list_model_impl.h"
 #include "components/sync/model/fake_model_type_change_processor.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
 // Tests that the transition from |entryA| to |entryB| is possible (|possible|
 // is true) or not.
 void ExpectAB(const sync_pb::ReadingListSpecifics& entryA,
               const sync_pb::ReadingListSpecifics& entryB,
               bool possible) {
   EXPECT_EQ(ReadingListStore::CompareEntriesForSync(entryA, entryB), possible);
   std::unique_ptr<ReadingListEntry> a =
-      ReadingListEntry::FromReadingListSpecifics(entryA);
+      ReadingListEntry::FromReadingListSpecifics(entryA,
+                                                 base::Time::FromTimeT(10));
   std::unique_ptr<ReadingListEntry> b =
-      ReadingListEntry::FromReadingListSpecifics(entryB);
+      ReadingListEntry::FromReadingListSpecifics(entryB,
+                                                 base::Time::FromTimeT(10));
   a->MergeWithEntry(*b);
   std::unique_ptr<sync_pb::ReadingListSpecifics> mergedEntry =
       a->AsReadingListSpecifics();
   if (possible) {
     // If transition is possible, the merge should be B.
     EXPECT_EQ(entryB.SerializeAsString(), mergedEntry->SerializeAsString());
   } else {
     // If transition is not possible, the transition shold be possible to the
     // merged state.
     EXPECT_TRUE(ReadingListStore::CompareEntriesForSync(entryA, *mergedEntry));
     EXPECT_TRUE(ReadingListStore::CompareEntriesForSync(entryB, *mergedEntry));
   }
 }

[gambard, 2017/03/21 13:08:07]

Add new line.

[Olivier, 2017/03/21 14:01:10]

Done.

+base::Time AdvanceAndGetTime(base::SimpleTestClock* clock) {
+  clock->Advance(base::TimeDelta::FromMilliseconds(10));
+  return clock->Now();
+}
+
+}  // namespace
 
 class FakeModelTypeChangeProcessorObserver {
  public:
   virtual void Put(const std::string& client_tag,
                    std::unique_ptr<syncer::EntityData> entity_data,
                    syncer::MetadataChangeList* metadata_change_list) = 0;
 
   virtual void Delete(const std::string& client_tag,
                       syncer::MetadataChangeList* metadata_change_list) = 0;
 };
@@ skipping 25 matching lines @@
                              public ReadingListStoreDelegate {
  protected:
   ReadingListStoreTest()
       : store_(syncer::ModelTypeStoreTestUtil::CreateInMemoryStoreForTest()) {
     ClearState();
     reading_list_store_ = base::MakeUnique<ReadingListStore>(
         base::Bind(&syncer::ModelTypeStoreTestUtil::MoveStoreToCallback,
                    base::Passed(&store_)),
         base::Bind(&ReadingListStoreTest::CreateModelTypeChangeProcessor,
                    base::Unretained(this)));
-    model_ = base::MakeUnique<ReadingListModelImpl>(nullptr, nullptr);
-    reading_list_store_->SetReadingListModel(model_.get(), this);
+    auto clock = base::MakeUnique<base::SimpleTestClock>();
+    clock_ = clock.get();
+    model_ = base::MakeUnique<ReadingListModelImpl>(nullptr, nullptr,
+                                                    std::move(clock));
+    reading_list_store_->SetReadingListModel(model_.get(), this, clock_);
 
     base::RunLoop().RunUntilIdle();

[gambard, 2017/03/21 13:08:07]

What is this used for?

   }
 
   std::unique_ptr<syncer::ModelTypeChangeProcessor>
   CreateModelTypeChangeProcessor(syncer::ModelType type,
                                  syncer::ModelTypeSyncBridge* service) {
     auto processor = base::MakeUnique<TestModelTypeChangeProcessor>();
     processor->SetObserver(this);
     return std::move(processor);
   }
 
@@ skipping 54 matching lines @@
     sync_merge_called_++;
     sync_merged_[entry->URL().spec()] = entry->IsRead();
     return model_->SyncMergeEntry(std::move(entry));
   }
 
   // In memory model type store needs a MessageLoop.
   base::MessageLoop message_loop_;
 
   std::unique_ptr<syncer::ModelTypeStore> store_;
   std::unique_ptr<ReadingListModelImpl> model_;
+  base::SimpleTestClock* clock_;
   std::unique_ptr<ReadingListStore> reading_list_store_;
   int put_called_;
   int delete_called_;
   int sync_add_called_;
   int sync_remove_called_;
   int sync_merge_called_;
   std::map<std::string, std::unique_ptr<syncer::EntityData>> put_multimap_;
   std::set<std::string> delete_set_;
   std::map<std::string, bool> sync_added_;
   std::set<std::string> sync_removed_;
   std::map<std::string, bool> sync_merged_;
 };
 
 TEST_F(ReadingListStoreTest, CheckEmpties) {
   EXPECT_EQ(0ul, model_->size());
 }
 
 TEST_F(ReadingListStoreTest, SaveOneRead) {
-  ReadingListEntry entry(GURL("http://read.example.com/"), "read title");
-  entry.SetRead(true);
+  ReadingListEntry entry(GURL("http://read.example.com/"), "read title",
+                         AdvanceAndGetTime(clock_));
+  entry.SetRead(true, AdvanceAndGetTime(clock_));
+  AdvanceAndGetTime(clock_);
   reading_list_store_->SaveEntry(entry);
   AssertCounts(1, 0, 0, 0, 0);
   syncer::EntityData* data = put_multimap_["http://read.example.com/"].get();
   const sync_pb::ReadingListSpecifics& specifics =
       data->specifics.reading_list();
   EXPECT_EQ(specifics.title(), "read title");

[gambard, 2017/03/21 13:08:07]

We are not testing for all update times?

[Olivier, 2017/03/21 14:01:10]

This is tested in ReadingListEntry unittests.

[gambard, 2017/03/21 15:36:53]

Acknowledged.

   EXPECT_EQ(specifics.url(), "http://read.example.com/");
   EXPECT_EQ(specifics.status(), sync_pb::ReadingListSpecifics::READ);
 }
 
 TEST_F(ReadingListStoreTest, SaveOneUnread) {
-  ReadingListEntry entry(GURL("http://unread.example.com/"), "unread title");
+  ReadingListEntry entry(GURL("http://unread.example.com/"), "unread title",
+                         AdvanceAndGetTime(clock_));
   reading_list_store_->SaveEntry(entry);
   AssertCounts(1, 0, 0, 0, 0);
   syncer::EntityData* data = put_multimap_["http://unread.example.com/"].get();
   const sync_pb::ReadingListSpecifics& specifics =
       data->specifics.reading_list();
   EXPECT_EQ(specifics.title(), "unread title");
   EXPECT_EQ(specifics.url(), "http://unread.example.com/");
   EXPECT_EQ(specifics.status(), sync_pb::ReadingListSpecifics::UNSEEN);
 }
 
 TEST_F(ReadingListStoreTest, SyncMergeOneEntry) {
   syncer::EntityDataMap remote_input;
-  ReadingListEntry entry(GURL("http://read.example.com/"), "read title");
-  entry.SetRead(true);
+  ReadingListEntry entry(GURL("http://read.example.com/"), "read title",
+                         AdvanceAndGetTime(clock_));
+  entry.SetRead(true, AdvanceAndGetTime(clock_));
   std::unique_ptr<sync_pb::ReadingListSpecifics> specifics =
       entry.AsReadingListSpecifics();
 
   syncer::EntityData data;
   data.client_tag_hash = "http://read.example.com/";
   *data.specifics.mutable_reading_list() = *specifics;
 
   remote_input["http://read.example.com/"] = data.PassToPtr();
 
   std::unique_ptr<syncer::MetadataChangeList> metadata_changes(
       reading_list_store_->CreateMetadataChangeList());
   auto error = reading_list_store_->MergeSyncData(std::move(metadata_changes),
                                                   remote_input);
   AssertCounts(0, 0, 1, 0, 0);
   EXPECT_EQ(sync_added_.size(), 1u);
   EXPECT_EQ(sync_added_.count("http://read.example.com/"), 1u);
   EXPECT_EQ(sync_added_["http://read.example.com/"], true);
 }
 
 TEST_F(ReadingListStoreTest, ApplySyncChangesOneAdd) {
   syncer::EntityDataMap remote_input;
-  ReadingListEntry entry(GURL("http://read.example.com/"), "read title");
-  entry.SetRead(true);
+  ReadingListEntry entry(GURL("http://read.example.com/"), "read title",
+                         AdvanceAndGetTime(clock_));
+  entry.SetRead(true, AdvanceAndGetTime(clock_));
   std::unique_ptr<sync_pb::ReadingListSpecifics> specifics =
       entry.AsReadingListSpecifics();
   syncer::EntityData data;
   data.client_tag_hash = "http://read.example.com/";
   *data.specifics.mutable_reading_list() = *specifics;
 
   syncer::EntityChangeList add_changes;
 
   add_changes.push_back(syncer::EntityChange::CreateAdd(
       "http://read.example.com/", data.PassToPtr()));
   auto error = reading_list_store_->ApplySyncChanges(
       reading_list_store_->CreateMetadataChangeList(), add_changes);
   AssertCounts(0, 0, 1, 0, 0);
   EXPECT_EQ(sync_added_.size(), 1u);
   EXPECT_EQ(sync_added_.count("http://read.example.com/"), 1u);
   EXPECT_EQ(sync_added_["http://read.example.com/"], true);
 }
 
 TEST_F(ReadingListStoreTest, ApplySyncChangesOneMerge) {
   syncer::EntityDataMap remote_input;
+  AdvanceAndGetTime(clock_);
   model_->AddEntry(GURL("http://unread.example.com/"), "unread title",
                    reading_list::ADDED_VIA_CURRENT_APP);
-  base::test::ios::SpinRunLoopWithMinDelay(
-      base::TimeDelta::FromMilliseconds(10));
 
-  ReadingListEntry new_entry(GURL("http://unread.example.com/"),
-                             "unread title");
-  new_entry.SetRead(true);
+  ReadingListEntry new_entry(GURL("http://unread.example.com/"), "unread title",
+                             AdvanceAndGetTime(clock_));
+  new_entry.SetRead(true, AdvanceAndGetTime(clock_));
   std::unique_ptr<sync_pb::ReadingListSpecifics> specifics =
       new_entry.AsReadingListSpecifics();
   syncer::EntityData data;
   data.client_tag_hash = "http://unread.example.com/";
   *data.specifics.mutable_reading_list() = *specifics;
 
   syncer::EntityChangeList add_changes;
   add_changes.push_back(syncer::EntityChange::CreateAdd(
       "http://unread.example.com/", data.PassToPtr()));
   auto error = reading_list_store_->ApplySyncChanges(
       reading_list_store_->CreateMetadataChangeList(), add_changes);
   AssertCounts(1, 0, 0, 0, 1);
   EXPECT_EQ(sync_merged_.size(), 1u);
   EXPECT_EQ(sync_merged_.count("http://unread.example.com/"), 1u);
   EXPECT_EQ(sync_merged_["http://unread.example.com/"], true);
 }
 
 TEST_F(ReadingListStoreTest, ApplySyncChangesOneIgnored) {
   // Read entry but with unread URL as it must update the other one.
   ReadingListEntry old_entry(GURL("http://unread.example.com/"),
-                             "old unread title");
-  old_entry.SetRead(true);
+                             "old unread title", AdvanceAndGetTime(clock_));
+  old_entry.SetRead(true, AdvanceAndGetTime(clock_));
 
-  base::test::ios::SpinRunLoopWithMinDelay(
-      base::TimeDelta::FromMilliseconds(10));
   syncer::EntityDataMap remote_input;
+  AdvanceAndGetTime(clock_);
   model_->AddEntry(GURL("http://unread.example.com/"), "new unread title",
                    reading_list::ADDED_VIA_CURRENT_APP);
   AssertCounts(0, 0, 0, 0, 0);
 
   std::unique_ptr<sync_pb::ReadingListSpecifics> specifics =
       old_entry.AsReadingListSpecifics();
   syncer::EntityData data;
   data.client_tag_hash = "http://unread.example.com/";
   *data.specifics.mutable_reading_list() = *specifics;
 
@@ skipping 132 matching lines @@
     entryB.set_status(status_oder[index_a]);
     ExpectAB(entryA, entryB, true);
     ExpectAB(entryB, entryA, true);
     for (int index_b = index_a + 1; index_b < 3; index_b++) {
       entryB.set_status(status_oder[index_b]);
       ExpectAB(entryA, entryB, true);
       ExpectAB(entryB, entryA, false);
     }
   }
 }