[java] use common annotations in BiDi tests

[joerg1985]

User description

Motivation and Context

This PR will use the common annotations in the BiDi tests to ensure the driver is restarted.
Additionally the provided NettyAppServer is used and not started before each test.

On my local system it takes ~500ms to start the NettyAppServer, this is how i stumbled into this.
I will improve the startup of the NettyAppServer as soon as this is merged.

@pujagani could you hava a look at this?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

enhancement, tests

---

Description

• Refactored multiple test classes to remove @BeforeEach and @AfterEach annotations, replacing them with @NeedsFreshDriver to ensure a fresh driver for each test.
• Replaced server with appServer in test classes to standardize server usage.
• Added @BeforeAll and @AfterAll annotations in RemoteWebDriverBiDiTest for server lifecycle management.
• Enhanced test reliability by ensuring a fresh driver instance for each test case.

---

Changes walkthrough 📝

Relevant files

Enhancement

16 files BiDiTest.java Refactor BiDiTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/BiDiTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +3/-21    BrowserCommandsTest.java Refactor BrowserCommandsTest to use common annotations      java/test/org/openqa/selenium/bidi/browser/BrowserCommandsTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +4/-16    BrowsingContextInspectorTest.java Refactor BrowsingContextInspectorTest to use common annotations java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +17/-29  BrowsingContextTest.java Refactor BrowsingContextTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +38/-27  LocateNodesTest.java Refactor LocateNodesTest to use common annotations              java/test/org/openqa/selenium/bidi/browsingcontext/LocateNodesTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +10/-20  ReleaseCommandTest.java Refactor ReleaseCommandTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/input/ReleaseCommandTest.java Removed @BeforeEach annotation. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +3/-7      SetFilesCommandTest.java Refactor SetFilesCommandTest to use common annotations      java/test/org/openqa/selenium/bidi/input/SetFilesCommandTest.java Removed @BeforeEach annotation. Added @NeedsFreshDriver annotation to test methods. +5/-6      LogInspectorTest.java Refactor LogInspectorTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +31/-35  AddInterceptParametersTest.java Refactor AddInterceptParametersTest to use common annotations java/test/org/openqa/selenium/bidi/network/AddInterceptParametersTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +7/-21    NetworkCommandsTest.java Refactor NetworkCommandsTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +19/-29  NetworkEventsTest.java Refactor NetworkEventsTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/network/NetworkEventsTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +13/-26  CallFunctionParameterTest.java Refactor CallFunctionParameterTest to use common annotations java/test/org/openqa/selenium/bidi/script/CallFunctionParameterTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +15/-21  EvaluateParametersTest.java Refactor EvaluateParametersTest to use common annotations java/test/org/openqa/selenium/bidi/script/EvaluateParametersTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. +8/-20    ScriptCommandsTest.java Refactor ScriptCommandsTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/script/ScriptCommandsTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +32/-23  ScriptEventsTest.java Refactor ScriptEventsTest to use common annotations and appServer java/test/org/openqa/selenium/bidi/script/ScriptEventsTest.java Removed @BeforeEach and @AfterEach annotations. Added @NeedsFreshDriver annotation to test methods. Replaced server with appServer. +5/-21    RemoteWebDriverBiDiTest.java Refactor RemoteWebDriverBiDiTest for server lifecycle management java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java Added @BeforeAll and @AfterAll annotations for server setup and teardown. Removed @BeforeEach annotation for server setup. +13/-4

---

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

[codiumai-pr-agent-pro]

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 Code Duplication There are multiple test methods with very similar structure and setup. Consider extracting common setup code into a separate method to reduce duplication. Error Handling The error handling in the canFailRequest test method could be improved. It's catching a generic WebDriverException, which might mask other unexpected exceptions.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Add assertions to verify the expected behavior after performing actions in tests Consider adding assertions to verify the expected behavior after performing the release action, such as checking the final state of the input element or any visual feedback. java/test/org/openqa/selenium/bidi/input/ReleaseCommandTest.java [59] input.release(windowHandle); +// Add assertions to verify the release action +String finalText = inputTextBox.getAttribute("value"); +assertThat(finalText).isEqualTo("A"); + +WebElement resultElement = driver.findElement(By.id("result")); +assertThat(resultElement.getText()).isEqualTo("Key released"); + Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding assertions to verify the expected behavior after performing actions in tests is crucial for validating test outcomes. This suggestion enhances test robustness and ensures that the code behaves as expected, improving test reliability.	8
	Extract common setup code into a helper method to reduce duplication Consider extracting the repeated pattern of creating a Script object with a try-with-resources block into a helper method. This would reduce code duplication and improve readability across multiple test methods. java/test/org/openqa/selenium/bidi/script/CallFunctionParameterTest.java [40-41] -String id = driver.getWindowHandle(); -try (Script script = new Script(id, driver)) { +private Script createScriptForCurrentWindow() { + String id = driver.getWindowHandle(); + return new Script(id, driver); +} + +// In test methods: +try (Script script = createScriptForCurrentWindow()) { // Test-specific code } Apply this suggestion Suggestion importance[1-10]: 6 Why: Extracting the common setup code into a helper method reduces code duplication and improves readability, making the test methods cleaner and more maintainable.	6
	Extract common setup code into a helper method to reduce duplication and improve readability Consider extracting the repeated pattern of creating a BrowsingContext object and asserting its ID into a setup method or a helper method. This would reduce code duplication across multiple test methods and improve test readability. java/test/org/openqa/selenium/bidi/browsingcontext/LocateNodesTest.java [44-45] -BrowsingContext browsingContext = new BrowsingContext(driver, driver.getWindowHandle()); -assertThat(browsingContext.getId()).isNotEmpty(); +private BrowsingContext createAndAssertBrowsingContext() { + BrowsingContext browsingContext = new BrowsingContext(driver, driver.getWindowHandle()); + assertThat(browsingContext.getId()).isNotEmpty(); + return browsingContext; +} +// In test methods: +BrowsingContext browsingContext = createAndAssertBrowsingContext(); + Apply this suggestion Suggestion importance[1-10]: 6 Why: This suggestion effectively reduces code duplication by extracting repeated setup code into a helper method, enhancing readability and maintainability of the test code.	6
	Use a more specific assertion for checking non-empty strings Consider using a more specific assertion for checking if the browsingContext.getId() is not empty, such as assertThat(browsingContext.getId()).isNotBlank() or assertThat(browsingContext.getId()).isNotEmpty(). java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextTest.java [55] -assertThat(browsingContext.getId()).isNotEmpty(); +assertThat(browsingContext.getId()).isNotBlank(); Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion to use isNotBlank() instead of isNotEmpty() provides a more precise assertion for non-empty strings, which can catch additional edge cases such as strings with only whitespace. This enhances the robustness of the test assertions.	5
Best practice	Use a constant for timeout values to improve maintainability and consistency Consider using a constant or configuration value for the timeout duration in latch.await() calls instead of hardcoding the value 5. This improves maintainability and allows for easier adjustments across the test suite. java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java [85] -boolean countdown = latch.await(5, TimeUnit.SECONDS); +private static final int LATCH_TIMEOUT_SECONDS = 5; +// ... +boolean countdown = latch.await(LATCH_TIMEOUT_SECONDS, TimeUnit.SECONDS); Suggestion importance[1-10]: 7 Why: Using a constant for timeout values enhances maintainability and consistency across the test suite. It allows for easier adjustments and improves code readability.	7
	Use try-with-resources for all AutoCloseable resources to ensure proper resource management Consider using a try-with-resources statement for the LogInspector to ensure it is properly closed after use, even if an exception occurs. java/test/org/openqa/selenium/bidi/BiDiTest.java [48-56] -try (org.openqa.selenium.bidi.module.LogInspector logInspector = new LogInspector(driver)) { +try (org.openqa.selenium.bidi.module.LogInspector logInspector = new LogInspector(driver); + BrowsingContext browsingContext = new BrowsingContext(driver, driver.getWindowHandle())) { CompletableFuture<JavascriptLogEntry> future = new CompletableFuture<>(); logInspector.onJavascriptLog( logEntry -> { if (logEntry.getLevel().equals(LogLevel.ERROR)) { future.complete(logEntry); } }); Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to use try-with-resources for managing AutoCloseable resources like BrowsingContext enhances resource management and ensures proper closure, reducing the risk of resource leaks. This is a good practice for improving code reliability.	7
	Extract magic number to a named constant for better code maintainability Consider using a constant for the timeout value in the future.get() call to improve maintainability and readability. java/test/org/openqa/selenium/bidi/script/ScriptCommandsTest.java [781] -ConsoleLogEntry logEntry = future.get(5, TimeUnit.SECONDS); +private static final int CONSOLE_LOG_TIMEOUT_SECONDS = 5; +... +ConsoleLogEntry logEntry = future.get(CONSOLE_LOG_TIMEOUT_SECONDS, TimeUnit.SECONDS); Suggestion importance[1-10]: 6 Why: The suggestion to use a named constant for the timeout value improves code readability and maintainability by avoiding magic numbers. This change is beneficial for future modifications and understanding of the code.	6
	Use constants or configuration values for file paths to improve maintainability and flexibility Consider using a constant or configuration value for the file path instead of hardcoding it directly in the test method. java/test/org/openqa/selenium/bidi/input/SetFilesCommandTest.java [52-53] -File testFile = new File("test-files/selenium-logo.png"); +private static final String TEST_FILE_PATH = "test-files/selenium-logo.png"; + +// In the test method: +File testFile = new File(TEST_FILE_PATH); String filePath = testFile.getAbsolutePath(); Apply this suggestion Suggestion importance[1-10]: 6 Why: Replacing hardcoded file paths with constants improves maintainability and flexibility, making it easier to update paths in the future. This suggestion enhances code readability and reduces potential errors.	6

💡 Need additional feedback ? start a PR chat