[Doodle] Don't create invalid DoodleImages or DoodleConfigs

components/doodle/doodle_fetcher_impl_unittest.cc

Index: components/doodle/doodle_fetcher_impl_unittest.cc
diff --git a/components/doodle/doodle_fetcher_impl_unittest.cc b/components/doodle/doodle_fetcher_impl_unittest.cc
index 939b7b82bd61ac49a38e3a752650bb3e932d07fe..b29e4ee35bcadd99fff7db84dede14bd8c948003 100644
--- a/components/doodle/doodle_fetcher_impl_unittest.cc
+++ b/components/doodle/doodle_fetcher_impl_unittest.cc
@@ -347,15 +347,6 @@ TEST_F(DoodleFetcherImplTest, ResponseContainsExactlyTheSampleImages) {
   ASSERT_TRUE(response.has_value());
   DoodleConfig config = response.value();
 
-  EXPECT_TRUE(config.transparent_large_image.url.is_valid());
-  EXPECT_THAT(config.transparent_large_image.url.spec(),
-              Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
-                         "8251-thp.png")));
-  EXPECT_THAT(config.transparent_large_image.width, Eq(510));
-  EXPECT_THAT(config.transparent_large_image.height, Eq(225));
-  EXPECT_FALSE(config.transparent_large_image.is_animated_gif);
-  EXPECT_FALSE(config.transparent_large_image.is_cta);
-
   EXPECT_TRUE(config.large_image.url.is_valid());
   EXPECT_THAT(config.large_image.url.spec(),
               Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
@@ -365,14 +356,25 @@ TEST_F(DoodleFetcherImplTest, ResponseContainsExactlyTheSampleImages) {
   EXPECT_TRUE(config.large_image.is_animated_gif);
   EXPECT_FALSE(config.large_image.is_cta);
 
-  EXPECT_TRUE(config.large_cta_image.url.is_valid());
-  EXPECT_THAT(config.large_cta_image.url.spec(),
+  ASSERT_TRUE(config.transparent_large_image.has_value());

[sfiera, 2017/03/13 10:29:49]

Shouldn't there be some tests in this file that deal with
!config.transparent_large_image.has_value()?

[Marc Treib, 2017/03/13 10:50:33]

I'd argue that most of this doesn't really belong here in the first place :) 
That said, there are some tests above that don't set the optional image fields,
but they don't check their absence either. I've added some checks there.

Also, I have another follow-up almost ready that adds dedicated tests for the
parsing.

+  EXPECT_TRUE(config.transparent_large_image->url.is_valid());
+  EXPECT_THAT(config.transparent_large_image->url.spec(),
+              Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
+                         "8251-thp.png")));
+  EXPECT_THAT(config.transparent_large_image->width, Eq(510));
+  EXPECT_THAT(config.transparent_large_image->height, Eq(225));
+  EXPECT_FALSE(config.transparent_large_image->is_animated_gif);
+  EXPECT_FALSE(config.transparent_large_image->is_cta);
+
+  ASSERT_TRUE(config.large_cta_image.has_value());
+  EXPECT_TRUE(config.large_cta_image->url.is_valid());
+  EXPECT_THAT(config.large_cta_image->url.spec(),
               Eq(Resolve("/logos/doodles/2015/new-years-eve-2015-5985438795"
                          "8251-cta.gif")));
-  EXPECT_THAT(config.large_cta_image.width, Eq(489));
-  EXPECT_THAT(config.large_cta_image.height, Eq(225));
-  EXPECT_TRUE(config.large_cta_image.is_animated_gif);
-  EXPECT_TRUE(config.large_cta_image.is_cta);
+  EXPECT_THAT(config.large_cta_image->width, Eq(489));
+  EXPECT_THAT(config.large_cta_image->height, Eq(225));
+  EXPECT_TRUE(config.large_cta_image->is_animated_gif);
+  EXPECT_TRUE(config.large_cta_image->is_cta);
 }
 
 TEST_F(DoodleFetcherImplTest, RespondsToMultipleRequestsWithSameFetcher) {