Skip to content

py/test/selenium/webdriver/common/network.py: remove python 2 code#14502

Merged
harsha509 merged 14 commits intoSeleniumHQ:trunkfrom
Delta456:interface_ip
Oct 1, 2024
Merged

py/test/selenium/webdriver/common/network.py: remove python 2 code#14502
harsha509 merged 14 commits intoSeleniumHQ:trunkfrom
Delta456:interface_ip

Conversation

@Delta456
Copy link
Copy Markdown
Member

@Delta456 Delta456 commented Sep 16, 2024 •
edited by qodo-code-review Bot
Loading

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Removes dead Python 2 code from py/test/selenium/webdriver/common/network.py as Python 2 support is now dropped

Motivation and Context

Migrate to Python 3 from Python 2

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement

Description

  • Removed obsolete Python 2 compatibility code from py/test/selenium/webdriver/common/network.py.
  • Simplified the _bytes function to only support Python 3, reflecting the project's migration to Python 3.

Changes walkthrough 📝

Relevant files
Enhancement
network.py
Remove Python 2 compatibility code from network module     

py/test/selenium/webdriver/common/network.py

  • Removed Python 2 compatibility code.
  • Simplified _bytes function to only support Python 3.
    		•  +1/-4     

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

    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review Bot commented Sep 16, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review Bot commented Sep 16, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a context manager for the socket to ensure proper resource management

    Consider using a context manager (with statement) for the socket to ensure it's
    properly closed after use.

    py/test/selenium/webdriver/common/network.py [30-32]

    -sckt = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-return socket.inet_ntoa(
-    fcntl.ioctl(sckt.fileno(), 0x8915, struct.pack("256s", _bytes(ifname[:15], "utf-8")))[20:24]  # SIOCGIFADDR
+with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sckt:
+    return socket.inet_ntoa(
+        fcntl.ioctl(sckt.fileno(), 0x8915, struct.pack("256s", _bytes(ifname[:15], "utf-8")))[20:24]  # SIOCGIFADDR
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Using a context manager for the socket is a best practice that ensures proper resource management and prevents potential resource leaks, making it a valuable improvement.

    		9
    Enhancement
    Move the helper function outside the main function to improve code structure

    Consider moving the _bytes function outside of get_interface_ip to improve
    readability and reduce function nesting.

    py/test/selenium/webdriver/common/network.py [26-32]

    +def _bytes(value, encoding):
+    return bytes(value, encoding)
+
 def get_interface_ip(ifname):
-    def _bytes(value, encoding):
-        return bytes(value, encoding)
-
     sckt = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
     return socket.inet_ntoa(
         fcntl.ioctl(sckt.fileno(), 0x8915, struct.pack("256s", _bytes(ifname[:15], "utf-8")))[20:24]  # SIOCGIFADDR
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Moving the _bytes function outside of get_interface_ip can improve readability and maintainability by reducing nesting, but it is not a critical change.

    		7

    @codecov
    Copy link
    Copy Markdown

    codecov Bot commented Sep 18, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 57.66%. Comparing base (b2ef56a) to head (12d9ccf).
    Report is 41 commits behind head on trunk.

    Additional details and impacted files 
    @@            Coverage Diff             @@
##            trunk   #14502      +/-   ##
==========================================
- Coverage   57.67%   57.66%   -0.02%     
==========================================
  Files          89       89              
  Lines        5578     5586       +8     
  Branches      240      245       +5     
==========================================
+ Hits         3217     3221       +4     
+ Misses       2121     2120       -1     
- Partials      240      245       +5

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @harsha509
    Copy link
    Copy Markdown
    Member

    harsha509 commented Oct 1, 2024

    Test failures seen here are unrelated to the changes in this PR.

    RBE Test failures:

    • //dotnet/test/common:DevTools/DevToolsConsoleTest-edge
    • //dotnet/test/common:DevTools/DevToolsNetworkTest-chrome
    • //java/test/org/openqa/selenium/chrome:ChromeOptionsFunctionalTest-remote

    @harsha509 harsha509 merged commit d1836b2 into SeleniumHQ:trunk Oct 1, 2024
    @Delta456 Delta456 deleted the interface_ip branch November 6, 2024 10:30
    M1troll pushed a commit to M1troll/selenium that referenced this pull request May 14, 2025
    @Delta456 @pujagani
    @harsha509 @M1troll
    py/test/selenium/webdriver/common/network.py: remove python 2 code (S…
    7a91437 
    …eleniumHQ#14502)

* java/cookie: escape cookie values when required

* add xss escaping

* Fix formatting

* use string builder

* py/test/webdriver/network.py: remove python 2 code

* remove java code

* restore

---------

Co-authored-by: Puja Jagani <[email protected]>
Co-authored-by: Sri Harsha <[email protected]>
    @dependabot dependabot Bot mentioned this pull request Sep 18, 2025
    Merged
    @dependabot dependabot Bot mentioned this pull request Oct 1, 2025
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @harsha509 harsha509 harsha509 approved these changes

    Assignees

    No one assigned

    Labels

    P-enhancement PR with a new feature Review effort [1-5]: 1

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @Delta456 @harsha509 @pujagani