ReadingListProvider handles dismissal

components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc

Index: components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc b/components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc
index 6f91ed2ef8b244c5c9aeea090885910f9dcf47db..ce42afc684f64133158493a1967d10f86d89f8ce 100644
--- a/components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/reading_list/reading_list_suggestions_provider_unittest.cc
@@ -46,11 +46,41 @@ class ReadingListSuggestionsProviderTest : public ::testing::Test {
     return Category::FromKnownCategory(KnownCategories::READING_LIST);
   }
 
+  void AddEntries() {
+    model_->AddEntry(url_unread1_, title_unread1_,
+                     reading_list::ADDED_VIA_CURRENT_APP);
+    clock_->Advance(base::TimeDelta::FromMilliseconds(10));
+    model_->AddEntry(url_unread2_, title_unread2_,
+                     reading_list::ADDED_VIA_CURRENT_APP);
+    clock_->Advance(base::TimeDelta::FromMilliseconds(10));
+    model_->AddEntry(url_read1_, title_read1_,
+                     reading_list::ADDED_VIA_CURRENT_APP);
+    model_->SetReadStatus(url_read1_, true);
+    clock_->Advance(base::TimeDelta::FromMilliseconds(10));
+    model_->AddEntry(url_unread3_, title_unread3_,
+                     reading_list::ADDED_VIA_CURRENT_APP);
+    clock_->Advance(base::TimeDelta::FromMilliseconds(10));
+    model_->AddEntry(url_unread4_, title_unread4_,
+                     reading_list::ADDED_VIA_CURRENT_APP);
+  }
+
  protected:
   base::SimpleTestClock* clock_;
   std::unique_ptr<ReadingListModelImpl> model_;
   testing::StrictMock<MockContentSuggestionsProviderObserver> observer_;
   std::unique_ptr<ReadingListSuggestionsProvider> provider_;
+
+  GURL url_unread1_ = GURL("http://www.foo1.bar");

[Marc Treib, 2017/04/11 12:35:15]

All these should probably be const, and can be initialized like this:
const GURL url_unread1_("http://www.foo1.bar");

Alternatively, it might actually be better to just duplicate those in the tests
that need them. IMO that'd make the tests easier to read, since the "input" and
the expected result are closer together. Up to you though.

[gambard, 2017/04/11 13:29:53]

If I move it to the test I need to move the AddEntries also and it create a lot
a duplication.
I created a constant.

[Marc Treib, 2017/04/11 13:41:08]

In tests, IMO duplication is sometimes better than making the reader jump around
between the actual test and the test class. But again, up to you.

[gambard, 2017/04/11 15:01:18]

I prefer it that way, as the values does not hold any meaning.

[Marc Treib, 2017/04/11 15:28:14]

What does hold a meaning is the times that are attached to the URLs, because
they affect the sorting.

[gambard, 2017/04/12 07:50:43]

Yes.

+  GURL url_unread2_ = GURL("http://www.foo2.bar");
+  GURL url_unread3_ = GURL("http://www.foo3.bar");
+  GURL url_unread4_ = GURL("http://www.foo4.bar");
+  GURL url_read1_ = GURL("http://www.bar.foor");
+
+  std::string title_unread1_ = "title1";
+  std::string title_unread2_ = "title2";
+  std::string title_unread3_ = "title3";
+  std::string title_unread4_ = "title4";
+  std::string title_read1_ = "title_read1";
 };
 
 TEST_F(ReadingListSuggestionsProviderTest, CategoryInfo) {
@@ -64,37 +94,15 @@ TEST_F(ReadingListSuggestionsProviderTest, CategoryInfo) {
 }
 
 TEST_F(ReadingListSuggestionsProviderTest, ReturnsThreeLatestUnreadSuggestion) {
-  GURL url_unread1 = GURL("http://www.foo1.bar");
-  GURL url_unread2 = GURL("http://www.foo2.bar");
-  GURL url_unread3 = GURL("http://www.foo3.bar");
-  GURL url_unread4 = GURL("http://www.foo4.bar");
-  GURL url_read1 = GURL("http://www.bar.foor");
-  std::string title_unread1 = "title1";
-  std::string title_unread2 = "title2";
-  std::string title_unread3 = "title3";
-  std::string title_unread4 = "title4";
-  std::string title_read1 = "title_read1";
-  model_->AddEntry(url_unread1, title_unread1,
-                   reading_list::ADDED_VIA_CURRENT_APP);
-  clock_->Advance(base::TimeDelta::FromMilliseconds(10));
-  model_->AddEntry(url_unread2, title_unread2,
-                   reading_list::ADDED_VIA_CURRENT_APP);
-  clock_->Advance(base::TimeDelta::FromMilliseconds(10));
-  model_->AddEntry(url_read1, title_read1, reading_list::ADDED_VIA_CURRENT_APP);
-  model_->SetReadStatus(url_read1, true);
-  clock_->Advance(base::TimeDelta::FromMilliseconds(10));
-  model_->AddEntry(url_unread3, title_unread3,
-                   reading_list::ADDED_VIA_CURRENT_APP);
-  clock_->Advance(base::TimeDelta::FromMilliseconds(10));
-  model_->AddEntry(url_unread4, title_unread4,
-                   reading_list::ADDED_VIA_CURRENT_APP);
+  AddEntries();
 
-  EXPECT_CALL(observer_,
-              OnNewSuggestions(
-                  _, ReadingListCategory(),
-                  ElementsAre(Property(&ContentSuggestion::url, url_unread4),
-                              Property(&ContentSuggestion::url, url_unread3),
-                              Property(&ContentSuggestion::url, url_unread2))));
+  EXPECT_CALL(
+      observer_,
+      OnNewSuggestions(
+          _, ReadingListCategory(),
+          ElementsAre(Property(&ContentSuggestion::url, url_unread4_),
+                      Property(&ContentSuggestion::url, url_unread3_),
+                      Property(&ContentSuggestion::url, url_unread2_))));
 
   CreateProvider();
 }
@@ -120,6 +128,32 @@ TEST_F(ReadingListSuggestionsProviderTest, ReturnsOnlyUnreadSuggestion) {
   CreateProvider();
 }
 
+TEST_F(ReadingListSuggestionsProviderTest, DismissesEntry) {
+  AddEntries();
+
+  EXPECT_CALL(
+      observer_,
+      OnNewSuggestions(
+          _, ReadingListCategory(),
+          ElementsAre(Property(&ContentSuggestion::url, url_unread4_),
+                      Property(&ContentSuggestion::url, url_unread3_),
+                      Property(&ContentSuggestion::url, url_unread2_))));
+
+  CreateProvider();
+
+  EXPECT_CALL(
+      observer_,
+      OnNewSuggestions(
+          _, ReadingListCategory(),
+          ElementsAre(Property(&ContentSuggestion::url, url_unread4_),
+                      Property(&ContentSuggestion::url, url_unread2_),
+                      Property(&ContentSuggestion::url, url_unread1_))));
+
+  provider_->DismissSuggestion(ContentSuggestion::ID(
+      Category::FromKnownCategory(KnownCategories::READING_LIST),
+      url_unread3_.spec()));
+}
+
 }  // namespace
 
 }  // namespace ntp_snippets