Skip to content

Commit

Permalink
[Java] Deprecate timout in favour of library based solutions
Browse files Browse the repository at this point in the history
It is possible to provide a timeout to step definitions, however the
semantics are unreliable ( #1506).

```java
@given(value = "^I have (\\d+) cukes in my belly$", timeout = 5000)
public void I_have_cukes_in_my_belly(int cukes) throws Throwable {

}
```

Fixes: #1694

When the step starts a long running task Cucumber will attempt to
interrupt the step once the timeout period is exceeded. If the long
running task ignores the interrupt Cucumber will however not stop the
test. Depending on the context this behavior is either desired or
undesirable. See #1506 for in detail discussion.

Additionally the current implementation is complex and has been prone
to failures (#1244, #1241, #811, #639, #540).

While it is possible to implement different strategies to deal with
timeouts; there is no perfect solution. And regardless of which solution
we pick we would take on a significant amount of complexity. So I
believe that for Cucumber there is no good solution.

However this problem has been solved by various libraries:

 * [JUnit 5 `Assertions.assertTimeout*`](https://junit.org/junit5/docs/5.0.1/api/org/junit/jupiter/api/Assertions.html#assertTimeout-java.time.Duration-org.junit.jupiter.api.function.Executable-)
 * [Awaitility](https://github.com/awaitility/awaitility)
 * [Guava `TimeLimiter`](https://github.com/google/guava/blob/master/guava/src/com/google/common/util/concurrent/TimeLimiter.java)

So rather then keeping a poor feature alive we should recommend users
to migrate to third party solutions.

Remove the eldritch horror that is `Invoker/Timeout.timeout`. While this
could have been done in v4.x, v5 will use Java 8 and most of the above
libraries require Java 8.

(cherry picked from commit 0ee6620)
  • Loading branch information
mpkorstanje committed Oct 8, 2019
1 parent e883987 commit ad78266
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 0 deletions.
3 changes: 3 additions & 0 deletions java/src/main/groovy/annotation.java.gsp
Expand Up @@ -49,6 +49,9 @@ public @interface ${kw} {
* cucumber will wait for the this hook to finish.
*
* @return timeout in milliseconds. 0 (default) means no restriction.
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
long timeout() default 0;
}
3 changes: 3 additions & 0 deletions java/src/main/java/io/cucumber/java/After.java
Expand Up @@ -28,7 +28,10 @@
* Cucumber will wait for the this hook to finish.
*
* @return timeout in milliseconds. 0 (default) means no restriction.
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
long timeout() default 0;

/**
Expand Down
3 changes: 3 additions & 0 deletions java/src/main/java/io/cucumber/java/AfterStep.java
Expand Up @@ -28,7 +28,10 @@
* Cucumber will wait for the this hook to finish.
*
* @return timeout in milliseconds. 0 (default) means no restriction.
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
long timeout() default 0;

/**
Expand Down
3 changes: 3 additions & 0 deletions java/src/main/java/io/cucumber/java/Before.java
Expand Up @@ -28,7 +28,10 @@
* Cucumber will wait for the this hook to finish.
*
* @return timeout in milliseconds. 0 (default) means no restriction.
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
long timeout() default 0;

/**
Expand Down
3 changes: 3 additions & 0 deletions java/src/main/java/io/cucumber/java/BeforeStep.java
Expand Up @@ -28,7 +28,10 @@
* Cucumber will wait for the this hook to finish.
*
* @return timeout in milliseconds. 0 (default) means no restriction.
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
long timeout() default 0;

/**
Expand Down
6 changes: 6 additions & 0 deletions java8/src/main/groovy/lambda.java.gsp
Expand Up @@ -63,7 +63,10 @@ public interface ${className} extends LambdaGlue {
* @param expression the cucumber expression
* @param timeoutMillis timeout in milliseconds. 0 (default) means no restriction.
* @param body a lambda expression with no parameters
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void ${java.text.Normalizer.normalize(kw.replaceAll("[\\s',!]", ""), java.text.Normalizer.Form.NFC)}(String expression, long timeoutMillis, A0 body) {
LambdaGlueRegistry.INSTANCE.get().addStepDefinition((typeRegistry) ->
Java8StepDefinition.create(expression, timeoutMillis, A0.class, body, typeRegistry)
Expand Down Expand Up @@ -101,7 +104,10 @@ public interface ${className} extends LambdaGlue {
* @param body a lambda expression with ${arity} parameters
* <% (1..arity).each { i -> %>
* @param <T${i}> type of argument ${i} <% } %>
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default <${genericSignature}> void ${java.text.Normalizer.normalize(kw.replaceAll("[\\s',!]", ""), java.text.Normalizer.Form.NFC)}(String expression, long timeoutMillis, A${arity}<${genericSignature}> body) {
LambdaGlueRegistry.INSTANCE.get().addStepDefinition((typeRegistry) ->
Java8StepDefinition.create(expression, timeoutMillis, A${arity}.class, body, typeRegistry)
Expand Down
51 changes: 51 additions & 0 deletions java8/src/main/java/io/cucumber/java8/LambdaGlue.java
Expand Up @@ -37,7 +37,10 @@ default void Before(String tagExpression, final HookBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void Before(long timeoutMillis, final HookBody body) {
JavaBackend.INSTANCE.get().addBeforeHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_BEFORE_ORDER, timeoutMillis, body));
}
Expand All @@ -59,7 +62,10 @@ default void Before(int order, final HookBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void Before(String tagExpression, long timeoutMillis, int order, final HookBody body) {
JavaBackend.INSTANCE.get().addBeforeHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -88,7 +94,10 @@ default void Before(String tagExpression, final HookNoArgsBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void Before(long timeoutMillis, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addBeforeHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_BEFORE_ORDER, timeoutMillis, body));
}
Expand All @@ -110,7 +119,10 @@ default void Before(int order, final HookNoArgsBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void Before(String tagExpression, long timeoutMillis, int order, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addBeforeHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -139,7 +151,10 @@ default void BeforeStep(String tagExpression, final HookBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void BeforeStep(long timeoutMillis, final HookBody body) {
JavaBackend.INSTANCE.get().addBeforeStepHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_BEFORE_ORDER, timeoutMillis, body));
}
Expand All @@ -161,7 +176,10 @@ default void BeforeStep(int order, final HookBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void BeforeStep(String tagExpression, long timeoutMillis, int order, final HookBody body) {
JavaBackend.INSTANCE.get().addBeforeStepHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -192,7 +210,10 @@ default void BeforeStep(String tagExpression, final HookNoArgsBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void BeforeStep(long timeoutMillis, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addBeforeStepHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_BEFORE_ORDER, timeoutMillis, body));
}
Expand All @@ -214,7 +235,10 @@ default void BeforeStep(int order, final HookNoArgsBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void BeforeStep(String tagExpression, long timeoutMillis, int order, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addBeforeStepHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -243,7 +267,10 @@ default void After(String tagExpression, final HookBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void After(long timeoutMillis, final HookBody body) {
JavaBackend.INSTANCE.get().addAfterHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_AFTER_ORDER, timeoutMillis, body));
}
Expand All @@ -265,7 +292,10 @@ default void After(int order, final HookBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void After(String tagExpression, long timeoutMillis, int order, final HookBody body) {
JavaBackend.INSTANCE.get().addAfterHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -294,7 +324,10 @@ default void After(String tagExpression, final HookNoArgsBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void After(long timeoutMillis, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addAfterHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_AFTER_ORDER, timeoutMillis, body));
}
Expand All @@ -316,7 +349,10 @@ default void After(int order, final HookNoArgsBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void After(String tagExpression, long timeoutMillis, int order, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addAfterHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down Expand Up @@ -345,7 +381,10 @@ default void AfterStep(String tagExpression, final HookBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void AfterStep(long timeoutMillis, final HookBody body) {
JavaBackend.INSTANCE.get().addAfterStepHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_AFTER_ORDER, timeoutMillis, body));
}
Expand All @@ -367,7 +406,10 @@ default void AfterStep(int order, final HookBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute, takes {@link cucumber.api.Scenario} as an argument
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void AfterStep(String tagExpression, long timeoutMillis, int order, final HookBody body) {
JavaBackend.INSTANCE.get().addAfterStepHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand All @@ -386,7 +428,10 @@ default void AfterStep(final HookNoArgsBody body) {
*
* @param tagExpression a tag expression, if the expression applies to the current scenario this hook will be executed
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void AfterStep(String tagExpression, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addAfterStepHookDefinition(new Java8HookDefinition(tagExpression, DEFAULT_AFTER_ORDER, NO_TIMEOUT, body));
}
Expand All @@ -396,7 +441,10 @@ default void AfterStep(String tagExpression, final HookNoArgsBody body) {
*
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void AfterStep(long timeoutMillis, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addAfterStepHookDefinition(new Java8HookDefinition(EMPTY_TAG_EXPRESSIONS, DEFAULT_AFTER_ORDER, timeoutMillis, body));
}
Expand All @@ -418,7 +466,10 @@ default void AfterStep(int order, final HookNoArgsBody body) {
* @param timeoutMillis max amount of milliseconds this is allowed to run for
* @param order the order in which this hook should run. Higher numbers are run first
* @param body lambda to execute
* @deprecated use a library based solution instead. E.g. Awaitility
* or JUnit 5s Assertions.assertTimeout.
*/
@Deprecated
default void AfterStep(String tagExpression, long timeoutMillis, int order, final HookNoArgsBody body) {
JavaBackend.INSTANCE.get().addAfterStepHookDefinition(new Java8HookDefinition(tagExpression, order, timeoutMillis, body));
}
Expand Down

0 comments on commit ad78266

Please sign in to comment.