Bug 5335 - Metrics timestamp assertions should be loosened with a precision buffer
Metrics timestamp assertions should be loosened with a precision buffer
Status: RESOLVED FIXED
Product: jbatch
Classification: Unclassified
Component: TCK
1
PC Windows
: P5 enhancement
: ---
Assigned To: ScottKurz
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-29 16:06 UTC by ScottKurz
Modified: 2013-11-05 19:05 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description ScottKurz 2013-08-29 16:06:37 UTC
See discussion on the public mailing list, where the last post was:
https://java.net/projects/jbatch/lists/public/archive/2013-08/message/20
Comment 1 ScottKurz 2013-11-04 17:20:48 UTC
This is a whole lot of verbiage over a small, simple change, but given how long it took to deliver, figured I'd post this code for review before re-publishing TCK in case I missed something.  

I'm planning on adding a "-1 second" buffer into the comparison.

I know an obvious approach would be to provide a configurable precision value.
I think we're unlikely to need that, and if we do I'd like to understand why better anyway.  Plus then I'd have to maintain what is the "officially-allowed precision range" and maintain that as doc.. so I didn't.

(You could argue the test is too loose already but at least it enforces that the API is actually implemented in the ballpark sense instead of altogether forgotten.)

If I don't hear any comments (e.g. from Michael, who raised the issue, we'll ship this).

Remember the Date(s) are both returned by the API as well as constructed by the tests via:
  long time = System.currentTimeMillis();
  Date ts = new Date(time);

--- Key part of the fix, in code ---
/*
* We want to confirm that 'd1' is roughly before 'd2', and also to
* allow for the fact that dates may be stored with a loss of precision.
* 
* Let's assume then that we only have whole seconds precision (without
* necessarily accounting for any fractional seconds).
* 
* So we can't simply perform d1 < d2, or even d1 <= d2 (the inclusion of 'equals' 
* corrects for a different problem, the problem of running so fast that
* the times for d1 and d2 are the same even though d1 may still have
* been executed first).
* 
* The "worst" case (in terms of highest rounding error), then, is that 'd1' gets
* rounded up while 'd2' gets rounded down, leaving the rounded 'd1' value a full 
* second higher than the rounded 'd2' value.
* 
* Therefore we check that d2 minus d1, which before rounding should be >= 0, is
* instead no less than -1000 (1 second).
*/
private static boolean roughlyOrdered(Date d1, Date d2) {
    long time1 = d1.getTime();
    long time2 = d2.getTime();

    long diff = time2 - time1;

    return diff >= -1000 ? true : false;
}