Add tests for TimeRemainingLong() and TimeDurationLong()

ui/base/l10n/time_format_unittest.cc

 // Copyright (c) 2011 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 "ui/base/l10n/time_format.h"
 
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/resource/resource_bundle.h"
 
 using base::ASCIIToUTF16;
 
 namespace ui {
 namespace {
 
 using base::TimeDelta;
 
-void TestTimeFormats(const TimeDelta& delta, const char* expected_ascii) {
-  base::string16 expected = ASCIIToUTF16(expected_ascii);
-  base::string16 expected_left = expected + ASCIIToUTF16(" left");
-  base::string16 expected_ago = expected + ASCIIToUTF16(" ago");
-  EXPECT_EQ(expected, TimeFormat::TimeRemainingShort(delta));
-  EXPECT_EQ(expected_left, TimeFormat::TimeRemaining(delta));
-  EXPECT_EQ(expected_ago, TimeFormat::TimeElapsed(delta));
+void TestTimeFormats(const TimeDelta& delta, const char* expect_ascii) {
+  const base::string16 expect(ASCIIToUTF16(expect_ascii));

[bartfab (slow), 2014/01/29 15:46:16]

Nit: s/expect/expected/

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  const base::string16 sec   (ASCIIToUTF16("sec"));

[bartfab (slow), 2014/01/29 15:46:16]

* Do not align with spaces.

* Please add one_ prefixes to sec and min

[Thiemo Nagel, 2014/01/29 16:18:05]

Done, except "one_" prefixes.

+  const base::string16 min   (ASCIIToUTF16("min"));
+  const base::string16 ond   (ASCIIToUTF16("ond"));

[bartfab (slow), 2014/01/29 15:46:16]

"ond" and "ute" are not exactly self-explanatory. Does it really make sense to
define them here as constants? Why not create the string16s on the fly where you
need them?

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  const base::string16 ute   (ASCIIToUTF16("ute"));
+  const base::string16 _ago  (ASCIIToUTF16(" ago"));

[bartfab (slow), 2014/01/29 15:46:16]

Do not start variable names with _. I know it stands for a space here but it
looks like something that implies a significant meaning. Just call the variables
"ago" and "left" or, better yet, create the string16s on the fly where you need
them.

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  const base::string16 _left (ASCIIToUTF16(" left"));
+
+  base::string16 expect_long        = expect;   // sec, min --> second, minute

[bartfab (slow), 2014/01/29 15:46:16]

Do not align with spaces.

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  base::string16 expect_long_minute = expect;   // min --> minute
+
+  if (expect_long.find(sec) != base::string16::npos) {
+    expect_long.insert(expect_long.find(sec)+sec.length(), ond);

[bartfab (slow), 2014/01/29 15:46:16]

Here and further down: Spaces around the + are missing.

Nit: No {} for single-line conditionals.

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  }
+  if (expect_long.find(min) != base::string16::npos) {
+    expect_long.insert(expect_long.find(min)+min.length(), ute);
+  }
+  if (expect_long_minute.find(min) != base::string16::npos) {
+    expect_long_minute.insert(expect_long_minute.find(min)+min.length(), ute);
+  }
+
+  const base::string16 expect_elapsed         = expect + _ago;

[bartfab (slow), 2014/01/29 15:46:16]

Here and throughout the rest of the CL: Do not align with spaces.

[Thiemo Nagel, 2014/01/29 16:18:05]

Done.

+  const base::string16 expect_remaining       = expect + _left;
+  const base::string16 expect_remaining_long  = expect_long_minute + _left;
+  const base::string16 expect_remaining_short = expect;
+  const base::string16 expect_duration_long   = expect_long;
+
+  EXPECT_EQ(expect_elapsed,         TimeFormat::TimeElapsed(delta));
+  EXPECT_EQ(expect_remaining,       TimeFormat::TimeRemaining(delta));
+  EXPECT_EQ(expect_remaining_long,  TimeFormat::TimeRemainingLong(delta));
+  EXPECT_EQ(expect_remaining_short, TimeFormat::TimeRemainingShort(delta));
+  EXPECT_EQ(expect_duration_long,   TimeFormat::TimeDurationLong(delta));
 }
 
 TEST(TimeFormat, FormatTime) {
   const TimeDelta one_day = TimeDelta::FromDays(1);
   const TimeDelta three_days = TimeDelta::FromDays(3);
   const TimeDelta one_hour = TimeDelta::FromHours(1);
   const TimeDelta four_hours = TimeDelta::FromHours(4);
   const TimeDelta one_min = TimeDelta::FromMinutes(1);
   const TimeDelta three_mins = TimeDelta::FromMinutes(3);
   const TimeDelta one_sec = TimeDelta::FromSeconds(1);
@@ skipping 30 matching lines @@
       TimeFormat::RelativeDate(two_days_ago, NULL);
   EXPECT_TRUE(two_days_ago_str.empty());
 
   base::Time a_week_ago = now - TimeDelta::FromDays(7);
   base::string16 a_week_ago_str = TimeFormat::RelativeDate(a_week_ago, NULL);
   EXPECT_TRUE(a_week_ago_str.empty());
 }
 
 }  // namespace
 }  // namespace ui