Skip to content
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

[py]: add firefox context example for python #2050

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

navin772
Copy link
Contributor

@navin772 navin772 commented Nov 8, 2024

User description

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

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

Description

Firefox context example for python

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

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

Tests, Documentation


Description

  • Added a new test test_set_context in the Python test suite for Firefox, which demonstrates context switching to Chrome and back to content.
  • Updated documentation across multiple languages (English, Japanese, Portuguese, Chinese) to include a reference to the new Python code example.

Changes walkthrough 📝

Relevant files
Tests
test_firefox.py
Add Firefox context switching test in Python                         

examples/python/tests/browsers/test_firefox.py

  • Added a new test test_set_context for Firefox.
  • Utilizes the context method to switch to Chrome context.
  • Verifies the context switch back to content.
  • +10/-0   
    Documentation
    firefox.en.md
    Update Python code block reference in English docs             

    website_and_docs/content/documentation/webdriver/browsers/firefox.en.md

    • Updated Python tab to include a code block reference.
    +3/-3     
    firefox.ja.md
    Update Python code block reference in Japanese docs           

    website_and_docs/content/documentation/webdriver/browsers/firefox.ja.md

    • Updated Python tab to include a code block reference.
    +3/-3     
    firefox.pt-br.md
    Update Python code block reference in Portuguese docs       

    website_and_docs/content/documentation/webdriver/browsers/firefox.pt-br.md

    • Updated Python tab to include a code block reference.
    +3/-3     
    firefox.zh-cn.md
    Update Python code block reference in Chinese docs             

    website_and_docs/content/documentation/webdriver/browsers/firefox.zh-cn.md

    • Updated Python tab to include a code block reference.
    +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    netlify bot commented Nov 8, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit d298415

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Assertion Improvement
    The assertion in the test_set_context function could be more specific. Instead of using execute("GET_CONTEXT"), consider using a dedicated method for getting the current context if available.

    Error Handling
    The test_set_context function doesn't include any error handling or cleanup. Consider adding a try-finally block to ensure the driver is properly closed, even if an exception occurs.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure proper resource cleanup by adding a teardown step to quit the driver after the test

    Consider adding a teardown step to quit the driver after the test, ensuring proper
    resource cleanup.

    examples/python/tests/browsers/test_firefox.py [146-153]

     def test_set_context(firefox_driver):
         driver = firefox_driver
     
    -    with driver.context(driver.CONTEXT_CHROME):
    -        driver.execute_script("console.log('Inside Chrome context');")
    +    try:
    +        with driver.context(driver.CONTEXT_CHROME):
    +            driver.execute_script("console.log('Inside Chrome context');")
     
    -    # Check if the context is back to content
    -    assert driver.execute("GET_CONTEXT")["value"] == "content"
    +        # Check if the context is back to content
    +        assert driver.execute("GET_CONTEXT")["value"] == "content"
    +    finally:
    +        driver.quit()
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a teardown step to quit the driver is a good practice for resource management and test isolation. It ensures that the browser session is properly closed after each test, preventing potential issues in subsequent tests.

    7
    Enhancement
    Improve test readability and debugging by adding a descriptive assertion message

    Consider adding a more descriptive assertion message to provide better context in
    case of test failure.

    examples/python/tests/browsers/test_firefox.py [152-153]

     # Check if the context is back to content
    -assert driver.execute("GET_CONTEXT")["value"] == "content"
    +assert driver.execute("GET_CONTEXT")["value"] == "content", "Context did not switch back to content after exiting Chrome context"
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Adding a descriptive assertion message improves the test's readability and makes debugging easier if the test fails. While it's a good practice, it has a moderate impact on the overall test quality and functionality.

    5

    💡 Need additional feedback ? start a PR chat

    Copy link
    Member

    @harsha509 harsha509 left a comment

    Choose a reason for hiding this comment

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

    Thank you @navin772 !

    @navin772
    Copy link
    Contributor Author

    navin772 commented Nov 8, 2024

    The safari stable tests are failing due to an issue in selenium - SeleniumHQ/selenium#14698

    @harsha509 harsha509 merged commit 88c7b5c into SeleniumHQ:trunk Nov 8, 2024
    8 of 9 checks passed
    selenium-ci added a commit that referenced this pull request Nov 8, 2024
    @navin772 navin772 deleted the firefox_context_py branch November 8, 2024 17:04
    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.

    2 participants