On Fri, 28 Feb 2025 14:40:28 GMT, serhiysachkov <d...@openjdk.org> wrote:
>> switching to nanoTime as suggested in ticket comments
>
> serhiysachkov has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8281511: update Copyright header
Additionally could you have a look at the superclass Tests.java? There is this
code there:
/* check the time got is within 50% of the time expected */
public static void checkTime (long got, long expected) {
checkTime(got, expected, expected);
}
/* check the time got is between start and end, given 50% tolerance */
public static void checkTime(long got, long start, long end) {
dprintln("checkTime: got = " + got + " start = " + start + " end = " +
end);
long upper = end + (end / 2);
long lower = start - (start / 2);
if (got > upper || got < lower) {
throw new RuntimeException("checkTime failed: got " + got
+ ", expected between " + start + " and " + end);
}
}
For the exception message, it would be better to use `lower` and `upper`
instead of `start` and `end` - because when start == end the messsage lacks
credibility.
test/jdk/java/net/ipv6tests/UdpTest.java line 149:
> 147: }
> 148: final long expectedTimeInNanos = TimeUnit.SECONDS.toNanos(4);
> 149: checkTime (System.nanoTime() - t1, expectedTimeInNanos);
Actually, maybe it would be better to keep the expectedTime in milliseconds
here, and convert `System.nanoTime() - t1` to milliseconds before calling
checkTime. We obviously don't need the nano second precision, and milliseconds
are more human readable than nanoseconds when looking at the log.
test/jdk/java/net/ipv6tests/UdpTest.java line 183:
> 181: final long startTimeInNanos = TimeUnit.SECONDS.toNanos(2);
> 182: final long endTimeInNanos = TimeUnit.SECONDS.toNanos(10);
> 183: checkTime (System.nanoTime() - t1, startTimeInNanos,
> endTimeInNanos);
same remark here.
-------------
PR Review: https://git.openjdk.org/jdk/pull/23840#pullrequestreview-2651325490
PR Review Comment: https://git.openjdk.org/jdk/pull/23840#discussion_r1975746133
PR Review Comment: https://git.openjdk.org/jdk/pull/23840#discussion_r1975746875
