Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Added se-config.toml Example to Selenium Manager #1991

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
VietND96 merged 8 commits into SeleniumHQ:trunk from shbenzer:document_selenium_bindings
Nov 7, 2024

Conversation

@shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Oct 12, 2024 •
edited by qodo-merge-pro bot
Loading

User description

Added se-config.toml example to Documentation.

Description

created example_se-config.toml
added example to all translations of Selenium Manager

Motivation and Context

Extends Issue #1496 , will now add what language bindings are available as well.

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

Documentation

Description

  • Added an example se-config.toml file for Selenium Manager with various configuration options.
  • Updated documentation in multiple languages (English, Japanese, Portuguese, Chinese) to include the se-config.toml example.
  • Enhanced documentation to help users understand how to configure Selenium Manager using a TOML file.

Changes walkthrough 📝

Relevant files
Documentation
example_se-config.toml
Add example `se-config.toml` for Selenium Manager

examples/python/tests/selenium_manager/example_se-config.toml

  • Added a new example configuration file for Selenium Manager.
  • Configures various settings like browser, driver, and paths.
  • Includes options for proxy, timeout, and debugging.
    • +21/-0
    selenium_manager.en.md
    Add `se-config.toml` example to English documentation

    website_and_docs/content/documentation/selenium_manager.en.md

  • Added a section for se-config.toml example.
  • Linked to the example configuration file.
    • +7/-0
    selenium_manager.ja.md
    Add `se-config.toml` example to Japanese documentation

    website_and_docs/content/documentation/selenium_manager.ja.md

  • Added a section for se-config.toml example.
  • Linked to the example configuration file.
    • +7/-0
    selenium_manager.pt-br.md
    Add `se-config.toml` example to Portuguese documentation

    website_and_docs/content/documentation/selenium_manager.pt-br.md

  • Added a section for se-config.toml example.
  • Linked to the example configuration file.
    • +7/-0
    selenium_manager.zh-cn.md
    Add `se-config.toml` example to Chinese documentation

    website_and_docs/content/documentation/selenium_manager.zh-cn.md

  • Added a section for se-config.toml example.
  • Linked to the example configuration file.
    • +7/-0

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

    A1exKH reacted with thumbs up emoji
    Copy link

    netlify bot commented Oct 12, 2024 •
    edited
    Loading

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 868df43

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation Review effort [1-5]: 2 labels Oct 12, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Configuration Clarity
    The example configuration file includes both command-line flags and their corresponding TOML keys. This might be confusing for users. Consider removing the command-line flag comments or clearly separating them from the TOML keys.

    Missing Explanation
    The newly added se-config.toml example lacks an explanation or context. Consider adding a brief description of how to use this configuration file and its purpose.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion Score
    Security
    Use a secure HTTPS proxy instead of an insecure HTTP proxy

    The proxy setting is using an insecure HTTP proxy. For better security, consider
    using an HTTPS proxy or removing the proxy setting if it's not required.

    examples/python/tests/selenium_manager/example_se-config.toml [11] 

    -proxy = "myproxy:8080" # --proxy PROXY
+proxy = "https://myproxy:8443" # --proxy PROXY
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Switching to a secure HTTPS proxy significantly improves security, making this suggestion highly relevant and impactful for protecting sensitive data.

    		9
    Use more specific and secure URLs for driver and browser mirrors

    Consider using more specific and secure URLs for the mirror settings. Generic URLs
    like 'https://custom-driver-mirror.com' may not be reliable or secure in a
    production environment.

    examples/python/tests/selenium_manager/example_se-config.toml [6-7] 

    -driver-mirror-url = "https://custom-driver-mirror.com" # --driver-mirror-url DRIVER_MIRROR_URL
-browser-mirror-url = "https://custom-browser-mirror.com" # --browser-mirror-url BROWSER_MIRROR_URL
+driver-mirror-url = "https://selenium-mirror.example.com/drivers" # --driver-mirror-url DRIVER_MIRROR_URL
+browser-mirror-url = "https://selenium-mirror.example.com/browsers" # --browser-mirror-url BROWSER_MIRROR_URL
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion to use more specific and secure URLs for the mirror settings is valid, as it enhances security and reliability, which is crucial for production environments.

    		8
    Best practice
    Disable debug and trace options by default to reduce log verbosity

    The debug and trace options are both set to true, which may generate excessive logs.
    In a production environment, consider setting these to false by default and enabling
    them only when needed for troubleshooting.

    examples/python/tests/selenium_manager/example_se-config.toml [16-17] 

    -debug = true # --debug
-trace = true # --trace
+debug = false # --debug
+trace = false # --trace
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Disabling debug and trace by default is a good practice to minimize log noise in production, though it may be less critical in a configuration file meant for examples.

    		7
    Enhancement
    Reduce the timeout value to prevent unnecessarily long waits

    The timeout value of 300 seconds (5 minutes) might be too long for most operations.
    Consider reducing it to a more reasonable value, such as 60 seconds, to prevent
    tests from hanging unnecessarily.

    examples/python/tests/selenium_manager/example_se-config.toml [12] 

    -timeout = 300 # --timeout TIMEOUT
+timeout = 60 # --timeout TIMEOUT
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Reducing the timeout can enhance test efficiency by preventing long waits, though the optimal timeout value may vary depending on specific use cases.

    		6

    💡 Need additional feedback ? start a PR chat

    Copy link
    Contributor Author

    shbenzer commented Nov 5, 2024

    @VietND96 Fails are due to flaky tests:

    (macos, stable)

    FAILED tests/browsers/test_safari.py::test_basic_options - TypeError: init() missing 1 required positional argument: 'remote_server_addr'
    FAILED tests/browsers/test_safari.py::test_enable_logging - TypeError: init() missing 1 required positional argument: 'remote_server_addr'

    and (macos, nightly)

    FAILED tests/drivers/test_options.py::test_timeouts_page_load - selenium.common.exceptions.TimeoutException: Message: timeout: Timed out receiving message from renderer: -0.050
    (Session info: chrome=130.0.6723.92)
    Stacktrace:
    0 chromedriver 0x000000010074b648 cxxbridge1$str$ptr + 3645404
    1 chromedriver 0x0000000100743ea8 cxxbridge1$str$ptr + 3614780
    2 chromedriver 0x00000001001b0104 cxxbridge1$string$len + 88416
    3 chromedriver 0x000000010019b704 cxxbridge1$string$len + 3936
    4 chromedriver 0x000000010019b48c cxxbridge1$string$len + 3304
    5 chromedriver 0x000000010019983c core::str::slice_error_fail::h1cab30ac4b13c655 + 60316
    6 chromedriver 0x0000000100199f04 core::str::slice_error_fail::h1cab30ac4b13c655 + 62052
    7 chromedriver 0x00000001001a779c cxxbridge1$string$len + 53240
    8 chromedriver 0x00000001001ba600 cxxbridge1$string$len + 130652
    9 chromedriver 0x000000010019a508 core::str::slice_error_fail::h1cab30ac4b13c655 + 63592
    10 chromedriver 0x00000001001ba498 cxxbridge1$string$len + 130292
    11 chromedriver 0x000000010022bb10 cxxbridge1$string$len + 594796
    12 chromedriver 0x00000001001e6f54 cxxbridge1$string$len + 313264
    13 chromedriver 0x00000001001e7ba4 cxxbridge1$string$len + 316416
    14 chromedriver 0x00000001007161e8 cxxbridge1$str$ptr + 3427196
    15 chromedriver 0x000000010071952c cxxbridge1$str$ptr + 3440320
    16 chromedriver 0x00000001006fd60c cxxbridge1$str$ptr + 3325856
    17 chromedriver 0x0000000100719df0 cxxbridge1$str$ptr + 3442564
    18 chromedriver 0x00000001006ee890 cxxbridge1$str$ptr + 3265060
    19 chromedriver 0x0000000100734898 cxxbridge1$str$ptr + 3551788
    20 chromedriver 0x0000000100734a14 cxxbridge1$str$ptr + 3552168
    21 chromedriver 0x0000000100743b40 cxxbridge1$str$ptr + 3613908
    22 libsystem_pthread.dylib 0x0000000183e55f94 _pthread_start + 136
    23 libsystem_pthread.dylib 0x0000000183e50d34 thread_start + 8

    Copy link
    Member

    VietND96 commented Nov 5, 2024 •
    edited
    Loading

    I am thinking that beside the TOML config with explanation, should we add another example TOML so that user can get started quickly in their trial?
    What do you think @harsha1502 @diemol ?

    Copy link

    @A1exKH A1exKH left a comment

    Choose a reason for hiding this comment

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

    @shbenzer Code LGTM.
    Please fix failed checks.

    Copy link
    Member

    I am thinking that beside the TOML config with explanation, should we add another example TOML so that user can get started quickly in their trial? What do you think @harsha1502 @diemol ?

    Hi @VietND96,

    Yes, It would be great if we could provide code samples or boilerplate code to help users get started more easily.

    I also think that the page Selenium Manager Documentation is quite extensive and could benefit from being split into multiple sections. We can consider adding a dedicated "Getting Started" section to make it easier for users to navigate and find essential information.

    Let me know what you think!

    Copy link
    Contributor Author

    shbenzer commented Nov 7, 2024

    @shbenzer Code LGTM. Please fix failed checks.

    contributed code is only a .toml and documentation.

    Copy link
    Member

    VietND96 commented Nov 7, 2024

    I agreed to split it into multiple sections. So we will merge this first and improve later.

    @VietND96 VietND96 merged commit a3018fd into SeleniumHQ:trunk Nov 7, 2024
    8 of 9 checks passed
    Copy link
    Member

    VietND96 commented Nov 7, 2024

    Thank you, @shbenzer !

    selenium-ci added a commit that referenced this pull request Nov 7, 2024 
    [deploy site]
Co-authored-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Co-authored-by: Sri Harsha <12621691+harsha509@users.noreply.github.com> a3018fd
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @VietND96 VietND96 VietND96 approved these changes

    @harsha509 harsha509 Awaiting requested review from harsha509

    @diemol diemol Awaiting requested review from diemol

    @bonigarcia bonigarcia Awaiting requested review from bonigarcia

    +1 more reviewer

    @A1exKH A1exKH A1exKH requested changes

    Reviewers whose approvals may not affect merge requirements

    Assignees

    No one assigned

    Labels

    documentation Improvements or additions to documentation Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    AltStyle によって変換されたページ (->オリジナル) /