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

Added Python Example to PrintPage Documentation #2009

Merged
merged 2 commits into from
Oct 27, 2024

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Oct 22, 2024

User description

Added python example to Printing in Print Options section

Description

added test_print_page.py
added code example to all translations of print_option.md

Motivation and Context

code example
hacktober

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


Description

  • Added a new Python test file to demonstrate the print_page() functionality using Selenium.
  • Updated the documentation in multiple languages (English, Japanese, Portuguese, Chinese) to include the new Python code example.
  • Linked the documentation to the newly added Python test example for better clarity and usability.

Changes walkthrough 📝

Relevant files
Tests
test_prints_page.py
Add Python test for print page functionality                         

examples/python/tests/interactions/test_prints_page.py

  • Added a new test file for printing a page using Selenium.
  • Implemented a fixture for setting up and tearing down the WebDriver.
  • Created a test function to verify the print page functionality.
  • +15/-0   
    Documentation
    print_page.en.md
    Update English documentation with Python print page example

    website_and_docs/content/documentation/webdriver/interactions/print_page.en.md

  • Added Python code example for print_page() function.
  • Linked to the new Python test example in the documentation.
  • +3/-2     
    print_page.ja.md
    Update Japanese documentation with Python print page example

    website_and_docs/content/documentation/webdriver/interactions/print_page.ja.md

  • Added Python code example for print_page() function.
  • Linked to the new Python test example in the documentation.
  • +3/-2     
    print_page.pt-br.md
    Update Portuguese documentation with Python print page example

    website_and_docs/content/documentation/webdriver/interactions/print_page.pt-br.md

  • Added Python code example for print_page() function.
  • Linked to the new Python test example in the documentation.
  • +3/-2     
    print_page.zh-cn.md
    Update Chinese documentation with Python print page example

    website_and_docs/content/documentation/webdriver/interactions/print_page.zh-cn.md

  • Added Python code example for print_page() function.
  • Linked to the new Python test example in the documentation.
  • +3/-2     

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

    Copy link

    netlify bot commented Oct 22, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 6e66612

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation tests Review effort [1-5]: 2 labels Oct 22, 2024
    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

    Test Improvement
    The test function only checks if the PDF has content, but doesn't verify the actual content or format of the printed page. Consider adding more specific assertions to ensure the printed page contains expected elements.

    Resource Management
    The driver fixture doesn't use a context manager or try-finally block to ensure the driver is always closed, even if an exception occurs during the test.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a context manager for WebDriver to ensure proper resource cleanup

    Consider using a context manager for the WebDriver to ensure proper resource
    cleanup, even if an exception occurs during the test.

    examples/python/tests/interactions/test_prints_page.py [5-9]

     @pytest.fixture()
     def driver():
    -    driver = webdriver.Chrome()
    -    yield driver
    -    driver.quit()
    +    with webdriver.Chrome() as driver:
    +        yield driver
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using a context manager for the WebDriver is a best practice that ensures proper resource cleanup, even if an exception occurs. This change enhances the reliability and maintainability of the test setup.

    8
    Enhancement
    Add error handling for the page printing operation to improve test robustness

    Add error handling to catch potential exceptions when printing the page, as network
    issues or page loading problems could cause the print operation to fail.

    examples/python/tests/interactions/test_prints_page.py [11-15]

     def test_prints_page(driver):
         driver.get("https://www.selenium.dev/")
         print_options = PrintOptions()
    -    pdf = driver.print_page(print_options)
    -    assert len(pdf) > 0
    +    try:
    +        pdf = driver.print_page(print_options)
    +        assert len(pdf) > 0
    +    except Exception as e:
    +        pytest.fail(f"Failed to print page: {e}")
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding error handling for the print operation improves the robustness of the test by catching potential exceptions due to network issues or page loading problems, providing clearer test failure messages.

    7
    Enhance test assertions to verify specific content or structure of the printed PDF

    Consider adding more specific assertions to verify the content or structure of the
    PDF, not just its length, to ensure the printed page contains the expected
    information.

    examples/python/tests/interactions/test_prints_page.py [11-15]

     def test_prints_page(driver):
         driver.get("https://www.selenium.dev/")
         print_options = PrintOptions()
         pdf = driver.print_page(print_options)
         assert len(pdf) > 0
    +    # Add more specific assertions, e.g.:
    +    # assert "Selenium" in pdf.decode('utf-8')
    +    # assert pdf.startswith(b'%PDF')  # Check if it's a valid PDF
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding more specific assertions to verify the content or structure of the PDF would improve the test's effectiveness by ensuring the printed page contains the expected information, though it requires additional implementation details.

    6

    💡 Need additional feedback ? start a PR chat

    @shbenzer
    Copy link
    Contributor Author

    Part of issue #1941

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

    @shbenzer
    Copy link
    Contributor Author

    Looks like that failure is from move_mouse_by_offset, not my code luckily lol

    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.

    Code LGTM.
    There is 1 failed check for this PR, but I don't see any affected code in this PR.

    @harsha509 harsha509 merged commit 430587a into SeleniumHQ:trunk Oct 27, 2024
    9 checks passed
    selenium-ci added a commit that referenced this pull request Oct 27, 2024
    Co-authored-by: Sri Harsha <12621691+harsha509@users.noreply.github.com> 430587a
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants