[java] custom duration for Actions constructor

[iampopovich]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

according to #12118 feature request, I started to discover the best way to implement duration overriding

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement

---

Description

• Added a new constructor to the Actions class in Selenium that allows specifying a custom Duration for actions.
• Introduced a new private member variable actionDuration to store the specified duration.

---

Changes walkthrough 📝

	Relevant files
Enhancement	Actions.java Add constructor with custom duration to Actions class        java/src/org/openqa/selenium/interactions/Actions.java Added a new constructor to the Actions class that accepts a Duration parameter. Introduced a new private member variable actionDuration to store the duration. +6/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the PR involves a straightforward addition of a new constructor to handle custom durations in the Actions class. The changes are localized and do not involve complex logic changes.
🧪 Relevant tests	No
⚡ Possible issues	Missing Initialization: The new field actionDuration is not initialized in the original constructor, which might lead to null issues if methods relying on this field are called without using the new constructor.
🔒 Security concerns	No

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	✅ Initialize the new variable in all constructors to prevent potential null pointer issues Suggestion Impact: The suggestion to initialize actionDuration in the existing constructor was implemented by modifying the constructor to call another constructor that initializes actionDuration. code diff: public Actions(WebDriver driver) { - this.driver = Require.nonNull("Driver", driver); + this(driver, Duration.ofMillis(250)); Ensure that actionDuration is also initialized in the existing constructor to avoid potential NullPointerException when the duration is used in other methods. java/src/org/openqa/selenium/interactions/Actions.java [60-61] this.driver = Require.nonNull("Driver", driver); +this.actionDuration = Duration.ofMillis(0); // or any default value Suggestion importance[1-10]: 8 Why: This is a crucial suggestion to prevent potential runtime errors due to uninitialized actionDuration in the existing constructor.	8
Best practice	✅ Add validation for the new constructor parameter to ensure it is not null Suggestion Impact: The commit added validation for the duration parameter in the constructor by using Require.nonNull, ensuring it is not null. code diff: - this.driver = Require.nonNull("Driver", driver); + this(driver, Duration.ofMillis(250)); Validate the duration parameter in the new constructor to ensure it is not null, similar to how the driver parameter is validated. java/src/org/openqa/selenium/interactions/Actions.java [64-65] this.driver = Require.nonNull("Driver", driver); -this.actionDuration = duration; +this.actionDuration = Require.nonNull("Duration", duration); Suggestion importance[1-10]: 7 Why: Validating the duration parameter for null values is important for robustness, similar to the existing validation for the driver parameter.	7
Maintainability	✅ Use constructor overloading to avoid code duplication and improve readability Suggestion Impact: The suggestion to use constructor overloading was implemented. The constructor `Actions(WebDriver driver)` now calls `this(driver, Duration.ofMillis(250));`, which reuses the existing constructor `Actions(WebDriver driver, Duration duration)`. code diff: public Actions(WebDriver driver) { - this.driver = Require.nonNull("Driver", driver); + this(driver, Duration.ofMillis(250)); } To improve code readability, consider overloading the existing constructor instead of duplicating the code. java/src/org/openqa/selenium/interactions/Actions.java [63-66] public Actions(WebDriver driver, Duration duration) { - this.driver = Require.nonNull("Driver", driver); + this(driver); // Call the existing constructor this.actionDuration = duration; } Suggestion importance[1-10]: 7 Why: Overloading the constructor to reuse existing code improves maintainability and reduces the risk of future errors in constructor logic.	7
Enhancement	✅ Add a default value for the new parameter to handle null values gracefully Suggestion Impact: The suggestion to add a default value for `actionDuration` if `duration` is null was implemented by modifying the constructor to use a default duration value of 250 milliseconds. code diff: @@ -57,7 +57,7 @@ private Duration actionDuration; public Actions(WebDriver driver) { - this.driver = Require.nonNull("Driver", driver); + this(driver, Duration.ofMillis(250)); Consider adding a default value for actionDuration in the new constructor if duration is null, to provide a fallback mechanism. java/src/org/openqa/selenium/interactions/Actions.java [64-65] this.driver = Require.nonNull("Driver", driver); -this.actionDuration = duration; +this.actionDuration = (duration != null) ? duration : Duration.ofMillis(0); // or any default value Suggestion importance[1-10]: 6 Why: Providing a default value for actionDuration if duration is null is a good enhancement for better fault tolerance.	6

[titusfortner]

Oh, thanks for taking on this one! I'm glad someone is finally looking into it.

Your code doesn't actually show actionDuration getting used anywhere, though? Need to make it so that the default is overridden and used in necessary places.

[pujagani]

Please add tests for the new constructor added.

[iampopovich]

@pujagani I reused two existing tests for the scrollToElement and scrollByAmount methods with a custom Duration. Additionally, I wrote two tests to check the setting of the default Duration and the custom Duration. Will this be enough?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.76%. Comparing base (5c5487f) to head (0ef4c94).
Report is 8 commits behind head on trunk.

❗ Current head 0ef4c94 differs from pull request most recent head f090146

Please upload reports for the commit f090146 to get more accurate results.

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #14085   +/-   ##
=======================================
  Coverage   57.76%   57.76%           
=======================================
  Files          87       87           
  Lines        5413     5413           
  Branches      228      228           
=======================================
  Hits         3127     3127           
  Misses       2058     2058           
  Partials      228      228           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[titusfortner]

Haven't looked at the failures, pushing this one back to the next release