SeleniumHQ / selenium

A browser automation framework and ecosystem.
https://selenium.dev
Apache License 2.0
29.73k stars 8.02k forks source link

Fix typing in remote webdriver #14166

Open M1troll opened 1 week ago

M1troll commented 1 week ago

User description

Description

The main change is changing the return type hint for def get_downloadable_files(self) from dict to List[str], which is actually what is returned.

Also some cosmetic improvements have been added.

Motivation and Context

I caught some error from my static type checker () and saw that get_downloadable_files has the wrong type annotation and decided to fix it =) image

Types of changes

Checklist


PR Type

Bug fix, Enhancement


Description


Changes walkthrough 📝

Relevant files
Enhancement
webdriver.py
Fix type hints and improve code formatting in `webdriver.py`

py/selenium/webdriver/remote/webdriver.py
  • Changed the return type hint for get_downloadable_files from dict to
    List[str].
  • Improved import statements formatting.
  • Updated type hints for several methods.
  • Applied minor cosmetic improvements and formatting changes.
  • +23/-31 

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

    codiumai-pr-agent-pro[bot] commented 1 week ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The method get_downloadable_files was changed from returning a dict to List[str]. Ensure that the actual implementation of this method and its usage across the project align with this change to avoid runtime errors.
    Code Consistency:
    The PR includes various cosmetic changes such as reformatting import statements and adjusting docstrings. While these changes improve readability, ensure they adhere to the project's coding standards.
    codiumai-pr-agent-pro[bot] commented 1 week ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add validation for the script parameter to ensure it is a non-empty string ___ **The pin_script method should validate the script parameter to ensure it is a non-empty
    string before proceeding.** [py/selenium/webdriver/remote/webdriver.py [375]](https://github.com/SeleniumHQ/selenium/pull/14166/files#diff-c87c150ed2a08f5293db58a06da8bf7f14c3a5582586c6270c1be661dfdd0985R375-R375) ```diff +if not isinstance(script, str) or not script.strip(): + raise ValueError("Script must be a non-empty string") script_key_instance = ScriptKey(script_key) ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: This is a valid suggestion to ensure robustness of the `pin_script` method by validating input, although it's not a critical bug fix.
    7
    Add validation for the name parameter to ensure it is a non-empty string ___ **The get_cookie method should validate the name parameter to ensure it is a non-empty
    string before proceeding.** [py/selenium/webdriver/remote/webdriver.py [586]](https://github.com/SeleniumHQ/selenium/pull/14166/files#diff-c87c150ed2a08f5293db58a06da8bf7f14c3a5582586c6270c1be661dfdd0985R586-R586) ```diff +if not isinstance(name, str) or not name.strip(): + raise ValueError("Cookie name must be a non-empty string") """Get a single cookie by name. Returns the cookie if found, None if ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: This suggestion correctly identifies a potential improvement in input validation for the `get_cookie` method, enhancing the method's robustness.
    7