Conversation
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds first-class wait task support to the Serverless Workflow Java Fluent API/DSL, along with runtime and test coverage, and fixes a duration-aggregation bug in the wait executor.
Changes:
- Introduces
WaitTaskBuilderand fluent interfaces to addwaittasks (including auto-naming support). - Adds DSL helper methods for constructing wait tasks from duration expressions or timeout builders.
- Fixes
WaitExecutorduration composition and adds new unit/integration tests for wait behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| impl/test/src/test/java/io/serverlessworkflow/impl/test/WaitTest.java | New runtime/integration-style tests validating that workflows actually pause for a duration. |
| impl/test/pom.xml | Adds Awaitility dependency for timing assertions (currently duplicated). |
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/WaitExecutor.java | Fixes duration accumulation to correctly add seconds/minutes/hours/days. |
| fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/TaskItemDefaultNamingTest.java | Adds coverage for wait task auto-naming and inline-duration mapping. |
| fluent/spec/src/test/java/io/serverlessworkflow/fluent/spec/dsl/DSLTest.java | Adds DSL helper coverage for wait tasks. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/WaitTaskBuilder.java | New builder for configuring wait tasks via expression, Duration, or TimeoutBuilder. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/TaskItemListBuilder.java | Adds fluent wait(...) task insertion into task lists. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/WaitFluent.java | New SPI fluent interface for wait(...) task addition. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/spi/DoFluent.java | Extends the fluent task API to include wait(...) (SPI surface change). |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/dsl/DSL.java | Adds DSL helper overloads for building wait tasks. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/DoTaskBuilder.java | Wires wait(...) into the top-level do builder. |
| fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/BaseTaskItemListBuilder.java | Adds TYPE_WAIT for deterministic auto-naming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <dependency> | ||
| <groupId>org.awaitility</groupId> | ||
| <artifactId>awaitility</artifactId> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
This module already declares org.awaitility:awaitility earlier in the dependencies list (currently without an explicit ). Adding a second declaration here results in a duplicate dependency entry and can produce Maven duplicate-dependency warnings / unexpected effective scope. Prefer updating the existing Awaitility dependency to have test (or removing the earlier one) rather than adding a second block.
| try (WorkflowApplication app = WorkflowApplication.builder().build()) { | ||
| WorkflowDefinition definition = app.workflowDefinition(workflow); | ||
| long startNanos = System.nanoTime(); | ||
| WorkflowModel model = definition.instance(Map.of()).start().join(); | ||
| long elapsedMillis = Duration.ofNanos(System.nanoTime() - startNanos).toMillis(); |
There was a problem hiding this comment.
definition.instance(...).start().join() has no timeout. If the wait task (or scheduler) regresses and never completes, this test will hang the entire build. Consider adding an explicit upper bound (e.g., CompletableFuture.orTimeout / get(timeout) / assertTimeoutPreemptively) so failures terminate promptly with a clear error.
| assertEquals(true, model.asMap().orElseThrow().get("finished")); | ||
| assertTrue( | ||
| elapsedMillis >= 5_000, | ||
| "Workflow should wait at least 5 seconds, but waited " + elapsedMillis + " ms"); |
There was a problem hiding this comment.
These tests enforce a real 5-second wall-clock delay (and the helper asserts against a hard-coded 5_000ms). This makes the suite slower and can be flaky on contended CI hosts. Consider reducing the exercised duration to ~1s (still catches the inline-duration aggregation bug) and/or using Awaitility with a small tolerance + an upper bound to keep runtime predictable.
| ForEachFluent<ForEachTaskBuilder<TaskItemListBuilder>, T>, | ||
| ForkFluent<ForkTaskBuilder, T>, | ||
| ListenFluent<ListenTaskBuilder, T>, | ||
| WaitFluent<WaitTaskBuilder, T>, | ||
| RaiseFluent<RaiseTaskBuilder, T>, | ||
| CallOpenAPIFluent<CallOpenAPITaskBuilder, T>, |
There was a problem hiding this comment.
DoFluent is in the public spi package; extending it with WaitFluent introduces a new abstract method (wait(String, Consumer)) and is a source/binary breaking change for any external implementations of DoFluent. If backward compatibility is important, consider providing a default implementation path (e.g., default methods) or clearly treating this as a major-version/SPI-breaking change.
This pull request introduces a new "wait" task type to the Serverless Workflow Java Fluent API, enabling workflows to pause execution for a specified duration.
New "wait" task support:
WaitTaskBuilderfor configuring wait tasks with duration expressions, inline durations, or timeout builders. (fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/WaitTaskBuilder.java)TaskItemListBuilder,DoTaskBuilder,DoFluent, and newWaitFluent) to support adding wait tasks fluently, including auto-naming and configuration. (fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/TaskItemListBuilder.java,DoTaskBuilder.java,spi/DoFluent.java,spi/WaitFluent.java) [1] [2] [3] [4]fluent/spec/src/main/java/io/serverlessworkflow/fluent/spec/dsl/DSL.java)Testing and validation:
TaskItemDefaultNamingTest.java,DSLTest.java) [1] [2]impl/test/src/test/java/io/serverlessworkflow/impl/test/WaitTest.java)Bug fix:
WaitExecutorwhere duration components were not being added correctly, ensuring accurate wait times. (impl/core/src/main/java/io/serverlessworkflow/impl/executors/WaitExecutor.java)Dependencies:
impl/test/pom.xml)