Add tests for TimeRemainingLong() and TimeDurationLong()

ui/base/l10n/time_format_unittest.cc

Index: ui/base/l10n/time_format_unittest.cc
diff --git a/ui/base/l10n/time_format_unittest.cc b/ui/base/l10n/time_format_unittest.cc
index 6f6acc42cd6008e9ebef23af354c585b88490d89..b241edcd5bf136015a024fefdfd8f783400fd601 100644
--- a/ui/base/l10n/time_format_unittest.cc
+++ b/ui/base/l10n/time_format_unittest.cc
@@ -17,13 +17,39 @@ 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) {