Skip to content

[dotnet] Add WebDriverWait constructor for specifying a sleep interval without a clock #15838

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

Jump to bottom
Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

[dotnet] Add WebDriverWait constructor for specifying a sleep interval without a clock #15838

wants to merge 1 commit into from

Conversation

RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Jun 1, 2025 •
edited by qodo-merge-pro bot
Loading

User description

🔗 Related Issues

Fixes #15314

💥 What does this PR do?

Adds a constructor to WebDriverWait for specifying a sleep interval without a clock. Use it in a test.

🔧 Implementation Notes

Note that the Java implementation already has this feature.

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

  • Add new WebDriverWait constructor without clock parameter

  • Update usage in PopupWindowFinder to use new constructor

  • Improve API parity with Java bindings

Changes walkthrough 📝

Relevant files
Enhancement
WebDriverWait.cs
Add WebDriverWait constructor without clock parameter       

dotnet/src/webdriver/Support/WebDriverWait.cs

  • Add new constructor to WebDriverWait accepting driver, timeout, and
    sleepInterval
  • Document new constructor and its parameters
  • Calls existing constructor with default clock and sleep timeout
    		• +12/-0   
    PopupWindowFinder.cs
    Use new WebDriverWait constructor in PopupWindowFinder     

    dotnet/src/support/UI/PopupWindowFinder.cs

  • Update to use new WebDriverWait constructor without clock
  • Simplify instantiation of WebDriverWait in popup handling
    		• +1/-1     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @selenium-ci selenium-ci added C-dotnet .NET Bindings B-support Issue or PR related to support classes labels Jun 1, 2025
    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 1, 2025 •
    edited
    Loading

    PR Reviewer Guide 🔍

    (Review updated until commit 56c0618)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    15314 - PR Code Verified

    Compliant requirements:

    • Add a constructor to WebDriverWait that allows specifying a custom sleep interval without requiring an IClock parameter
    • Match the Java binding's functionality which already has this constructor overload

    Requires further human verification:

    • Verify that the implementation correctly matches the Java binding's functionality

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Parameter Mismatch

    The new constructor calls the base constructor with DefaultSleepTimeout instead of the provided sleepInterval parameter, which would ignore the user-specified sleep interval.

     
        : this(SystemClock.Instance, driver, timeout, DefaultSleepTimeout)
{

    @selenium-ci
    Copy link
    Member

    Thank you, @RenderMichael for this code suggestion.

    The support packages contain example code that many users find helpful, but they do not necessarily represent
    the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

    We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
    If you have any questions, please contact us

    @selenium-ci selenium-ci closed this Jun 1, 2025
    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 1, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Latest suggestions up to 56c0618
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix parameter usage

    The constructor is ignoring the provided sleepInterval parameter and using
    DefaultSleepTimeout instead. This defeats the purpose of allowing custom sleep
    intervals. Change the constructor to use the provided sleepInterval parameter.

    dotnet/src/webdriver/Support/WebDriverWait.cs [68-71]

     public WebDriverWait(IWebDriver driver, TimeSpan timeout, TimeSpan sleepInterval)
-    : this(SystemClock.Instance, driver, timeout, DefaultSleepTimeout)
+    : this(SystemClock.Instance, driver, timeout, sleepInterval)
 {
 }
    • Apply / Chat
    Suggestion importance[1-10]: 9

    __

    Why: This is a critical bug where the sleepInterval parameter is ignored and DefaultSleepTimeout is used instead, breaking the constructor's intended functionality. This would cause unexpected behavior in PopupWindowFinder.cs which depends on custom sleep intervals.

    		High
    • More

    Previous suggestions

    Suggestions up to commit 56c0618
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix parameter usage

    The constructor is not using the provided sleepInterval parameter but instead
    uses DefaultSleepTimeout. This defeats the purpose of allowing custom sleep
    intervals. Change the constructor to use the passed sleepInterval parameter.

    dotnet/src/webdriver/Support/WebDriverWait.cs [68-71]

     public WebDriverWait(IWebDriver driver, TimeSpan timeout, TimeSpan sleepInterval)
-    : this(SystemClock.Instance, driver, timeout, DefaultSleepTimeout)
+    : this(SystemClock.Instance, driver, timeout, sleepInterval)
 {
 }
    Suggestion importance[1-10]: 9

    __

    Why: This is a critical bug where the sleepInterval parameter is ignored and DefaultSleepTimeout is used instead, breaking the intended functionality of the new constructor.

    		High

    @RenderMichael RenderMichael reopened this Jun 1, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers
    No reviews
    Assignees
    No one assigned
    Labels
    B-support Issue or PR related to support classes C-dotnet .NET Bindings Review effort 1/5
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    [dotnet] [🚀 Feature] Add WebDriverWait constructor for timeout and interval without a clock
    2 participants
    @RenderMichael @selenium-ci