Change DateTime.parse FormatException messages; update tests

Change DateTime.parse FormatException messages; update tests

BUG= https://code.google.com/p/dart/issues/detail?id=17753
R=lrn@google.com

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

[srawlins, 2014-08-01 02:09:01]

Hi floitsch@, I think you were the last one to touch DateTime.parse; would you
mind reviewing?

[floitsch, 2014-08-01 13:08:26]

On vacation. Reassigning to Lasse.

[Lasse Reichstein Nielsen, 2014-08-04 07:25:47]

lgtm

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart
File sdk/lib/core/date_time.dart (right):

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:242: r'^([+-]?\d{4,5})-?(\d\d)-?(\d\d)'  // The day
part.
If the max year is 275760, this should probably be \d{4,6} instead.

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:285: throw new FormatException('Out of range:
$formattedString');
Use:
  new FormattedException("Date out of range", formattedString);

I believe this change has already been made in r38378.

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:291: throw new FormatException('Could not parse:
$formattedString');
throw new FormatException("Invalid DateTime format", formattedString);

https://codereview.chromium.org/324213003/diff/1/tests/corelib/date_time_test...
File tests/corelib/date_time_test.dart (right):

https://codereview.chromium.org/324213003/diff/1/tests/corelib/date_time_test...
tests/corelib/date_time_test.dart:859: Expect.throws(() =>
DateTime.parse(bad_year.toString() + "-1-1"),
make it "-01-01", just so the reader (me) doesn't have to wonder whether
something like 1970-1-1 is a valid date. I actually think it isn't, so that
alone should give the format exception.

Shouldn't this be 1970 based somehow? 
It seems to work anyway because you only divide by 365, not 365.2425 (avg.
days/year in a 400 year cycle).

Try using:
  bad_year = 1970 + (_MAX_MILLISECONDS ~/ 1000*60*60*24*365.2425) + 1;
(which is year 275761)
So this also fails because we only accept up to 5-digit years. Change this in
datetime.dart so that this test is meaningful.

The first invalid *day* is actually
  +275760-09-14
so check that 09-13 works and -09-14 fails (and so does
"275760-09-13T00:00:00.001Z")

For negative dates, use -271821-04-20Z (good) and "-271821-04-19T23:59:59.999Z"
(bad).

[srawlins, 2014-08-04 19:58:31]

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart
File sdk/lib/core/date_time.dart (right):

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:242: r'^([+-]?\d{4,5})-?(\d\d)-?(\d\d)'  // The day
part.
On 2014/08/04 07:25:47, Lasse Reichstein Nielsen wrote:
> If the max year is 275760, this should probably be \d{4,6} instead.

Done.

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:285: throw new FormatException('Out of range:
$formattedString');
On 2014/08/04 07:25:47, Lasse Reichstein Nielsen wrote:
> Use:
>   new FormattedException("Date out of range", formattedString);
> 
> I believe this change has already been made in r38378.

Done.

https://codereview.chromium.org/324213003/diff/1/sdk/lib/core/date_time.dart#...
sdk/lib/core/date_time.dart:291: throw new FormatException('Could not parse:
$formattedString');
On 2014/08/04 07:25:47, Lasse Reichstein Nielsen wrote:
> throw new FormatException("Invalid DateTime format", formattedString);

Done.

https://codereview.chromium.org/324213003/diff/1/tests/corelib/date_time_test...
File tests/corelib/date_time_test.dart (right):

https://codereview.chromium.org/324213003/diff/1/tests/corelib/date_time_test...
tests/corelib/date_time_test.dart:859: Expect.throws(() =>
DateTime.parse(bad_year.toString() + "-1-1"),
On 2014/08/04 07:25:47, Lasse Reichstein Nielsen wrote:
> make it "-01-01", just so the reader (me) doesn't have to wonder whether
> something like 1970-1-1 is a valid date. I actually think it isn't, so that
> alone should give the format exception.
> 
> Shouldn't this be 1970 based somehow? 
> It seems to work anyway because you only divide by 365, not 365.2425 (avg.
> days/year in a 400 year cycle).
> 
> Try using:
>   bad_year = 1970 + (_MAX_MILLISECONDS ~/ 1000*60*60*24*365.2425) + 1;
> (which is year 275761)
> So this also fails because we only accept up to 5-digit years. Change this in
> datetime.dart so that this test is meaningful.
> 
> The first invalid *day* is actually
>   +275760-09-14
> so check that 09-13 works and -09-14 fails (and so does
> "275760-09-13T00:00:00.001Z")
> 
> For negative dates, use -271821-04-20Z (good) and
"-271821-04-19T23:59:59.999Z"
> (bad).

Thanks so much for these tips, lrn@! I've added the tests you suggested, and
tweaked the RegExp to accept 6-digit years. I think all is good now.

[Lasse Reichstein Nielsen, 2014-08-05 07:12:57]

lgtm

https://codereview.chromium.org/324213003/diff/20001/tests/corelib/date_time_...
File tests/corelib/date_time_test.dart (right):

https://codereview.chromium.org/324213003/diff/20001/tests/corelib/date_time_...
tests/corelib/date_time_test.dart:858: var bad_year = 1970 + (_MAX_MILLISECONDS
~/ 1000*60*60*24*365.2425) + 1;
I think this needs parentheses around the divisor.

[Lasse Reichstein Nielsen, 2014-08-07 09:41:53]

Committed patchset #3 manually as 38974 (presubmit successful).