Skip to content

[java][py]: move java and python firefox profile code #2090

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
Dec 2, 2024

Conversation

navin772
Copy link
Member

@navin772 navin772 commented Dec 2, 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

Moved the java and python firefox profile code.

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

enhancement, documentation


Description

  • Added new tests for Firefox profile configuration in both Java and Python.
  • Updated documentation to reference the new test files for Java and Python examples.
  • Removed inline code examples from documentation and replaced them with links to the new test files.

Changes walkthrough 📝

Relevant files
Enhancement
test_firefox.py
Add Firefox profile test in Python                                             

examples/python/tests/browsers/test_firefox.py

  • Added a new test for Firefox profile configuration.
  • Configured Firefox profile to disable JavaScript.
  • Utilized webdriver.Firefox with the specified options.
  • +14/-0   
    FirefoxTest.java
    Add Firefox profile test in Java                                                 

    examples/java/src/test/java/dev/selenium/browsers/FirefoxTest.java

  • Added a new test for Firefox profile configuration.
  • Configured Firefox profile to disable JavaScript.
  • Utilized FirefoxDriver with the specified options.
  • +12/-0   
    Documentation
    firefox.en.md
    Update Firefox profile documentation with new code references

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

  • Updated Java and Python code examples to reference new test files.
  • Removed inline code examples for Java and Python.
  • +6/-14   
    firefox.ja.md
    Update Firefox profile documentation with new code references

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

  • Updated Java and Python code examples to reference new test files.
  • Removed inline code examples for Java and Python.
  • +7/-15   
    firefox.pt-br.md
    Update Firefox profile documentation with new code references

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

  • Updated Java and Python code examples to reference new test files.
  • Removed inline code examples for Java and Python.
  • +6/-14   
    firefox.zh-cn.md
    Update Firefox profile documentation with new code references

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

  • Updated Java and Python code examples to reference new test files.
  • Removed inline code examples for Java and Python.
  • +6/-14   

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

    Copy link

    netlify bot commented Dec 2, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 78f79d1

    Copy link
    Contributor

    qodo-merge-pro bot commented Dec 2, 2024

    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

    Type Mismatch
    The JavaScript preference value is set as a string "False" but should likely be a boolean false for proper type safety

    Resource Leak
    The driver is created but may not be properly closed if an exception occurs before driver.quit(). Consider using a context manager or try-finally block

    Copy link
    Contributor

    qodo-merge-pro bot commented Dec 2, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Use correct data type when setting browser preferences

    Use boolean value instead of string "False" when setting JavaScript preference to
    ensure correct type matching.

    examples/java/src/test/java/dev/selenium/browsers/FirefoxTest.java [213]

    -profile.setPreference("javascript.enabled", "False");
    +profile.setPreference("javascript.enabled", false);
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using string "False" instead of boolean false could cause incorrect preference setting or type mismatch issues, potentially affecting the JavaScript disabling functionality.

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

    Add a try-finally block to ensure the driver is always closed, even if an exception
    occurs during test execution.

    examples/python/tests/browsers/test_firefox.py [165-167]

     driver = webdriver.Firefox(options=options)
    -driver.quit()
    +try:
    +    # Test code here
    +finally:
    +    driver.quit()
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using try-finally ensures proper resource cleanup even if test execution fails, preventing potential resource leaks and browser process orphaning.

    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 @navin772 !

    @harsha509 harsha509 merged commit 9060cb9 into SeleniumHQ:trunk Dec 2, 2024
    15 checks passed
    selenium-ci added a commit that referenced this pull request Dec 2, 2024
    …site]
    
    move java and python firefox profile code 9060cb9
    @navin772 navin772 deleted the move-firefox-profile-code branch December 2, 2024 17:13
    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