Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate timout in favour of library based solutions #1694

Closed
mpkorstanje opened this issue Jul 12, 2019 · 0 comments
Closed

Deprecate timout in favour of library based solutions #1694

mpkorstanje opened this issue Jul 12, 2019 · 0 comments
Milestone

Comments

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jul 12, 2019

Summary

It is possible to provide a timeout to step definitions, however the semantics are unreliable ( #1506).

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

Current Behavior

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).

Possible Solution

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:

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

Context & Motivation

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.

@mpkorstanje mpkorstanje added this to the 5.x.x milestone Jul 12, 2019
mpkorstanje added a commit that referenced this issue Jul 12, 2019
## Summary

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

## Current Behavior

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).

## Possible Solution

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.

## Context & Motivation

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.
mpkorstanje added a commit that referenced this issue Oct 8, 2019
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant