[Webkit-unassigned] [Bug 130123] Incorrect Date returned between March 1, 2034 and February 28, 2100.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130123


Mark Lam <mark.lam at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #226728|review?                     |review-
               Flag|                            |




--- Comment #10 from Mark Lam <mark.lam at apple.com>  2014-03-14 10:39:12 PST ---
(From update of attachment 226728)
View in context: https://bugs.webkit.org/attachment.cgi?id=226728&action=review

> LayoutTests/js/script-tests/date-constructor.js:73
> +var monthNames = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December" ];

Please remove the space between “December” and “];”.

> LayoutTests/js/script-tests/date-constructor.js:81
> +    if (dateObj.getFullYear() != year) {
> +        success = false;

Please add:
    shouldBe(“new Date(“ + dateString + “).getFullYear()”, year);

That way, if it fails, we’ll know what values it failed on, and lessen the effort to get a reproduction case for debugging the issue.

> LayoutTests/js/script-tests/date-constructor.js:83
> +    } if (dateObj.getMonth() != month) {
> +        success = false;

Ditto.  Please add:
    shouldBe(“new Date(“ + dateString + “).getMonth()”, month);

> LayoutTests/js/script-tests/date-constructor.js:85
> +    } if (dateObj.getDate() != date) {
> +        success = false;

Ditto.  Please add:
    shouldBe(“new Date(“ + dateString + “).getDate()”, date);

> LayoutTests/js/script-tests/date-constructor.js:100
> +var leapTestResult = true;
> +var year = 1970;
> +var month = 2;
> +var date = 1;
> +
> +// Check value of March 1 day since 1970 till 9999
> +while (year < 10000) {
> +    leapTestResult = testDate(year, month, date); 
> +    if(!leapTestResult) break;
> +    year += 1; 
> +} 

In my original suggestion, I had the month and date incrementing by a prime number so that we can test different months and dates as well:  
     month = (month + 7) % 12;
     date = (date + 13) % 28;

Is there a reason you opted to go only with "March 1”?  I feel that this actually lessens the coverage of the test.  If you don’t have a good reason for this, please add back the date and month increments so that we can test different values.  If there’s something special about “March 1”, then please add a line to test it explicitly (in addition to the one that tests the varying date values) with a comment to describe why it is :
     leapTestResult ||= testDate(year, 2, 1); // <reason why you’re explicitly testing this month and date …>

In my original suggestion, I also incremented the year by a prime number greater than 1.  The reason was so that the test will take less time to run.  But I supposed any modern computer should be able to blow thru 10000 iterations in no time.  So, incrementing the year by 1 is fine.

According to http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/getFullYear, the full year ranges from 1000 to 9999.  So, we might as well test Date ranges in the past as well.  A quick test on FF, Chrome, and Safari all reveals that they can take years going back to year 100.  On Safari and Chrome, years less than that get aliases e.g. 99 become 1999, 10 becomes 2010.  On FF, 99 becomes 1999, 10 returns a NaN.  How about starting the year with year 100?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.