Github user laurentgo commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1055#discussion_r154247977
  
    --- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java
 ---
    @@ -144,6 +144,34 @@ public void pauseInjected() {
         }
       }
     
    +  @Test
    +  public void timedPauseInjected() {
    +    final long pauseDuration = 2000L;
    +    final long expectedDuration = pauseDuration;
    +    final ExtendedLatch trigger = new ExtendedLatch(1);
    +    final Pointer<Exception> ex = new Pointer<>();
    +    final String controls = Controls.newBuilder()
    +      .addTimedPause(DummyClass.class, DummyClass.PAUSES, 0, pauseDuration)
    +      .build();
    +
    +    ControlsInjectionUtil.setControls(session, controls);
    +
    +    final QueryContext queryContext = new QueryContext(session, 
bits[0].getContext(), QueryId.getDefaultInstance());
    +    //We don't need a ResumingThread because this should automatically 
resume
    +
    +    // test that the pause happens
    +    final DummyClass dummyClass = new DummyClass(queryContext, trigger);
    +    final long actualDuration = dummyClass.pauses();
    +    assertTrue(String.format("Test should stop for at least %d 
milliseconds.", expectedDuration),
    +      expectedDuration <= actualDuration);
    +    assertTrue("No exception should be thrown.", ex.value == null);
    +    try {
    +      queryContext.close();
    +    } catch (final Exception e) {
    +      fail("Failed to close query context: " + e);
    --- End diff --
    
    maybe better let JUnit handle unexpected expection (at least you would get 
the whole stack trace instead of just the error message) (I guess it's not new 
code, so feel free to ignore)


---

Reply via email to