Skip to content

User-facing TestWorkflowRule doesn't enforce test timeouts anymore #719

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.

Sign up for GitHub

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

Merged
Spikhalskiy merged 1 commit into from
Sep 16, 2021
Merged

User-facing TestWorkflowRule doesn't enforce test timeouts anymore #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions temporal-kotlin/src/test/kotlin/io/temporal/workflow/KotlinAsyncChildWorkflowTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import io.temporal.common.converter.JacksonJsonPayloadConverter
import io.temporal.common.converter.KotlinObjectMapperFactory
import io.temporal.internal.async.FunctionWrappingUtil
import io.temporal.internal.sync.AsyncInternal
import io.temporal.testing.TestWorkflowRule
import io.temporal.testing.internal.SDKTestWorkflowRule
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
Expand All @@ -35,7 +35,7 @@ class KotlinAsyncChildWorkflowTest {

@Rule
@JvmField
var testWorkflowRule: TestWorkflowRule = TestWorkflowRule.newBuilder()
var testWorkflowRule: SDKTestWorkflowRule = SDKTestWorkflowRule.newBuilder()
.setWorkflowTypes(ParentWorkflowImpl::class.java, ChildWorkflowImpl::class.java)
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
Expand Down
6 changes: 3 additions & 3 deletions temporal-kotlin/src/test/kotlin/io/temporal/workflow/KotlinAsyncCompanionFunctionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import io.temporal.common.converter.JacksonJsonPayloadConverter
import io.temporal.common.converter.KotlinObjectMapperFactory
import io.temporal.internal.async.FunctionWrappingUtil
import io.temporal.internal.sync.AsyncInternal
import io.temporal.testing.TestWorkflowRule
import junit.framework.Assert.assertTrue
import io.temporal.testing.internal.SDKTestWorkflowRule
import org.junit.Assert
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import java.util.concurrent.atomic.AtomicBoolean
Expand All @@ -46,7 +46,7 @@ class KotlinAsyncCompanionFunctionTest {

@Rule
@JvmField
var testWorkflowRule: TestWorkflowRule = TestWorkflowRule.newBuilder()
var testWorkflowRule: SDKTestWorkflowRule = SDKTestWorkflowRule.newBuilder()
.setWorkflowTypes(CompanionFunctionReferenceWorkflowImpl::class.java)
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
Expand Down
6 changes: 3 additions & 3 deletions temporal-kotlin/src/test/kotlin/io/temporal/workflow/KotlinAsyncLambdaTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ import io.temporal.common.converter.JacksonJsonPayloadConverter
import io.temporal.common.converter.KotlinObjectMapperFactory
import io.temporal.internal.async.FunctionWrappingUtil
import io.temporal.internal.sync.AsyncInternal
import io.temporal.testing.TestWorkflowRule
import junit.framework.Assert.assertTrue
import io.temporal.testing.internal.SDKTestWorkflowRule
import org.junit.Assert
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import java.util.concurrent.atomic.AtomicBoolean
Expand All @@ -41,7 +41,7 @@ class KotlinAsyncLambdaTest {

@Rule
@JvmField
var testWorkflowRule: TestWorkflowRule = TestWorkflowRule.newBuilder()
var testWorkflowRule: SDKTestWorkflowRule = SDKTestWorkflowRule.newBuilder()
.setWorkflowTypes(LambdaWorkflowImpl::class.java)
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
Expand Down
8 changes: 4 additions & 4 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/ActivityFailureTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

package io.temporal.opentracing;

import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;

import io.opentracing.Scope;
import io.opentracing.mock.MockSpan;
Expand All @@ -34,7 +34,7 @@
import io.temporal.client.WorkflowOptions;
import io.temporal.common.RetryOptions;
import io.temporal.failure.ApplicationFailure;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -53,8 +53,8 @@ public class ActivityFailureTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
6 changes: 3 additions & 3 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/CustomSpanNamingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import io.temporal.common.RetryOptions;
import io.temporal.failure.ApplicationFailure;
import io.temporal.opentracing.internal.ActionTypeAndNameSpanBuilderProvider;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -55,8 +55,8 @@ public class CustomSpanNamingTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(
Expand Down
6 changes: 3 additions & 3 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/LocalActivityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import io.temporal.client.WorkflowClient;
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -50,8 +50,8 @@ public class LocalActivityTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
6 changes: 3 additions & 3 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/NoClientSpanTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import io.temporal.client.WorkflowClient;
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -47,8 +47,8 @@ public class NoClientSpanTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
14 changes: 7 additions & 7 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/SpanContextPropagationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@

package io.temporal.opentracing;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.*;

import io.opentracing.Scope;
import io.opentracing.Span;
Expand All @@ -37,9 +35,11 @@
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.failure.ApplicationFailure;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.*;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
import io.temporal.workflow.WorkflowMethod;
import java.time.Duration;
import org.junit.After;
import org.junit.Before;
Expand All @@ -54,8 +54,8 @@ public class SpanContextPropagationTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
6 changes: 3 additions & 3 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/WorkflowReplayTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.common.RetryOptions;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -52,8 +52,8 @@ public class WorkflowReplayTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
6 changes: 3 additions & 3 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/WorkflowRetryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import io.temporal.client.WorkflowOptions;
import io.temporal.common.RetryOptions;
import io.temporal.failure.ApplicationFailure;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
Expand All @@ -53,8 +53,8 @@ public class WorkflowRetryTest {
new MockTracer(new ThreadLocalScopeManager(), MockTracer.Propagator.TEXT_MAP);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor())
Expand Down
16 changes: 11 additions & 5 deletions temporal-opentracing/src/test/java/io/temporal/opentracing/integration/JaegerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,21 @@
import io.temporal.client.WorkflowClient;
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.opentracing.*;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.opentracing.OpenTracingClientInterceptor;
import io.temporal.opentracing.OpenTracingOptions;
import io.temporal.opentracing.OpenTracingSpanContextCodec;
import io.temporal.opentracing.OpenTracingWorkerInterceptor;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.worker.WorkerFactoryOptions;
import io.temporal.workflow.Workflow;
import io.temporal.workflow.WorkflowInterface;
import io.temporal.workflow.WorkflowMethod;
import java.time.Duration;
import java.util.List;
import org.junit.*;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

public class JaegerTest {
private static final OpenTracingOptions JAEGER_COMPATIBLE_CONFIG =
Expand All @@ -57,8 +63,8 @@ public class JaegerTest {
private Tracer tracer;

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowClientOptions(
WorkflowClientOptions.newBuilder()
.setInterceptors(new OpenTracingClientInterceptor(JAEGER_COMPATIBLE_CONFIG))
Expand Down
6 changes: 3 additions & 3 deletions temporal-sdk/src/test/java/io/temporal/workflow/WorkflowRetryAfterActivityFailureTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import io.temporal.common.RetryOptions;
import io.temporal.failure.ApplicationFailure;
import io.temporal.failure.TerminatedFailure;
import io.temporal.testing.TestWorkflowRule;
import io.temporal.testing.internal.SDKTestWorkflowRule;
import io.temporal.workflow.shared.TestActivities.TestActivity1;
import io.temporal.workflow.shared.TestWorkflows.TestWorkflow1;
import java.time.Duration;
Expand All @@ -42,8 +42,8 @@ public class WorkflowRetryAfterActivityFailureTest {
private static AtomicInteger failureCounter = new AtomicInteger(1);

@Rule
public TestWorkflowRule testWorkflowRule =
TestWorkflowRule.newBuilder()
public SDKTestWorkflowRule testWorkflowRule =
SDKTestWorkflowRule.newBuilder()
.setWorkflowTypes(WorkflowImpl.class)
.setActivityImplementations(new FailingActivityImpl())
.build();
Expand Down
33 changes: 24 additions & 9 deletions temporal-testing-junit4/src/main/java/io/temporal/testing/TestWorkflowRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import io.temporal.api.workflowservice.v1.WorkflowServiceGrpc;
import io.temporal.client.WorkflowClient;
import io.temporal.client.WorkflowClientOptions;
import io.temporal.client.WorkflowOptions;
import io.temporal.client.WorkflowStub;
import io.temporal.common.interceptors.WorkerInterceptor;
import io.temporal.internal.common.DebugModeUtils;
import io.temporal.worker.Worker;
Expand Down Expand Up @@ -68,8 +70,6 @@
*/
public class TestWorkflowRule implements TestRule {

private static final long DEFAULT_TEST_TIMEOUT_SECONDS = 10;

private final String namespace;
private final boolean useExternalService;
private final boolean doNotStart;
Expand Down Expand Up @@ -112,11 +112,8 @@ private TestWorkflowRule(Builder builder) {
? WorkflowImplementationOptions.getDefaultInstance()
: builder.workflowImplementationOptions;
globalTimeout =
!DebugModeUtils.isTemporalDebugModeOn()
? Timeout.seconds(
builder.testTimeoutSeconds == 0
? DEFAULT_TEST_TIMEOUT_SECONDS
: builder.testTimeoutSeconds)
!DebugModeUtils.isTemporalDebugModeOn() && builder.testTimeoutSeconds != 0
? Timeout.seconds(builder.testTimeoutSeconds)
: null;

WorkflowClientOptions clientOptions =
Expand Down Expand Up @@ -145,7 +142,6 @@ public static class Builder {
private String target;
private boolean useExternalService;
private boolean doNotStart;
private long testTimeoutSeconds;
private long initialTimeMillis;

private Class<?>[] workflowTypes;
Expand All @@ -154,6 +150,7 @@ public static class Builder {
private WorkflowClientOptions workflowClientOptions;
private WorkerFactoryOptions workerFactoryOptions;
private WorkerOptions workerOptions;
private long testTimeoutSeconds;

protected Builder() {}

Expand Down Expand Up @@ -221,7 +218,11 @@ public Builder setTarget(String target) {
return this;
}

/** Global test timeout. Default is 10 seconds. */
/**
* @deprecated Temporal test rule shouldn't be responsible for enforcing test timeouts. Use
* toolchain of your test framework to enforce timeouts.
Comment on lines +222 to +223
Copy link
Copy Markdown
Member

@Sushisource Sushisource Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should mention not only is this deprecated, it now does nothing.

Copy link
Copy Markdown
Member

@mfateev mfateev Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about deprecating it for now, but setting timeout only if the value is set?

Copy link
Copy Markdown
Contributor Author

@Spikhalskiy Spikhalskiy Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rolled back the removal of the actual implementation. Let's clean it out in 1.5

*/
@Deprecated
public Builder setTestTimeoutSeconds(long testTimeoutSeconds) {
this.testTimeoutSeconds = testTimeoutSeconds;
return this;
Expand Down Expand Up @@ -369,4 +370,18 @@ public History getWorkflowExecutionHistory(WorkflowExecution execution) {
public Worker getWorker() {
return testEnvironment.getWorkerFactory().getWorker(getTaskQueue());
}

public <T> T newWorkflowStub(Class<T> workflow) {
return getWorkflowClient()
.newWorkflowStub(workflow, newWorkflowOptionsForTaskQueue(getTaskQueue()));
}

public WorkflowStub newUntypedWorkflowStub(String workflow) {
return getWorkflowClient()
.newUntypedWorkflowStub(workflow, newWorkflowOptionsForTaskQueue(getTaskQueue()));
}

private static WorkflowOptions newWorkflowOptionsForTaskQueue(String taskQueue) {
return WorkflowOptions.newBuilder().setTaskQueue(taskQueue).build();
}
}
4 changes: 0 additions & 4 deletions temporal-testing-junit4/src/main/java/io/temporal/testing/internal/SDKTestOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ public class SDKTestOptions {
// stepping through code in a debugger without timing out.
private static final boolean DEBUGGER_TIMEOUTS = false;

public static WorkflowOptions newWorkflowOptionsForTaskQueue(String taskQueue) {
return WorkflowOptions.newBuilder().setTaskQueue(taskQueue).build();
}

public static WorkflowOptions newWorkflowOptionsForTaskQueue200sTimeout(String taskQueue) {
return WorkflowOptions.newBuilder()
.setWorkflowRunTimeout(Duration.ofSeconds(200))
Expand Down
Loading