Expand documentation on fresh browser per test #1671

jflorez · 2024-04-15T01:07:10Z

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

Added an extended explanation and code examples on how to implement a fresh browser per test. Tried to avoid specific framework code like JUnit vs TestNG.

Motivation and Context

I see a lot of Selenium automation project in the industry with a static WebDriver that gets shared between tests and across all these teams there has always been confusion on how to get started with a fresh browser per test. This is made more prominent with requirements to run tests is parallel and a lot of automation coders not understanding the implications a static driver (without ThreadLocal which brings its own complications) has in attempts to run parallel tests in multi-threaded languages like Java or C#

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.

Type

documentation, enhancement

Description

Added comprehensive examples in Java and Python across English, Japanese, Portuguese, and Chinese documentation for setting up a fresh browser instance for each test.
Discussed the drawbacks of using a static WebDriver, especially in the context of parallel testing, and recommended using ThreadLocal for maintaining thread safety.
Aimed at clarifying best practices for test isolation and reducing confusion around setting up WebDriver instances for Selenium tests.

Changes walkthrough

Relevant files

Documentation

fresh_browser_per_test.en.md `Enhance Documentation with Fresh Browser Per Test Examples` website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md Added detailed explanation and Java, Python code examples for implementing a fresh browser per test. Highlighted issues with using a static WebDriver and recommended using ThreadLocal for thread safety.	+54/-1
fresh_browser_per_test.ja.md `Add Fresh Browser Per Test Examples in Japanese Documentation` website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.ja.md Added Java code examples for setting up a fresh browser per test. Discussed the drawbacks of using a static WebDriver in parallel test execution.	+50/-1
fresh_browser_per_test.pt-br.md `Update Portuguese Documentation with Browser Per Test Strategy` website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.pt-br.md Introduced Java and Python examples for initiating a new browser instance per test. Explained the complications of using static WebDriver with ThreadLocal for thread safety.	+49/-1
fresh_browser_per_test.zh-cn.md `Enhance Chinese Documentation with New Browser Per Test Examples` website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.zh-cn.md Provided Java, Python examples for fresh browser setup per test. Addressed the issue of using static WebDriver and the importance of ThreadLocal in test isolation.	+49/-1

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

…o patch-1

netlify · 2024-04-15T01:07:13Z

👷 Deploy request for selenium-dev pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	`8d72b74`

CLAassistant · 2024-04-15T01:07:16Z

All committers have signed the CLA.

codiumai-pr-agent-pro · 2024-04-15T01:07:34Z

PR Description updated to latest commit (7f2aa0a)

Copy walkthrough table to "Files Changed" Tab

codiumai-pr-agent-pro · 2024-04-15T01:07:43Z

PR Review

⏱️ Estimated effort to review [1-5]	2, because the PR primarily focuses on expanding documentation with code examples across multiple languages. The changes are straightforward and involve adding explanations and code snippets to illustrate the concept of using a fresh browser for each test. The complexity is low, and the main effort would be in verifying the correctness and clarity of the examples provided.
🧪 Relevant tests	No
🔍 Possible issues	Clarity: The documentation could benefit from a brief explanation of why using a static WebDriver can lead to issues, especially for readers who might not be familiar with concepts like thread safety and parallel test execution.
🔍 Possible issues	Consistency: Ensure that the code examples provided are consistent across different language versions of the documentation. This includes not only the code syntax but also the comments and explanations that accompany them.
🔒 Security concerns	No

✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

codiumai-pr-agent-pro · 2024-04-15T01:08:12Z

PR Code Suggestions

Category	Suggestions
Maintainability	Improve code indentation for better readability. Consider using proper indentation to enhance code readability. In Java code blocks, it's common practice to use 4 spaces or a tab for indentation. The inconsistent indentation in the `BaseTest` class and `ThreadLocal` example makes the code harder to read. website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md [22-64] public abstract class BaseTest { -protected WebDriver driver; -... + protected WebDriver driver; + ...
Bug	Correct variable reference in Python fixture to prevent AttributeError. In the Python fixture example, there's a mistake where `yield self.driver` is used instead of `yield driver`. This could lead to an AttributeError since `self.driver` is not defined within the fixture's scope. website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md [48] -yield self.driver +yield driver
Enhancement	Clarify the use of `ThreadLocal` for parallel test execution. For the `ThreadLocal` example, consider explaining the necessity of using `ThreadLocal` for WebDriver instances in the context of parallel test execution. This will help readers understand why `ThreadLocal` is used instead of a simpler approach. website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md [59] -This forces the ThreadLocal<WebDriver> variable to call driver.get() every time the driver wants to be used. +This forces the ThreadLocal<WebDriver> variable to call driver.get() every time the driver wants to be used, which is essential for thread-safe operations during parallel test execution.
Enhancement	Enhance Python fixture with error handling for improved debugging. To enhance the Python fixture example for better test isolation and debugging, consider adding a try-except block around the yield statement. This can be used to catch and log exceptions from the test, potentially adding screenshots or additional debugging information before the browser quits. website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md [48-51] -yield driver -driver.quit() +try: + yield driver +finally: + # Optional: Add error handling or screenshots here + driver.quit()
Best practice	Ensure WebDriver is properly closed with a finally block. To avoid potential resource leaks, it's a good practice to wrap the `driver.quit()` call in a finally block within the `teardownTest` method. This ensures that the WebDriver is properly closed even if an exception occurs. website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md [33-35] public void teardownTest() { - ... - driver.quit(); + try { + ... + } finally { + driver.quit(); + } }

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

diemol

Thank you for improving our docs, @jflorez!

We are trying to move away from code in markdown files as it is hard to maintain. We prefer to link the actual code through the Hugo shortcode. Here is an example.

diemol · 2024-04-15T15:56:47Z

website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md

+@pytest.fixture(autouse=True, scope='function')
+def driver(self, request, page: Page):
+    # Create the driver
+    driver = webdriver.Firefox()
+
+    # Return control to the test 
+    yield self.driver
+
+    # Test ends driver quits
+    driver.quit()
+```


There is already a code sample for this: https://github.com/SeleniumHQ/seleniumhq.github.io/blob/trunk/examples/python/tests/conftest.py#L15-L21

diemol · 2024-04-15T15:58:50Z

website_and_docs/content/documentation/test_practices/encouraged/fresh_browser_per_test.en.md

+# This forces the ThreadLocal<WebDriver> variable to call driver.get() every time the driver wants to be used.
+
+# In general static variables in non-thread safe code can have unintended consequences and increase the maintanance effort in the code base.
+
+public abstract class BaseTest {
+	protected ThreadLocal<WebDriver> driver;
+    ...
+    // Before each test hook
+    public void setupTest() {
+        BaseTest.driver = ThreadLocal.withInitial(()->new FirefoxDriver());
+        ...
+    }
+}


We prefer to avoid mentioning ThreadDriver as it has brought more damage than benefit.

Perhaps show here the code examples we have already:

https://github.com/SeleniumHQ/seleniumhq.github.io/blob/trunk/examples/java/src/test/java/dev/selenium/BaseChromeTest.java#L7-L10

https://github.com/SeleniumHQ/seleniumhq.github.io/blob/trunk/examples/java/src/test/java/dev/selenium/BaseTest.java#L109-L114

jflorez · 2024-04-15T20:45:54Z

I was wondering about showing ThreadLocal or not given its many issues. I'll remove the references to existing code in examples

jflorez and others added 6 commits April 10, 2024 07:16

Update fresh_browser_per_test.en.md

203bd69

extend explanation of browser per test with additional code examples

7bc6b21

Merge branch 'SeleniumHQ:trunk' into patch-1

d95089b

extend explanation of browser per test with additional code examples

70ce085

extend explanation of browser per test with additional code examples

bbee03e

Merge branch 'patch-1' of github.com:jflorez/seleniumhq.github.io int…

7f2aa0a

…o patch-1

codiumai-pr-agent-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 15, 2024

Merge branch 'trunk' into patch-1

8d72b74

codiumai-pr-agent-pro bot added the Review effort [1-5]: 2 label Apr 15, 2024

diemol requested changes Apr 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand documentation on fresh browser per test #1671

Expand documentation on fresh browser per test #1671

jflorez commented Apr 15, 2024 •

edited by codiumai-pr-agent-pro bot

netlify bot commented Apr 15, 2024 •

edited

CLAassistant commented Apr 15, 2024 •

edited

codiumai-pr-agent-pro bot commented Apr 15, 2024

codiumai-pr-agent-pro bot commented Apr 15, 2024

codiumai-pr-agent-pro bot commented Apr 15, 2024

diemol left a comment

diemol Apr 15, 2024

diemol Apr 15, 2024

jflorez commented Apr 15, 2024

Expand documentation on fresh browser per test #1671

Are you sure you want to change the base?

Expand documentation on fresh browser per test #1671

Conversation

jflorez commented Apr 15, 2024 • edited by codiumai-pr-agent-pro bot

User description

Description

Motivation and Context

Types of changes

Checklist

Type

Description

Changes walkthrough

netlify bot commented Apr 15, 2024 • edited

👷 Deploy request for selenium-dev pending review.

CLAassistant commented Apr 15, 2024 • edited

codiumai-pr-agent-pro bot commented Apr 15, 2024

codiumai-pr-agent-pro bot commented Apr 15, 2024

PR Review

codiumai-pr-agent-pro bot commented Apr 15, 2024

PR Code Suggestions

diemol left a comment

Choose a reason for hiding this comment

diemol Apr 15, 2024

Choose a reason for hiding this comment

diemol Apr 15, 2024

Choose a reason for hiding this comment

jflorez commented Apr 15, 2024

jflorez commented Apr 15, 2024 •

edited by codiumai-pr-agent-pro bot

netlify bot commented Apr 15, 2024 •

edited

CLAassistant commented Apr 15, 2024 •

edited