Retry date comparison to avoid a rare flake that might be associated with DST

Retry date comparison to avoid a rare flake that might be associated with DST

BUG=
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=30767

[kustermann, 2013-11-28 10:16:21]

https://codereview.chromium.org/92333002/diff/1/pkg/intl/test/date_time_forma...
File pkg/intl/test/date_time_format_test_core.dart (right):

https://codereview.chromium.org/92333002/diff/1/pkg/intl/test/date_time_forma...
pkg/intl/test/date_time_format_test_core.dart:174: }
I don't think you should do this.

Now that I see the code for the test, I think it should be 100% deterministic.
There should be no reason why this should be flaky.

I looked a bit deeper, and the only effect that the
format.parse()/format.format() calls have is calling two native functions:

 15   static int _timeZoneOffsetInSecondsForClampedSeconds(int
secondsSinceEpoch)
 16       native "DateNatives_timeZoneOffsetInSeconds";
 17 
 18   static int _localTimeZoneAdjustmentInSeconds()
 19       native "DateNatives_localTimeZoneAdjustmentInSeconds";

The only possibility is that the timezone on the buildbot VM changed, which is
*very unlikely*.

If that happens again, we'd like to get more information: Maybe you could print
information in case the check fails (such as local timezone, .....)

[floitsch, 2013-11-28 10:29:05]

https://codereview.chromium.org/92333002/diff/1/pkg/intl/test/date_time_forma...
File pkg/intl/test/date_time_format_test_core.dart (right):

https://codereview.chromium.org/92333002/diff/1/pkg/intl/test/date_time_forma...
pkg/intl/test/date_time_format_test_core.dart:174: }
On 2013/11/28 10:16:26, kustermann wrote:
> I don't think you should do this.
> 
> Now that I see the code for the test, I think it should be 100% deterministic.
> There should be no reason why this should be flaky.
> 
> I looked a bit deeper, and the only effect that the
> format.parse()/format.format() calls have is calling two native functions:
> 
>  15   static int _timeZoneOffsetInSecondsForClampedSeconds(int
> secondsSinceEpoch)
>  16       native "DateNatives_timeZoneOffsetInSeconds";
>  17 
>  18   static int _localTimeZoneAdjustmentInSeconds()
>  19       native "DateNatives_localTimeZoneAdjustmentInSeconds";
> 
> The only possibility is that the timezone on the buildbot VM changed, which is
> *very unlikely*.
> 
> If that happens again, we'd like to get more information: Maybe you could
print
> information in case the check fails (such as local timezone, .....)

I agree with Martin. This should never happen, and if it does again we want to
know why and how.
Instead of expect(thenPrintAgain, equals(actualResult)), do an if before:
if (thenPrintAgain != actualResult) {
  print(date);
  print(date.millisecondsSinceEpoch);
  print(date.timeZoneOffset);
  print(date);
  print(parsed.millisecondsSinceEpoch);
  print(parsed.timeZoneOffset);
  print(parsed.timeZone);
  print(parsed.timeZone);
  print(new DateTime.now().millisecondsSinceEpoch);
  // and any other information that could be helpful to debug.
}

[Alan Knight, 2013-11-28 19:08:42]

Yes, that's a much better idea. Revised.

[floitsch, 2013-11-28 19:13:16]

LGTM.

[Alan Knight, 2013-11-28 19:31:37]

Committed patchset #2 manually as r30767 (presubmit successful).