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 Examples for PrintOptions() #2000

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

shbenzer
Copy link
Contributor

Added python examples to the printoptions sections of the documentation

Description

added test_print_options.py
added examples to print_options.md in all translations

Motivation and Context

Issue #1941
Fixed conflicts in previous pr
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.

Copy link

netlify bot commented Oct 15, 2024

👷 Deploy request for selenium-dev pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit e0e5892

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1941 - Partially compliant

Fully compliant requirements:

  • Add documentation for Selenium's PrintOptions class

Not compliant requirements:

  • Expand on the use cases for PrintOptions
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incomplete Test Coverage
The test cases cover basic functionality but may not explore edge cases or error scenarios for PrintOptions

Redundant Test
The test_scale function appears to be redundant with test_size, as they both test the scale property

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Best practice
Utilize context managers for better resource management in test fixtures

Use a context manager for the WebDriver to ensure proper resource cleanup.

examples/python/tests/interactions/test_print_options.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]: 7

Why:

7
Use descriptive variable names to enhance code clarity and self-documentation

Use more descriptive variable names to improve code readability.

examples/python/tests/interactions/test_print_options.py [43-46]

 print_options = PrintOptions()
-print_options.scale = 0.5 ## 0.1 to 2.0
-current_scale = print_options.scale
-assert current_scale == 0.5
+expected_scale = 0.5  # Valid range: 0.1 to 2.0
+print_options.scale = expected_scale
+actual_scale = print_options.scale
+assert actual_scale == expected_scale
  • Apply this suggestion
Suggestion importance[1-10]: 7

Why:

7
Maintainability
Use constants for frequently used values to improve code maintainability

Use a constant for the test URL to improve maintainability and reduce duplication.

examples/python/tests/interactions/test_print_options.py [12]

-driver.get("https://www.selenium.dev/")
+SELENIUM_URL = "https://www.selenium.dev/"
+...
+driver.get(SELENIUM_URL)
Suggestion importance[1-10]: 7

Why:

7

💡 Need additional feedback ? start a PR chat

@shbenzer
Copy link
Contributor Author

@harsha1502 use this pr instead of the previous

@shbenzer
Copy link
Contributor Author

shbenzer commented Oct 15, 2024

Also this is PR #2000 - that's fun

@shbenzer
Copy link
Contributor Author

My bad, had wrong assertion

@harsha509 harsha509 merged commit 6703150 into SeleniumHQ:trunk Oct 16, 2024
9 checks passed
selenium-ci added a commit that referenced this pull request Oct 16, 2024
* Added Python Examples for PrintOptions()

* fixed assertion error

* now the assertion error is fixed

---------

Co-authored-by: Sri Harsha <[email protected]> 6703150
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