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]: move python code to test_locators.py #2102

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

navin772
Copy link
Contributor

@navin772 navin772 commented Dec 16, 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 python code of locators to test_locators.py and

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, Enhancement


Description

  • Added new Python test file test_locators.py with comprehensive test cases for all Selenium locator types
  • Created HTML test page with form elements to support locator testing
  • Added HTTP server fixture in conftest.py to serve test files
  • Updated documentation across multiple languages to reference the new test implementations
  • Improved code examples by linking to actual test implementations instead of inline code

Changes walkthrough 📝

Relevant files
Tests
locators.html
Add HTML test page for locator examples                                   

examples/python/tests/elements/locators.html

  • Added new HTML test page with form elements for testing locators
  • Contains various input fields, radio buttons, checkboxes and links
  • +30/-0   
    test_locators.py
    Add comprehensive locator tests in Python                               

    examples/python/tests/elements/test_locators.py

  • Added test cases for all locator types (class name, CSS, ID, etc.)
  • Implemented WebDriver fixture for browser initialization
  • Added assertions to verify element properties
  • +52/-0   
    Configuration changes
    conftest.py
    Add HTTP server fixture for serving test files                     

    examples/python/tests/conftest.py

  • Added HTTP server fixture to serve test HTML files
  • Implemented server setup and teardown logic
  • +20/-0   
    Documentation
    locators.*.md
    Update documentation to reference new Python test examples

    website_and_docs/content/documentation/webdriver/elements/locators.*.md

  • Updated Python code examples to reference new test file
  • Replaced inline code with references to test implementation
  • Applied changes across multiple language versions (en, ja, pt-br,
    zh-cn)

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

    Copy link

    netlify bot commented Dec 16, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 45aa857

    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
    WebDriver instance is not properly closed if test fails. Consider using try-finally or context manager to ensure driver.quit() is called

    Thread Safety
    HTTP server is started in a daemon thread which may not properly clean up resources. Consider using a non-daemon thread with proper shutdown handling

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Ensure proper server cleanup in case of errors during startup to prevent resource leaks

    Add server cleanup in case of startup failure to prevent resource leaks. The server
    should be shut down if an error occurs during thread startup.

    examples/python/tests/conftest.py [203-206]

    -thread = Thread(target=server.serve_forever, daemon=True)
    -thread.start()
    +try:
    +    thread = Thread(target=server.serve_forever, daemon=True)
    +    thread.start()
    +    yield f"http://localhost:{port}"
    +finally:
    +    server.shutdown()
     
    -yield f"http://localhost:{port}"
    -
    -server.shutdown()
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses an important resource management issue by ensuring server cleanup even if errors occur during startup. This prevents potential resource leaks and improves test reliability.

    8
    Add explicit wait for page load completion to improve test reliability

    Add explicit wait for page load after navigation to ensure elements are available
    before running tests.

    examples/python/tests/elements/test_locators.py [10-12]

     driver = webdriver.Chrome()
     driver.implicitly_wait(0.5)
     driver.get(f"{html_server}/elements/locators.html")
    +WebDriverWait(driver, 10).until(lambda d: d.execute_script('return document.readyState') == 'complete')
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding an explicit wait for page load completion helps prevent flaky tests by ensuring all elements are fully loaded before test execution begins. This is particularly important for element locator tests.

    7

    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.

    HI Navin,

    Thank you for the PR. Could you please look into comments provided.

    Thanks,
    Sri

    Copy link
    Member

    Choose a reason for hiding this comment

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

    We don't place HTML code here. Instead, you can use the static pages deployed on the official Selenium website
    https://www.selenium.dev/selenium/web/

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    Can't find a similar html page, so created a PR to add it in selenium - SeleniumHQ/selenium#14905

    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