Skip to content

[py] Use ruff for linting and code formatting #15746

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 22 commits into
base: trunk
Choose a base branch
from

Conversation

cgoldberg
Copy link
Contributor

@cgoldberg cgoldberg commented May 16, 2025

User description

💥 What does this PR do?

As discussed here:
#13679 (comment)

This PR replaces the current Python linters (black, isort, docformatter, autoflake, flake8) with ruff:

ruff is a linter and formatter that provides all of the functionality of the old linters, plus:

  • it is insanely fast (written in rust). It can lint and format our entire Python codebase in about 30 milliseconds on my crappy Chromebook
  • it consolidates our tooling
  • it has a lot of linting rules and configuration options that aren't available in the old tools that we can enable in the future
  • it is under very active development and hugely popular in the Python community
  • it will allow us better integration with Bazel

🔧 Implementation Notes

Previously, we had 2 tox environments: linting-ci and linting, for use in CI and for local development. This PR consolidates that into a single environment (linting) that can be used for both. I adjusted the ci-python.yaml GHA workflow to use this.

This PR also fixes all of the linting and formatting violations across the codebase that were fixed using the new tooling.

💡 Additional Considerations

Now we should setup ruff to be used hermetically via Bazel. (@p0deje has graciously volunteered to do that work)

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Build Infrastructure

PR Type

Enhancement, Other


Description

  • Replace multiple Python linters with ruff for linting/formatting

  • Remove isort, black, autoflake, flake8, docformatter configs and dependencies

  • Update CI workflow and tox to use ruff exclusively

  • Apply ruff-based formatting and linting fixes across codebase


Changes walkthrough 📝

Relevant files
Configuration changes
4 files
ci-python.yml
Update CI to use ruff for Python linting                                 
+1/-3     
pyproject.toml
Remove old linter configs, add ruff configuration               
+16/-35 
tox.ini
Remove old linting environments, add ruff to linting         
+3/-38   
format.sh
Switch Python formatting to ruff in script                             
+1/-1     
Documentation
16 files
README.md
Update Python linting instructions to use ruff                     
+3/-7     
webdriver.py
Update docstring examples to use double quotes, ruff formatting
+7/-6     
action_chains.py
Update docstring examples to use double quotes, ruff formatting
+3/-2     
action_builder.py
Update docstring examples to use double quotes, ruff formatting
+2/-2     
alert.py
Update docstring examples to use double quotes, ruff formatting
+1/-1     
by.py
Update docstring examples to use double quotes, ruff formatting
+8/-8     
desired_capabilities.py
Update docstring examples to use double quotes, ruff formatting
+3/-4     
firefox_profile.py
Update docstring example to use double quotes, ruff formatting
+6/-4     
webdriver.py
Update docstring examples to use double quotes, ruff formatting
+4/-4     
switch_to.py
Update docstring examples to use double quotes, ruff formatting
+3/-3     
webdriver.py
Update docstring examples to use double quotes, ruff formatting
+21/-20 
webelement.py
Update docstring examples to use double quotes, ruff formatting
+11/-11 
color.py
Update docstring examples to use double quotes, ruff formatting
+7/-4     
expected_conditions.py
Update docstring examples to use double quotes, ruff formatting
+33/-39 
relative_locator.py
Update warnings and docstring examples, ruff formatting   
+2/-4     
wait.py
Update docstring examples to use double quotes, ruff formatting
+1/-2     
Formatting
23 files
conf.py
Apply ruff formatting to Sphinx config                                     
+78/-66 
release-selenium.py
Apply ruff formatting (add blank lines)                                   
+2/-0     
browsing_context.py
Apply ruff formatting to docstrings and line wrapping       
+4/-2     
driver_finder.py
Use consistent double quotes in error messages, ruff formatting
+2/-2     
log.py
Use consistent double quotes in f-strings, ruff formatting
+1/-1     
service.py
Apply ruff formatting to docstrings and line wrapping       
+2/-1     
event_firing_webdriver.py
Apply ruff formatting to docstrings and line wrapping       
+5/-1     
service.py
Apply ruff formatting to docstrings and line wrapping       
+4/-2     
service.py
Apply ruff formatting to docstrings and line wrapping       
+4/-2     
bidi_network_tests.py
Remove blank lines, ruff formatting                                           
+0/-3     
bidi_storage_tests.py
Remove blank lines, ruff formatting                                           
+0/-1     
executing_async_javascript_tests.py
Use parentheses for long assert statements, ruff formatting
+6/-6     
executing_javascript_tests.py
Use single string for JS dict, ruff formatting                     
+1/-1     
interactions_tests.py
Add blank line after docstring, ruff formatting                   
+1/-0     
interactions_with_device_tests.py
Add blank line after docstring, ruff formatting                   
+1/-0     
virtual_authenticator_tests.py
Use string concatenation for long JS, ruff formatting       
+2/-1     
w3c_interaction_tests.py
Use single line for chained calls, ruff formatting             
+1/-3     
webdriverwait_tests.py
Use string concatenation for long JS, ruff formatting       
+2/-1     
webserver.py
Add blank line after docstring, ruff formatting                   
+1/-0     
window_tests.py
Use line continuation for lambda, ruff formatting               
+2/-1     
launcher_tests.py
Use parentheses for long assert, ruff formatting                 
+3/-3     
event_firing_webdriver_tests.py
Use single string for assert, ruff formatting                       
+1/-3     
error_handler_tests.py
Use string concatenation for long error message, ruff formatting
+7/-1     
Additional files
1 files
session.py +0/-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-py Python Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related B-support Issue or PR related to support classes labels May 16, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented May 16, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 95b5594)

    Here are some key observations to aid the review process:

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

    String Concatenation

    The RGBA_PCT_PATTERN regex is constructed using string concatenation which could be simplified to a single string for better readability.

        r"^\s*rgba\(\s*(\d{1,3}|\d{1,2}\.\d+)%\s*,\s*(\d{1,3}|\d{1,2}\.\d+)%\s*,"
        + r"\s*(\d{1,3}|\d{1,2}\.\d+)%\s*,\s*(0|1|0\.\d+)\s*\)\s*$"
    )

    Copy link
    Contributor

    qodo-merge-pro bot commented May 16, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 95b5594
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix string quote nesting

    The CSS selector uses single quotes inside an f-string that's already using
    single quotes, which could lead to syntax errors. Use double quotes for the
    outer string or escape the inner quotes properly.

    py/selenium/webdriver/common/log.py [93]

    -elements: list = self.driver.find_elements(By.CSS_SELECTOR, f'*[data-__webdriver_id="{payload["target"]}"]')
    +elements: list = self.driver.find_elements(By.CSS_SELECTOR, f"*[data-__webdriver_id=\"{payload['target']}\"]")
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion addresses a minor code style/readability issue by clarifying string quoting in an f-string, but the original code is already valid Python and unlikely to cause errors. The improvement is correct but has only a small impact.

    Low
    • Update

    Previous suggestions

    ✅ Suggestions up to commit 95b5594
    CategorySuggestion                                                                                                                                    Impact
    Learned
    best practice
    Consistent string quote style

    Use double quotes consistently in f-strings to match the project's style. The PR
    is updating string literals to use double quotes throughout the codebase, but
    these f-strings still use single quotes.

    py/selenium/webdriver/common/driver_finder.py [71-75]

    -raise ValueError(f'The driver path is not a valid file: {output["driver_path"]}')
    +raise ValueError(f"The driver path is not a valid file: {output['driver_path']}")
     ...
    -raise ValueError(f'The browser path is not a valid file: {output["browser_path"]}')
    +raise ValueError(f"The browser path is not a valid file: {output['browser_path']}")
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Use f-strings with double quotes for string formatting to maintain consistency

    Low
    Possible issue
    Fix unbalanced parentheses
    Suggestion Impact:The commit implemented the suggestion by restructuring the code example in the docstring to have proper parentheses alignment. The closing parenthesis is now properly aligned with the opening one, making the code more readable and following better style practices.

    code diff:

    -    >>> is_text_in_element = WebDriverWait(driver, 10).until(EC.text_to_be_present_in_element(
    -            (By.CLASS_NAME, "foo"), "bar")
    +    >>> is_text_in_element = WebDriverWait(driver, 10).until(
    +            EC.text_to_be_present_in_element((By.CLASS_NAME, "foo"), "bar")
             )

    The parentheses in this example are not properly balanced. The closing
    parenthesis for the method call is placed on a new line but is not aligned with
    the opening parenthesis, which can lead to syntax errors or confusion.

    py/selenium/webdriver/support/expected_conditions.py [391-393]

    ->>> is_text_in_element = WebDriverWait(driver, 10).until(EC.text_to_be_present_in_element(
    -        (By.CLASS_NAME, "foo"), "bar")
    -    )
    +>>> is_text_in_element = WebDriverWait(driver, 10).until(
    +...     EC.text_to_be_present_in_element((By.CLASS_NAME, "foo"), "bar")
    +... )
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion correctly identifies a minor style/readability issue with the placement of parentheses in a docstring example, but it does not affect the actual functionality or correctness of the code. The improved code is clearer and more idiomatic, but the original would still work as a docstring example in most cases.

    Low

    Copy link
    Contributor

    @shbenzer shbenzer left a comment

    Choose a reason for hiding this comment

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

    LGTM

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related 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.

    4 participants