dlmarion commented on code in PR #4784: URL: https://github.com/apache/accumulo/pull/4784#discussion_r1704009688
########## core/src/test/java/org/apache/accumulo/core/util/TimerTest.java: ########## @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.core.util; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Test; + +public class TimerTest { + + @Test + public void testRestart() throws InterruptedException { + Timer timer = Timer.startNew(); + + // Perform a longer sleep initially + Thread.sleep(100); + + Duration firstElapsed = timer.elapsed(); + + assertTrue(timer.hasElapsed(Duration.ofMillis(100)), + "Should see at least the sleep time has elapsed."); + + timer.restart(); + + // Perform a shorter sleep + Thread.sleep(50); + + Duration secondElapsed = timer.elapsed(); + + // Assert that the elapsed time after restart is greater than 0 + assertFalse(secondElapsed.isNegative(), + "Elapsed time should be greater than 0 after restarting the timer."); + assertTrue(secondElapsed.compareTo(firstElapsed) < 0, + "Elapsed time after restart should be less than the initial elapsed time."); + + } + + @Test + public void testHasElapsed() throws InterruptedException { + Timer timer = Timer.startNew(); + + Thread.sleep(50); + + assertTrue(timer.hasElapsed(Duration.ofMillis(50)), + "The timer should indicate that 50 milliseconds have elapsed."); + assertFalse(timer.hasElapsed(Duration.ofMillis(100)), + "The timer should not indicate that 100 milliseconds have elapsed."); + } + + @Test + public void testHasElapsedWithTimeUnit() throws InterruptedException { + Timer timer = Timer.startNew(); + + Thread.sleep(50); + + assertTrue(timer.hasElapsed(50, MILLISECONDS), + "The timer should indicate that 50 milliseconds have elapsed."); + assertFalse(timer.hasElapsed(100, MILLISECONDS), + "The timer should not indicate that 100 milliseconds have elapsed."); + } + + @Test + public void testElapsedPrecision() throws InterruptedException { + Timer timer = Timer.startNew(); + + final int sleepMillis = 50; + Thread.sleep(sleepMillis); + + long elapsedMillis = timer.elapsed(MILLISECONDS); + assertEquals(sleepMillis, elapsedMillis, 5, "Elapsed time in milliseconds is not accurate."); Review Comment: @DomGarguilo - this test failed in the most recent elasticity full IT build. I'm not sure that you want to perform an `assertEquals` check on the duration of a `Thread.sleep` call. The [javadoc](https://docs.oracle.com/en%2Fjava%2Fjavase%2F11%2Fdocs%2Fapi%2F%2F/java.base/java/lang/Thread.html#sleep(long)) says that it might not be accurate and I tend to think of the Thread.sleep parameter as a lower bound. I'm fairly certain that when you call `Thread.sleep` the OS pauses the execution of that Thread, executing other tasks that need the CPU, and does not consider it for scheduling until the time has elapsed. On a lightly loaded system this likely means that the Thread will resume at or close to the time parameter that you specify. On a busy system, there is no guarantee when the Thread will resume. I think you want to change this check from `elapsedMillis == sleepMillis` to `elapsedMillis >= sleepMillis`. There may be other places in this test where that needs to happen as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
