Dates.round() behaves incorrectly for minutes and seconds

• Key: LANG-346
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Henri Yandell
• Reporter: Ken Dombeck
• Affects Version/s: 2.2, 2.3
• Fix Version/s: 2.4

Description

Get unexpected output for rounding by minutes or seconds.

public void testRound()
{
Calendar testCalendar = Calendar.getInstance(TimeZone.getTimeZone("GMT"));
testCalendar.set(2007, 6, 2, 8, 9, 50);
Date date = testCalendar.getTime();
System.out.println("Before round() " + date);
System.out.println("After round() " + DateUtils.round(date, Calendar.MINUTE));
}

--2.1 produces
Before round() Mon Jul 02 03:09:50 CDT 2007
After round() Mon Jul 02 03:10:00 CDT 2007 – this is what I would expect

--2.2 and 2.3 produces
Before round() Mon Jul 02 03:09:50 CDT 2007
After round() Mon Jul 02 03:01:00 CDT 2007 – this appears to be wrong

Comments

Digging into this - I can definitely reproduce the error with the test case you include - many thanks for that.

Looking at the source code history, the error presumably came in during changes to the modify(Calendar, int, boolean) method for LANG-59.

[Dave Meikle]

Unit Test to follow as I need to shoot away to work

[Dave Meikle]

New Patch with Unit Tests

[Henri Yandell]

The patch fails for me. I'm on PDT rather than GMT and the time is off by 8 hours.

Testcase: testRoundLang346(org.apache.commons.lang.time.DateUtilsTest): FAILED
Minute Round Up Failed expected:<Mon Jul 02 09:09:00 PDT 2007> but was:<Mon Jul 02 01:09:00 PDT 2007>
junit.framework.AssertionFailedError: Minute Round Up Failed expected:<Mon Jul 02 09:09:00 PDT 2007> but was:<Mon Jul 02 01:09:00 PDT 2007>
at org.apache.commons.lang.time.DateUtilsTest.testRoundLang346(DateUtilsTest.java:704)

[Dave Meikle]

Sorry about that - This patch should remove the unit test's dependency on GMT.

[Henri Yandell]

That works better. Fails with the test, passes with the fix.

I'll dig through it a bit more before committing - thanks for finding/fixing this.

[Henri Yandell]

Here's a slightly different fix patch, what do you think Dave?

Doing the < Calendar.SECOND felt a bit weird to me, and I think the bit needed is to pull the 'done = true's outside of their blocks.

[Dave Meikle]

Yeah, that looks good - I totally agree with the < Calendar.SECOND bit. Now I see it, I am wondering why I didn't too that

Cheers,
Dave

[Henri Yandell]

Patches applied. Thanks Dave, and thanks to Ken for originally reporting this.