Fixed tests TimeFormattingTest.TimeFormatDateUS & GB.

Fixed tests TimeFormattingTest.TimeFormatDateUS & GB.

Tests TimeFormattingTest.TimeFormatDateUS and
TimeFormattingTest.TimeFormatDateGB were failed previously in some time zones
because difference between local time and GMT is time-dependent.

Committed: https://crrev.com/846dea256fbf77f226bc27317a5c9181c6dd3507
Cr-Commit-Position: refs/heads/master@{#337021}

[Vitaly Baranov, 2015-06-30 14:13:34]

vitbar@yandex-team.ru changed reviewers:
+ danakj@chromium.org, jww@chromium.org

[Vitaly Baranov, 2015-06-30 14:17:44]

On 2015/06/30 14:13:34, Vitaly Baranov wrote:
> mailto:vitbar@yandex-team.ru changed reviewers:
> + mailto:danakj@chromium.org, mailto:jww@chromium.org

Without my changes test TimeFormattingTest.TimeFormatDateUS fails with the
following output when it runs in Moscow:

Value of: TimeFormatShortDateAndTimeWithTimeZone(time)
Actual: 4/30/11, 3:42:07 PM GMT+4
Expected: ASCIIToUTF16("4/30/11, 3:42:07 PM ") + GetShortTimeZone()
Which is: 4/30/11, 3:42:07 PM GMT+3

[Vitaly Baranov, 2015-06-30 14:41:52]

On 2015/06/30 14:17:44, Vitaly Baranov wrote:
> On 2015/06/30 14:13:34, Vitaly Baranov wrote:
> > mailto:vitbar@yandex-team.ru changed reviewers:
> > + mailto:danakj@chromium.org, mailto:jww@chromium.org
> 
> Without my changes test TimeFormattingTest.TimeFormatDateUS fails with the
> following output when it runs in Moscow:
> 
> Value of: TimeFormatShortDateAndTimeWithTimeZone(time)
> Actual: 4/30/11, 3:42:07 PM GMT+4
> Expected: ASCIIToUTF16("4/30/11, 3:42:07 PM ") + GetShortTimeZone()
> Which is: 4/30/11, 3:42:07 PM GMT+3

And I guess these test without my changes can possibly fail in USA in
November-March too.

[danakj, 2015-06-30 18:59:39]

LGTM


Please wrap your CL description at 72 chars.

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
File base/i18n/time_formatting_unittest.cc (right):

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:35:
zone_formatter->format(UTZFMT_STYLE_SPECIFIC_SHORT, *zone.get(),
nit: don't need .get()

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:37: name, NULL);
nit: nullptr

[jww, 2015-06-30 19:16:56]

lgtm

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
File base/i18n/time_formatting_unittest.cc (right):

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:33: EXPECT_TRUE(U_SUCCESS(status));
nit: I would make this an ASSERT_TRUE as it would be very surprising for this to
fail, and the rest of the test doesn't make sense if it fails.

[Vitaly Baranov, 2015-07-01 14:32:51]

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
File base/i18n/time_formatting_unittest.cc (right):

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:33: EXPECT_TRUE(U_SUCCESS(status));
On 2015/06/30 19:16:56, jww wrote:
> nit: I would make this an ASSERT_TRUE as it would be very surprising for this
to
> fail, and the rest of the test doesn't make sense if it fails.

I cannot use ASSERT_TRUE here because the function returns base::string16 and
macro ASSERT_TRUE is expanded to "...;return;" i.e. ASSERT_TRUE returns void
after generating a failure message.

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:35:
zone_formatter->format(UTZFMT_STYLE_SPECIFIC_SHORT, *zone.get(),
On 2015/06/30 18:59:38, danakj OOO back july 6 wrote:
> nit: don't need .get()

Done.

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:37: name, NULL);
On 2015/06/30 18:59:38, danakj OOO back july 6 wrote:
> nit: nullptr

Done.

[Vitaly Baranov, 2015-07-01 14:33:20]

The CQ bit was checked by vitbar@yandex-team.ru

[Vitaly Baranov, 2015-07-01 14:33:20]

The patchset sent to the CQ was uploaded after l-g-t-m from jww@chromium.org,
danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/1218043003/#ps40001
(title: "A few corrections.")

[commit-bot: I haz the power, 2015-07-01 14:33:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1218043003/40001

[commit-bot: I haz the power, 2015-07-01 14:33:38]

The author vitbar@yandex-team.ru has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-07-01 15:01:46]

The author vitbar@yandex-team.ru has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-07-01 15:02:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-01 15:03:26]

Patchset 3 (id:??) landed as
https://crrev.com/846dea256fbf77f226bc27317a5c9181c6dd3507
Cr-Commit-Position: refs/heads/master@{#337021}

[jww, 2015-07-01 17:47:36]

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
File base/i18n/time_formatting_unittest.cc (right):

https://codereview.chromium.org/1218043003/diff/20001/base/i18n/time_formatti...
base/i18n/time_formatting_unittest.cc:33: EXPECT_TRUE(U_SUCCESS(status));
On 2015/07/01 14:32:51, Vitaly Baranov wrote:
> On 2015/06/30 19:16:56, jww wrote:
> > nit: I would make this an ASSERT_TRUE as it would be very surprising for
this
> to
> > fail, and the rest of the test doesn't make sense if it fails.
> 
> I cannot use ASSERT_TRUE here because the function returns base::string16 and
> macro ASSERT_TRUE is expanded to "...;return;" i.e. ASSERT_TRUE returns void
> after generating a failure message.

Good point :-)