You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Correct the right link of Hugo Installation
Add a Python example for the test practices.
Motivation and Context
For the test practices, there's no python example.
Types of changes
Change to the site
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)
I have used hugo server to render the site/docs locally, but failed. Since I only have minor changes in the existing pages, and I am sure it works.
PR Type
Documentation, Tests
Description
Updated the Hugo installation link in the README to point directly to the official Hugo site, improving clarity and accessibility.
Added a detailed Python example using pytest and Selenium to the test practices documentation. This example demonstrates best practices with the "ActionBot" and "LoadableComponent" patterns.
The example includes multiple test cases for interacting with a Todo application, showcasing various functionalities such as adding, toggling, and deleting todos.
Changes walkthrough 📝
Relevant files
Documentation
README.md
Update Hugo installation link and instructions
README.md
Updated the Hugo installation link to a more direct source.
Provided clearer instructions for getting started with Hugo.
Code Complexity The added Python example is quite long and complex. Consider breaking it down into smaller, more manageable sections or separate files for better readability and maintainability.
Error Handling The is_loaded method in the TodoPage class uses a bare except clause, which might catch and suppress unexpected errors. Consider using a more specific exception or at least logging the error.
Commented Code There is commented out code in the hover method of the ActionBot class. Consider removing it if it's not needed or add a comment explaining why it's kept.
✅ Use a context manager for WebDriver to ensure proper resource cleanupSuggestion Impact:The suggestion to use a context manager for the WebDriver was implemented, ensuring proper resource cleanup.
code diff:
+ with webdriver.Chrome() as driver:+ driver.set_window_size(1024, 768)+ driver.implicitly_wait(0.5)+ yield driver
Consider using a context manager for the WebDriver to ensure proper resource cleanup, even if an exception occurs during the test execution.
Why: This suggestion is a best practice for resource management, ensuring that the WebDriver is properly closed even if an exception occurs, which is crucial for preventing resource leaks.
9
Use specific exception handling instead of a bare except clause
In the is_loaded method, consider using a more specific exception catch instead of a bare except clause to handle potential errors more gracefully and avoid masking unexpected exceptions.
Why: Using specific exceptions improves error handling by avoiding the masking of unexpected exceptions, which is a significant improvement in code robustness.
8
✅ Add assertions to verify initial state in test methodsSuggestion Impact:An assertion was added to verify the initial state of the todo count in the test method, which aligns with the suggestion to enhance test reliability by ensuring the test environment is correctly set up.
In the test methods, consider adding assertions to verify the initial state before performing actions. This helps ensure that the test environment is in the expected state and can catch potential setup issues.
def test_new_todo(self, page: TodoPage):
+ assert page.todo_count() == 0, "Initial todo count should be 0"
page.new_todo("aaa")
assert page.count_todo_items_left() == page.build_count_todo_left(1)
Apply this suggestion
Suggestion importance[1-10]: 7
Why: Adding assertions for initial state verification enhances test reliability by ensuring the test environment is correctly set up, which is a good practice for test accuracy.
7
Enhancement
✅ Convert static method to instance method for consistency and flexibilitySuggestion Impact:The static method build_count_todo_left was converted to an instance method in the TodoPage class.
Instead of using a static method for build_count_todo_left, consider making it an instance method to improve consistency with other methods in the class and allow for potential future customization.
Why: While converting the static method to an instance method can improve consistency, it is not critical to the functionality and mainly enhances potential future flexibility.
As suggested,
1. Break the long and complex example down into smaller, more manageable sections or separate files for better readability and maintainability
2. Use a `with` context manager for WebDriver to ensure proper resource cleanup
3. Use specific exception handling instead of a bare except clause
4. Convert static method to instance method for consistency and flexibility
5. Add assertions to verify initial state in test methods
6. Remove the commented code in the `hover` method
PeteSong
changed the title
Petesong/Add a Python example for the test practice
petesong/Add a Python example for the test practice
Sep 5, 2024
Category Suggestion Score Best practice
✅ Use a context manager for WebDriver to ensure proper resource cleanup
9
Use specific exception handling instead of a bare except clause
8
✅ Add assertions to verify initial state in test methods
7 Enhancement
✅ Convert static method to instance method for consistency and flexibility
5
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Key issues to review
Code ComplexityThe added Python example is quite long and complex. Consider breaking it down into smaller, more manageable sections or separate files for better readability and maintainability.
Error HandlingThe is_loaded method in the TodoPage class uses a bare except clause, which might catch and suppress unexpected errors. Consider using a more specific exception or at least logging the error.
Commented CodeThere is commented out code in the hover method of the ActionBot class. Consider removing it if it's not needed or add a comment explaining why it's kept.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Motivation and Context
For the test practices, there's no python example.
Types of changes
Checklist
hugo server
to render the site/docs locally, but failed. Since I only have minor changes in the existing pages, and I am sure it works.PR Type
Documentation, Tests
Description
Changes walkthrough 📝
README.md
Update Hugo installation link and instructions
README.md
design_strategies.en.md
Add Python example for test practices with Selenium
website_and_docs/content/documentation/test_practices/design_strategies.en.md