[i18n] components directory to $i18n{}

components/supervised_user_error_page/supervised_user_error_page_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/supervised_user_error_page/supervised_user_error_page.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "components/grit/components_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ skipping 58 matching lines @@
 class SupervisedUserErrorPageTest_BuildHtml
     : public ::testing::TestWithParam<BuildHtmlTestParameter> {};
 
 TEST_P(SupervisedUserErrorPageTest_BuildHtml, BuildHtml) {
   BuildHtmlTestParameter param = GetParam();
   std::string result = BuildHtml(
       param.allow_access_requests, param.profile_image_url,
       param.profile_image_url2, param.custodian, param.custodian_email,
       param.second_custodian, param.second_custodian_email,
       param.is_child_account, param.reason, "");
-  // The result should contain the original HTML plus scripts that plug values
-  // into it. The test can't easily check that the scripts are correct, but
-  // can check that the output contains the expected values.
-  std::string html =
-      ResourceBundle::GetSharedInstance()
-          .GetRawDataResource(IDR_SUPERVISED_USER_BLOCK_INTERSTITIAL_HTML)
-          .as_string();
-  EXPECT_THAT(result, testing::HasSubstr(html));

[dschuyler, 2017/05/31 21:10:53]

This was previously testing that BuildHtml and GetRawDataResource would load the
same IDR_SUPERVISED_USER_BLOCK_INTERSTITIAL_HTML text. That assumption doesn't
hold in the the $i18n{} world, because the i18n is inlined immediately (that's
the power of $i18n{}, i.e. why it exists). I considered what it would mean to
keep this comparison, but it would end up being a second call to BuildHtml()
(i.e. testing that "BuildHtml(...) == BuildHtml(...)" which isn't a valuable
comparison.

[tommycli, 2017/06/12 18:49:20]

Agreed, let's just axe that above EXPECT clause. ^

Thanks for the thorough explanation.

[tommycli, 2017/06/12 18:52:05]

I looked into it some more out of curiosity. Previous test was pretty much
tautological anyways. No loss in removal IMO.

+  // The result should contain the original HTML (with $i18n{} replacements)
+  // plus scripts that plug values into it. The test can't easily check that the
+  // scripts are correct, but can check that the output contains the expected
+  // values.
   EXPECT_THAT(result, testing::HasSubstr(param.profile_image_url));
   EXPECT_THAT(result, testing::HasSubstr(param.profile_image_url2));
   EXPECT_THAT(result, testing::HasSubstr(param.custodian));
   EXPECT_THAT(result, testing::HasSubstr(param.custodian_email));
   if (param.has_two_parents) {
     EXPECT_THAT(result, testing::HasSubstr(param.second_custodian));
     EXPECT_THAT(result, testing::HasSubstr(param.second_custodian_email));
   }
   if (param.reason == ASYNC_CHECKER || param.reason == BLACKLIST) {
     EXPECT_THAT(result, testing::HasSubstr("\"showFeedbackLink\":true"));
@@ skipping 103 matching lines @@
      "custodian2_email", false, DEFAULT, true},
     {true, "url1", "url2", "custodian", "custodian_email", "custodian2",
      "custodian2_email", true, ASYNC_CHECKER, true},
 };
 
 INSTANTIATE_TEST_CASE_P(GetBlockMessageIDParameterized,
                         SupervisedUserErrorPageTest_BuildHtml,
                         ::testing::ValuesIn(build_html_test_parameter));
 
 }  //  namespace supervised_user_error_page