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 more Internet Explorer example code #2097

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Conversation

Delta456
Copy link
Contributor

@Delta456 Delta456 commented Dec 10, 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

Add more code examples for Internet Explorer

Motivation and Context

It was missing so it is required to do

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 new test functions for various Internet Explorer options in Python, enhancing test coverage.
  • Updated documentation across multiple languages (English, Japanese, Portuguese, Chinese) to reference new test cases instead of inline examples.
  • Ensured that tests are conditionally skipped on non-Windows platforms.

Changes walkthrough 📝

Relevant files
Tests
test_internet_explorer.py
Add comprehensive Internet Explorer options tests               

examples/python/tests/browsers/test_internet_explorer.py

  • Added multiple test functions for Internet Explorer options.
  • Tests include file upload timeout, clean session, zoom level, and
    more.
  • Each test is conditionally skipped if not on Windows.
  • +75/-6   
    Documentation
    internet_explorer.en.md
    Update Python examples with test file references                 

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

  • Replaced inline Python code examples with references to test file.
  • Updated documentation to reflect new test cases.
  • +22/-79 
    internet_explorer.ja.md
    Update Japanese Python examples with test file references

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

  • Replaced inline Python code examples with references to test file.
  • Updated documentation to reflect new test cases.
  • +21/-78 
    internet_explorer.pt-br.md
    Update Portuguese Python examples with test file references

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

  • Replaced inline Python code examples with references to test file.
  • Updated documentation to reflect new test cases.
  • +21/-85 
    internet_explorer.zh-cn.md
    Update Chinese Python examples with test file references 

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

  • Replaced inline Python code examples with references to test file.
  • Updated documentation to reflect new test cases.
  • +22/-78 

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

    Copy link

    netlify bot commented Dec 10, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit d3e22fa

    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

    Resource Management
    Multiple test functions create and quit WebDriver instances but don't use context managers (with statements) which could ensure proper cleanup even if tests fail

    Test Coverage
    Tests only verify that options can be set but don't validate if they actually take effect - consider adding assertions to verify the behavior changes

    Copy link
    Contributor

    qodo-merge-pro bot commented Dec 10, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Ensure proper cleanup of browser resources by using try-finally blocks

    Add cleanup of the driver in a try-finally block to ensure the driver is always
    quit, even if an error occurs during test execution.

    examples/python/tests/browsers/test_internet_explorer.py [29-33]

     options = webdriver.IeOptions()
     options.file_upload_timeout = 2000
    -driver = webdriver.Ie(options=options)
    -driver.quit()
    +driver = None
    +try:
    +    driver = webdriver.Ie(options=options)
    +finally:
    +    if driver:
    +        driver.quit()
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding try-finally blocks is important for ensuring proper cleanup of browser resources even if tests fail, preventing resource leaks and browser process orphaning.

    8
    Possible issue
    Add file existence check before reading log files

    Add verification that the log file exists before attempting to read it, to prevent
    FileNotFoundError.

    examples/python/tests/browsers/test_internet_explorer.py [102-103]

    +import os
    +assert os.path.exists(log_path), "Log file was not created"
     with open(log_path, "r") as fp:
         assert "Starting WebDriver server" in fp.readline()
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a file existence check before reading prevents FileNotFoundError exceptions and provides clearer error messages when log files are missing.

    7

    💡 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 @Delta456 !

    @harsha509 harsha509 merged commit eeebb04 into SeleniumHQ:trunk Dec 19, 2024
    7 of 9 checks passed
    selenium-ci added a commit that referenced this pull request Dec 19, 2024
    * [py] add more Internet Explorer example code
    
    * remove extra tag
    
    * fix and skip some code eeebb04
    @Delta456 Delta456 deleted the ie branch December 19, 2024 18:50
    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