[Doodle] Don't create invalid DoodleImages or DoodleConfigs

components/doodle/doodle_fetcher_impl_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/doodle/doodle_fetcher_impl.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 329 matching lines @@
           "is_cta":false,
           "url":"/logos/doodles/2015/new-years-eve-2015-59854387958251-thp.png",
           "width":510
         }
       }})json");
 
   EXPECT_THAT(state, Eq(DoodleState::AVAILABLE));
   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"
                          "8251-hp.gif")));
   EXPECT_THAT(config.large_image.width, Eq(489));
   EXPECT_THAT(config.large_image.height, Eq(225));
   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) {
   DoodleState state1(DoodleState::NO_DOODLE);
   DoodleState state2(DoodleState::NO_DOODLE);
   base::Optional<DoodleConfig> response1;
   base::Optional<DoodleConfig> response2;
 
   // Trigger two requests.
   doodle_fetcher()->FetchDoodle(
@@ skipping 31 matching lines @@
       switches::kGoogleBaseURL, "http://www.google.kz");
 
   doodle_fetcher()->FetchDoodle(CreateResponseSavingCallback(
       /*state=*/nullptr, nullptr, /*response=*/nullptr));
 
   EXPECT_THAT(GetRunningFetcher()->GetOriginalURL(),
               Eq(GURL("http://www.google.kz").Resolve(kDoodleConfigPath)));
 }
 
 }  // namespace doodle