Skip to content

[py] support for custom error messages through functions for WebDriverWait.until/until_not #15723

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.

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

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

Conversation

Delta456
Copy link
Member

@Delta456 Delta456 commented May 8, 2025

User description

🔗 Related Issues

Implements #14552

💥 What does this PR do?

Allow specifying a callable for the message argument in WebDriverWait.until / until_not

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement, Tests


Description

  • Allow callable as message in WebDriverWait.until/until_not

  • Update method signatures and error handling for callables

  • Add tests for callable and string timeout messages


Changes walkthrough 📝

Relevant files
Enhancement
wait.py
Add callable support for timeout messages in WebDriverWait

py/selenium/webdriver/support/wait.py

  • Accept callable or string for message in until and until_not
  • Update method signatures and docstrings to reflect new type
  • Evaluate callable message at timeout before raising exception
  • Ensure backward compatibility with string messages
  • +14/-6   
    Tests
    webdriverwait_tests.py
    Add tests for callable and string timeout messages             

    py/test/selenium/webdriver/common/webdriverwait_tests.py

  • Add tests for callable and string timeout messages in WebDriverWait
  • Assert correct message appears in TimeoutException
  • +15/-0   

    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-py Python Bindings B-support Issue or PR related to support classes labels May 8, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented May 8, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 147913a)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    14552 - PR Code Verified

    Compliant requirements:

    • Allow specifying a callable for the message argument in WebDriverWait.until / until_not
    • Enable constructing error messages using the driver when an error occurs
    • Make code more readable by avoiding try-catch structures

    Requires further human verification:

    • Support the usage example: WebDriverWait(driver, 10).until(lambda x: x.find_element(By.ID, "someId"), lambda x: f'Message: {x.current_url}') - The implementation accepts callables but the test doesn't verify this exact usage pattern with driver as parameter

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

    Callable Parameter

    The implementation calls the message callable without any parameters, but the ticket suggests passing the driver to the callable. The type hint suggests Any as the parameter type, but the implementation doesn't pass any parameter.

    final_msg = message() if callable(message) else message
    raise TimeoutException(final_msg, screen, stacktrace)
    Test Coverage

    The test uses a lambda without parameters, but doesn't test the case where the callable uses the driver to construct the message, which was the main motivation in the ticket.

        EC.presence_of_element_located((By.ID, "box0")), message=lambda: "custom timeout message"
    )

    @selenium-ci
    Copy link
    Member

    Thank you, @Delta456 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 May 8, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented May 8, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 147913a
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Pass exception to callable

    The current implementation calls the callable message without any arguments, but
    the type hint suggests it should accept an argument. Pass the exception
    information to the callable to allow for more informative error messages.

    py/selenium/webdriver/support/wait.py [150]

    -final_msg = message() if callable(message) else message
    +final_msg = message(exc) if callable(message) else message
    • Apply / Chat
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies a mismatch between the type hint and implementation. The type hint Callable[[Any], str] indicates the callable should accept an argument, but the code calls message() without arguments. Passing the exception object would allow for more informative dynamic error messages.

    High
    • More

    Previous suggestions

    Suggestions up to commit 147913a
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Pass exception to callable message

    The current implementation calls the callable message without any arguments, but
    the type hint suggests it should accept an argument. Pass the exception
    information to the callable to allow for more contextual error messages.

    py/selenium/webdriver/support/wait.py [150]

    -final_msg = message() if callable(message) else message
    +final_msg = message(exc) if callable(message) else message
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies that the callable message is defined to accept a parameter (per the type hint) but is being called without arguments. Passing the exception information would allow for more contextual error messages, which is a significant improvement to the error handling functionality.

    High
    General
    Improve type hint specificity

    The type hint for the message parameter is too generic. Since the callable is
    intended to receive exception information, use a more specific type hint that
    reflects the actual usage pattern.

    py/selenium/webdriver/support/wait.py [19-97]

    -from typing import Any
    +from typing import Any, Optional
     ...
    -message: Union[str, Callable[[Any], str]] = ""
    +message: Union[str, Callable[[Optional[Exception]], str]] = ""
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion correctly points out that the type hint for the message parameter could be more specific. Using Optional[Exception] instead of Any would better document the intended usage pattern, improving code clarity and maintainability, though it doesn't affect runtime behavior.

    Low

    @Delta456 Delta456 reopened this May 8, 2025
    @@ -92,7 +93,9 @@ def __init__(
    def __repr__(self) -> str:
    return f'<{type(self).__module__}.{type(self).__name__} (session="{self._driver.session_id}")>'

    def until(self, method: Callable[[D], Union[Literal[False], T]], message: str = "") -> T:
    def until(
    self, method: Callable[[D], Union[Literal[False], T]], message: Union[str, Callable[[Any], str]] = ""
    Copy link

    Choose a reason for hiding this comment

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

    Thank you for your work.
    I think it would be better to indicate that the Callable cannot take any arguments.

    Suggested change
    self, method: Callable[[D], Union[Literal[False], T]], message: Union[str, Callable[[Any], str]] = ""
    self, method: Callable[[D], Union[Literal[False], T]], message: Union[str, Callable[[], str]] = ""

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-support Issue or PR related to support classes C-py Python Bindings Review effort 2/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants