From af117e9764db5f6ea3d64fb921893022b3ef7474 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 27 Feb 2024 10:53:17 -0800 Subject: [PATCH] core: Don't use real time when comparing deadline in ClientCallImplTest We can just compare the Deadline instances instead of asserting that very little time has passed during the test. Real time probably still matters in the test, but only insofar that the deadline is not expired by the time ClientCallImpl sees it. This fixes a test failure seen in the emulated aarch64 CI. Note that the message says "ns" when it should say "ms", but this change deletes the code with that typo. ``` java.lang.AssertionError: timeout: 548 ns at org.junit.Assert.fail(Assert.java:89) at org.junit.Assert.assertTrue(Assert.java:42) at io.grpc.internal.ClientCallImplTest.assertTimeoutBetween(ClientCallImplTest.java:1102) at io.grpc.internal.ClientCallImplTest.contextDeadlineShouldBePropagatedToStream(ClientCallImplTest.java:828) ``` --- .../io/grpc/internal/ClientCallImplTest.java | 38 ++++++------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java index d41f2113f5..34011cd844 100644 --- a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java @@ -807,8 +807,8 @@ public class ClientCallImplTest { @Test public void contextDeadlineShouldBePropagatedToStream() { - Context context = Context.current() - .withDeadlineAfter(1000, TimeUnit.MILLISECONDS, deadlineCancellationExecutor); + Deadline deadline = Deadline.after(1000, TimeUnit.MILLISECONDS); + Context context = Context.current().withDeadline(deadline, deadlineCancellationExecutor); Context origContext = context.attach(); ClientCallImpl call = new ClientCallImpl<>( @@ -822,16 +822,13 @@ public class ClientCallImplTest { context.detach(origContext); - ArgumentCaptor deadlineCaptor = ArgumentCaptor.forClass(Deadline.class); - verify(stream).setDeadline(deadlineCaptor.capture()); - - assertTimeoutBetween(deadlineCaptor.getValue().timeRemaining(TimeUnit.MILLISECONDS), 600, 1000); + verify(stream).setDeadline(eq(deadline)); } @Test public void contextDeadlineShouldOverrideLargerCallOptionsDeadline() { - Context context = Context.current() - .withDeadlineAfter(1000, TimeUnit.MILLISECONDS, deadlineCancellationExecutor); + Deadline deadline = Deadline.after(1000, TimeUnit.MILLISECONDS); + Context context = Context.current().withDeadline(deadline, deadlineCancellationExecutor); Context origContext = context.attach(); CallOptions callOpts = baseCallOptions.withDeadlineAfter(2000, TimeUnit.MILLISECONDS); @@ -846,19 +843,17 @@ public class ClientCallImplTest { context.detach(origContext); - ArgumentCaptor deadlineCaptor = ArgumentCaptor.forClass(Deadline.class); - verify(stream).setDeadline(deadlineCaptor.capture()); - - assertTimeoutBetween(deadlineCaptor.getValue().timeRemaining(TimeUnit.MILLISECONDS), 600, 1000); + verify(stream).setDeadline(eq(deadline)); } @Test public void contextDeadlineShouldNotOverrideSmallerCallOptionsDeadline() { Context context = Context.current() .withDeadlineAfter(2000, TimeUnit.MILLISECONDS, deadlineCancellationExecutor); + Deadline deadline = Deadline.after(1000, TimeUnit.MILLISECONDS); Context origContext = context.attach(); - CallOptions callOpts = baseCallOptions.withDeadlineAfter(1000, TimeUnit.MILLISECONDS) + CallOptions callOpts = baseCallOptions.withDeadline(deadline) .withOption(NAME_RESOLUTION_DELAYED, 1200000000L); ClientCallImpl call = new ClientCallImpl<>( method, @@ -871,10 +866,8 @@ public class ClientCallImplTest { context.detach(origContext); - ArgumentCaptor deadlineCaptor = ArgumentCaptor.forClass(Deadline.class); - verify(stream).setDeadline(deadlineCaptor.capture()); + verify(stream).setDeadline(eq(deadline)); - assertTimeoutBetween(deadlineCaptor.getValue().timeRemaining(TimeUnit.MILLISECONDS), 600, 1000); fakeClock.forwardNanos(TimeUnit.MILLISECONDS.toNanos(1000)); verify(stream, timeout(1000)).cancel(statusCaptor.capture()); String deadlineExceedDescription = statusCaptor.getValue().getDescription(); @@ -884,7 +877,8 @@ public class ClientCallImplTest { @Test public void callOptionsDeadlineShouldBePropagatedToStream() { - CallOptions callOpts = baseCallOptions.withDeadlineAfter(1000, TimeUnit.MILLISECONDS); + Deadline deadline = Deadline.after(1000, TimeUnit.MILLISECONDS); + CallOptions callOpts = baseCallOptions.withDeadline(deadline); ClientCallImpl call = new ClientCallImpl<>( method, MoreExecutors.directExecutor(), @@ -894,10 +888,7 @@ public class ClientCallImplTest { channelCallTracer, configSelector); call.start(callListener, new Metadata()); - ArgumentCaptor deadlineCaptor = ArgumentCaptor.forClass(Deadline.class); - verify(stream).setDeadline(deadlineCaptor.capture()); - - assertTimeoutBetween(deadlineCaptor.getValue().timeRemaining(TimeUnit.MILLISECONDS), 600, 1000); + verify(stream).setDeadline(eq(deadline)); } @Test @@ -1097,11 +1088,6 @@ public class ClientCallImplTest { assertEquals(attrs, call.getAttributes()); } - private static void assertTimeoutBetween(long timeout, long from, long to) { - assertTrue("timeout: " + timeout + " ns", timeout <= to); - assertTrue("timeout: " + timeout + " ns", timeout >= from); - } - private static final class DelayedExecutor implements Executor { private final BlockingQueue commands = new LinkedBlockingQueue<>();