Skip to content

Fix naming in Python cookie examples #2284

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

Merged
merged 1 commit into from
Apr 24, 2025

Conversation

cgoldberg
Copy link
Contributor

@cgoldberg cgoldberg commented Apr 24, 2025

User description

Description

This PR fixes the function names in ./examples/python/tests/interactions/test_cookies.py so they are valid names that PyTest will discover (test_*).

I didn't change anything else, so the line numbers in the documentation should still point to the correct place.

Motivation and Context

These examples were getting skipped in test runs (CI and local) because of this.

Types of changes

  • Code example changed

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Enhancement


Description

  • Renamed cookie example functions to start with test_

  • Ensured PyTest discovers and runs all cookie tests

  • No logic or documentation changes, only function names updated


Changes walkthrough 📝

Relevant files
Enhancement
test_cookies.py
Rename cookie example functions for PyTest compatibility 

examples/python/tests/interactions/test_cookies.py

  • Renamed all cookie-related functions to start with test_
  • Enabled PyTest to discover and execute these example tests
  • No changes to test logic or documentation references
  • +6/-6     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Sorry, something went wrong.

    Copy link

    netlify bot commented Apr 24, 2025

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 1d82395

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add proper test assertions

    Replace print() statements with proper test assertions to verify the cookie was
    correctly set. This ensures the test actually validates the functionality.

    examples/python/tests/interactions/test_cookies.py [12-20]

     def test_get_named_cookie():
         driver = webdriver.Chrome()
    -    driver.get("http://www.example.com")
    +    try:
    +        driver.get("http://www.example.com")
    +    
    +        # Adds the cookie into current browser context
    +        driver.add_cookie({"name": "foo", "value": "bar"})
    +    
    +        # Get cookie details with named cookie 'foo'
    +        cookie = driver.get_cookie("foo")
    +        assert cookie is not None
    +        assert cookie["value"] == "bar"
    +    finally:
    +        driver.quit()
     
    -    # Adds the cookie into current browser context
    -    driver.add_cookie({"name": "foo", "value": "bar"})
    -
    -    # Get cookie details with named cookie 'foo'
    -    print(driver.get_cookie("foo"))
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    __

    Why: Converting print statements to proper assertions transforms this from a demonstration into an actual test that validates functionality. The suggestion also adds resource cleanup, making this a comprehensive improvement that significantly enhances test quality and reliability.

    High
    Add resource cleanup

    Add a teardown step to close the WebDriver after the test completes to prevent
    resource leaks. WebDriver instances should always be properly closed.

    examples/python/tests/interactions/test_cookies.py [4-9]

     def test_add_cookie():
         driver = webdriver.Chrome()
    -    driver.get("http://www.example.com")
    +    try:
    +        driver.get("http://www.example.com")
    +    
    +        # Adds the cookie into current browser context
    +        driver.add_cookie({"name": "key", "value": "value"})
    +    finally:
    +        driver.quit()
     
    -    # Adds the cookie into current browser context
    -    driver.add_cookie({"name": "key", "value": "value"})
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: Adding proper resource cleanup with driver.quit() in a finally block is an important practice to prevent resource leaks and zombie browser processes. This is a significant improvement for test reliability and system resource management.

    Medium
    • More

    Sorry, something went wrong.

    @cgoldberg cgoldberg changed the title [py] Fix naming in cookie examples Fix naming in Python cookie examples Apr 24, 2025
    @titusfortner titusfortner merged commit ea51639 into SeleniumHQ:trunk Apr 24, 2025
    9 checks passed
    @cgoldberg cgoldberg deleted the py-fix-cookie-examples branch April 24, 2025 02:09
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants