junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Dynamic computation of timeouts?

Open ctubbsii opened this issue 4 years ago • 9 comments

In JUnit4, it was possible to create a dynamic timeout by placing a @Rule on a method that returned a Timeout object. This provided the ability to set a timeout based on factors in the environment.

See this Apache Accumulo integration test class for example.

There does not seem to be an equivalent capability in JUnit5. If there is, how is this done? If there isn't, please add such a capability. As many of our integration tests are very resource-intensive, it is crucial for our use case of being able to scale our timeouts for when we run them in resource-constrained environments (like a developer laptop) to reduce false positives.

ctubbsii avatar Mar 03 '22 15:03 ctubbsii

It's not ideal, but JUnit 5 has a method called assertTimeoutPreemptively, which you can call with a programmatic timeout duration in every test method that needs a timeout.

For example,

@Test 
void theTest() {
  Duration duration = ...;
  assertTimeoutPreemptively(duration, () -> {
    // The test body goes here.
  });
}

This is very much a workaround, but I hope it helps in the meantime.

jbduncan avatar Mar 03 '22 16:03 jbduncan

@jbduncan wrote:

It's not ideal, but JUnit 5 has a method called assertTimeoutPreemptively, which you can call with a programmatic timeout duration in every test method that needs a timeout.

Thanks. "not ideal" is a bit of an understatement, though. We have several hundred tests that would need to be modified to make this work. This is a big deterrent to switching to JUnit5 from JUnit4.

ctubbsii avatar Mar 03 '22 18:03 ctubbsii

Thanks. "not ideal" is a bit of an understatement, though. We have several hundred tests that would need to be modified to make this work. This is a big deterrent to switching to JUnit5 from JUnit4.

@ctubbsii You're welcome! But sure, that makes sense. So I've another suggestion you can try.

JUnit 5 has an extension API called InvocationInterceptor, which can be used to wrap tests with assertTimeoutPreemptively automatically.

For example:

Extension
public static final class Timeout implements InvocationInterceptor {

    private final Duration timeoutDuration;

    public static Timeout from(Supplier<Duration> timeoutDurationSupplier) {
        return new Timeout(timeoutDurationSupplier.get());
    }

    private Timeout(Duration timeoutDuration) {
        this.timeoutDuration = Objects.requireNonNull(timeoutDuration);
    }

    @Override
    public <T> T interceptTestClassConstructor(
            Invocation<T> invocation,
            ReflectiveInvocationContext<Constructor<T>> invocationContext,
            ExtensionContext extensionContext) {
        return assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptBeforeAllMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptBeforeEachMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptTestMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public <T> T interceptTestFactoryMethod(
            Invocation<T> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        return assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptTestTemplateMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptDynamicTest(
            Invocation<Void> invocation,
            DynamicTestInvocationContext invocationContext, ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptAfterEachMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }

    @Override
    public void interceptAfterAllMethod(
            Invocation<Void> invocation,
            ReflectiveInvocationContext<Method> invocationContext,
            ExtensionContext extensionContext) {
        assertTimeoutPreemptively(timeoutDuration, invocation::proceed);
    }
}

Here's an example using this extension, adapted from Apache Accumulo's own JUnit 4 @Rule usage.

Test
class ExampleTests {

    @RegisterExtension
    static Timeout timeout = Timeout.from(
        () -> {
            int waitLonger = 0;
            try {
                String timeoutString = System.getProperty("timeout.factor");
                if (timeoutString != null && !timeoutString.isEmpty()) {
                    waitLonger = Integer.parseInt(timeoutString);
                }
            } catch (NumberFormatException exception) {
                System.err.println("Could not parse timeout.factor, defaulting to no timeout.");
            }

            return Duration.ofSeconds(waitLonger);
        }
    );

    @Test
    void theTest() {
        assertTrue(true);
    }
}

Disclaimer: I've only tested this extension with the @Test-based example above. I don't know for sure if it works correctly for other kinds of tests like @ParameterizedTests and DynamicTests, but it should. Test this extension yourself if you want more confidence. :)

Alternatively, see if a refactoring tool (like IntelliJ, OpenRewrite, comby.dev or Refaster) would allow you to change the Apache Accumulo test suite to use my first suggestion more easily.

I hope this helps!

jbduncan avatar Mar 03 '22 19:03 jbduncan

@jbduncan Thanks! I think the extension mechanism might work well for our case. It would still be nice if this were more easily supported directly, though.

ctubbsii avatar Mar 03 '22 20:03 ctubbsii

Hi @jbduncan Thanks for your solution. Is there a way to overwrite the value of @Timeout in the method or class? I want to set a different timeout for varieties @Tag. For example, I want to set a longer timeout or ignore the timeout in class for all tests with the performance tag.

@Timeout(30)
public class RESTTest {

@Test
public void normalTest() {
  // ...
}

@Tag("Performance")
public void performanSendTest() {
  // ...
}
}

lwl5219 avatar May 13 '22 03:05 lwl5219

Hi @lwl5219 ; We ended up solving this with the following code: https://github.com/apache/accumulo/blob/b00c6d216bca33330755bd0d1fdc26518ae99a59/test/src/main/java/org/apache/accumulo/harness/AccumuloITBase.java#L116-L136

We put this in a common base class for a lot of our tests. You could do the same thing, but instead of reading from a method, you can check the class for an annotation.

ctubbsii avatar May 13 '22 03:05 ctubbsii

Hi @ctubbsii , Thanks for your example. It works if we get the timeout or timeout factor from other mechanisms instead of @Timeout annotation. But the situation is different in our project. All test classes inherit from a common base class, too. We set different values for a test class via @Timeout. The @Timeout cannot be removed due to some reasons. And we also put tests with performance tag in the same class. The solution that you and @jbduncan mentioned works if the timeout value is less than the value which defines in @Timeout tag. Otherwise, it doesn't work.

lwl5219 avatar May 13 '22 04:05 lwl5219

The @Timeout cannot be removed due to some reasons.

Ah, I can't see how to do it if you have that restriction.

ctubbsii avatar May 13 '22 04:05 ctubbsii

Team decision: The @Timeout annotation and related configuration parameters could be extended to allow configuring a "provider" of timeouts that could take env variables etc. into account. However, at this time, we're not convinced how generally useful this would be. Therefore, this issue is now "waiting-for-interest" and 👍 reactions can be used to vote for it.

marcphilipp avatar May 13 '22 10:05 marcphilipp

Team decision: Given our limited capacity, the team has decided to close issues for which there has not been considerable interest.

marcphilipp avatar Apr 13 '23 14:04 marcphilipp