Skip to content

[py][BiDi] use constant for LogLevel #15677

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

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!

    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