[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread rchandia 

LGTM


http://gwt-code-reviews.appspot.com/534801/diff/15001/16002
File user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java
(right):

http://gwt-code-reviews.appspot.com/534801/diff/15001/16002#newcode76
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:76: Date
date = new Date();
Would something like date = getMockDate(final int tzoffset) overriding
getTimezoneOffset() help testing specific timezones? Although I agree
that very, very few people would have a system with say, GMT +15 min.

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread jat 

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread jat 

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread jat 

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread jat 

I18NSuite is updated, but that file is included in another change so it
isn't listed here.  All it does is add an entry for DateRecordTest.


http://gwt-code-reviews.appspot.com/534801/diff/1/2
File user/src/com/google/gwt/i18n/client/impl/DateRecord.java (right):

http://gwt-code-reviews.appspot.com/534801/diff/1/2#newcode102
user/src/com/google/gwt/i18n/client/impl/DateRecord.java:102: Date
defaultCenturyStart = new Date();
On 2010/05/18 18:22:34, rchandia wrote:

Adding an overridable protected getCurrentDate() (or something like

that) to

allow mocking the current date may help testing this part of the code.


Done.

http://gwt-code-reviews.appspot.com/534801/diff/1/2#newcode221
user/src/com/google/gwt/i18n/client/impl/DateRecord.java:221: // HBJ
date.setTime(date.getTime() + this.tzOffset * 60 * 1000);
On 2010/05/18 18:22:34, rchandia wrote:

Remove comment


Done.

http://gwt-code-reviews.appspot.com/534801/diff/1/3
File user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java
(right):

http://gwt-code-reviews.appspot.com/534801/diff/1/3#newcode38
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:38: Date
date = new Date();
On 2010/05/18 18:22:34, rchandia wrote:

Good thing we will not be around in 2040!... well, may be we will. Can

we make

these tests independent from the current date?


Done.

http://gwt-code-reviews.appspot.com/534801/diff/1/3#newcode41
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:41:
assertEquals(60, date.getYear());
On 2010/05/18 18:22:34, rchandia wrote:

Add tests for the corner cases (+19, +20, -80 and -81)


Done.

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix issue 3415 and other Date/Time parsing issues. (issue534801)

2010-05-18 Thread rchandia 

Will the test run if it is not in a test suite (I18NSuite, perhaps)?


http://gwt-code-reviews.appspot.com/534801/diff/1/2
File user/src/com/google/gwt/i18n/client/impl/DateRecord.java (right):

http://gwt-code-reviews.appspot.com/534801/diff/1/2#newcode102
user/src/com/google/gwt/i18n/client/impl/DateRecord.java:102: Date
defaultCenturyStart = new Date();
Adding an overridable protected getCurrentDate() (or something like
that) to allow mocking the current date may help testing this part of
the code.

http://gwt-code-reviews.appspot.com/534801/diff/1/2#newcode221
user/src/com/google/gwt/i18n/client/impl/DateRecord.java:221: // HBJ
date.setTime(date.getTime() + this.tzOffset * 60 * 1000);
Remove comment

http://gwt-code-reviews.appspot.com/534801/diff/1/3
File user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java
(right):

http://gwt-code-reviews.appspot.com/534801/diff/1/3#newcode38
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:38: Date
date = new Date();
Good thing we will not be around in 2040!... well, may be we will. Can
we make these tests independent from the current date?

http://gwt-code-reviews.appspot.com/534801/diff/1/3#newcode41
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:41:
assertEquals(60, date.getYear());
Add tests for the corner cases (+19, +20, -80 and -81)

http://gwt-code-reviews.appspot.com/534801/diff/1/3#newcode63
user/test/com/google/gwt/i18n/client/impl/DateRecordTest.java:63: Date
date = new Date();
See above.

http://gwt-code-reviews.appspot.com/534801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors