Skip to content

Conversation

Delta456
Copy link
Member

@Delta456 Delta456 commented Apr 29, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Use typed string for LogLevel which matches the rest bindings

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement, Tests


Description

  • Introduced LogLevel constants for BiDi log levels

  • Updated tests to use LogLevel constants

  • Improved consistency with other Selenium bindings


Changes walkthrough 📝

Relevant files
Enhancement
log.py
Add LogLevel constants for BiDi logging                                   

py/selenium/webdriver/common/bidi/log.py

  • Added LogLevel class with log level constants
  • Provided documentation for the new class
  • +9/-0     
    Tests
    bidi_script_tests.py
    Update BiDi log tests to use LogLevel constants                   

    py/test/selenium/webdriver/common/bidi_script_tests.py

  • Imported and used LogLevel constants in assertions
  • Replaced string literals with LogLevel usage in tests
  • +4/-3     

    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-devtools Includes everything BiDi or Chrome DevTools related labels Apr 29, 2025
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix issue where JavaScript in link's href is not triggered on click() in Firefox 42.0

    Requires further human verification:

    • Need to verify if the PR's changes have any impact on the Firefox click() behavior, though it appears unrelated

    5678 - Not compliant

    Non-compliant requirements:

    • Fix "Error: ConnectFailure (Connection refused)" when instantiating ChromeDriver

    Requires further human verification:

    • Need to verify if the PR's changes have any impact on ChromeDriver connection issues, though it appears unrelated

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link
    Contributor

    qodo-merge-pro bot commented Apr 29, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @cgoldberg
    Copy link
    Contributor

    LGTM 👍

    Copy link
    Member

    @navin772 navin772 left a comment

    Choose a reason for hiding this comment

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

    LGTM!

    @cgoldberg
    Copy link
    Contributor

    for some reason your branch failed when building the docs.. I can't see anything you changed that would cause that. I just merged trunk into your branch to run CI again.

    @cgoldberg cgoldberg merged commit 1b54248 into SeleniumHQ:trunk May 8, 2025
    17 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-devtools Includes everything BiDi or Chrome DevTools related C-py Python Bindings Review effort 1/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    4 participants