Skip to content

Conversation

@abudevich
Copy link

@abudevich abudevich commented Oct 9, 2025

Summary by CodeRabbit

  • New Features
    • Added configurable run-level timeout separate from story-level timeouts, with an option to fail the run when exceeded.
    • Improved monitoring/logging to announce configured run timeouts and report when they trigger, including duration details.
    • On run timeout, execution stops early and a clear run-timeout error is surfaced to users.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Run execution timeout” directly refers to the new run-level timeout feature introduced throughout the codebase and succinctly summarizes the primary change. It is concise and specific enough that a teammate scanning the history understands the main purpose at a glance. The phrase clearly conveys the feature without extraneous details.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@abudevich abudevich marked this pull request as draft October 9, 2025 08:07
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
jbehave-core/src/main/java/org/jbehave/core/embedder/DelegatingEmbedderMonitor.java (1)

167-200: Delegate the new run-timeout callbacks.

By swallowing runTimeout/usingRunTimeout instead of forwarding them, every delegate monitor misses the notification, effectively disabling the feature for the standard delegating setup. Please keep the delegation consistent with the rest of the class.

     @Override
     public void runTimeout(String name, RunDuration runDuration) {
-        // do nothing
+        delegates.forEach(d -> d.runTimeout(name, runDuration));
     }
 ...
     @Override
     public void usingRunTimeout(String name, long timeout) {
-        // do nothing
+        delegates.forEach(d -> d.usingRunTimeout(name, timeout));
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c593a1 and f0f552d.

📒 Files selected for processing (8)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/DelegatingEmbedderMonitor.java (3 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/EmbedderControls.java (3 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/EmbedderMonitor.java (3 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/NullEmbedderMonitor.java (3 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/PrintingEmbedderMonitor.java (3 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/embedder/StoryManager.java (6 hunks)
  • jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1 hunks)
  • jbehave-maven-plugin/src/main/java/org/jbehave/mojo/AbstractEmbedderMojo.java (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
jbehave-core/src/main/java/org/jbehave/core/embedder/DelegatingEmbedderMonitor.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)
jbehave-core/src/main/java/org/jbehave/core/embedder/PrintingEmbedderMonitor.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)
jbehave-core/src/main/java/org/jbehave/core/embedder/StoryManager.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)
jbehave-core/src/main/java/org/jbehave/core/embedder/EmbedderMonitor.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)
jbehave-maven-plugin/src/main/java/org/jbehave/mojo/AbstractEmbedderMojo.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)
jbehave-core/src/main/java/org/jbehave/core/embedder/NullEmbedderMonitor.java (1)
jbehave-core/src/main/java/org/jbehave/core/model/RunDuration.java (1)
  • RunDuration (6-39)


public class RunDuration {
private final long timeoutInSecs;
private final Map<String, Long> inProgressStories = new HashMap<>();
Copy link

Choose a reason for hiding this comment

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

why is it needed to track?

Copy link
Author

Choose a reason for hiding this comment

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

to have ability to stop execution of batch and interrupt not finished story

Copy link

Choose a reason for hiding this comment

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

but it's managed in StoryManager, is not it?

@abudevich abudevich force-pushed the run-execution-timeout branch from f0f552d to 7dd3ba5 Compare October 13, 2025 09:42
@abudevich abudevich force-pushed the run-execution-timeout branch from 7dd3ba5 to cc5273d Compare October 13, 2025 13:57
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

Successfully merging this pull request may close these issues.

2 participants