Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[java] Add high-level BiDi API log examples #1835

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

Merged
harsha509 merged 1 commit into trunk from add-log-bidi-example-java
Aug 3, 2024

Conversation

@pujagani
Copy link
Contributor

@pujagani pujagani commented Aug 2, 2024 •
edited by qodo-merge-pro bot
Loading

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

Demonstrate usage of log handlers.

Motivation and Context

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

Tests

Description

  • Added a new test class LogTest to demonstrate the usage of BiDi API log handlers in Java.
  • Implemented a setup method to initialize FirefoxDriver with WebSocket capability.
  • Added tests to verify the addition and removal of console message handlers.
  • Added tests to verify the addition and removal of JavaScript error handlers.

Changes walkthrough 📝

Relevant files
Tests
LogTest.java
Add BiDi API log handling tests in Java

examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/high_level/LogTest.java

  • Added new test class LogTest for BiDi API log examples.
  • Implemented setup method to initialize FirefoxDriver with WebSocket
    capability.
  • Added tests for adding and removing console message handlers.
  • Added tests for adding and removing JavaScript error handlers.
    • +89/-0

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

    Copy link
    Contributor

    qodo-merge-pro bot commented Aug 2, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Duplication
    The setup method for initializing the FirefoxDriver with FirefoxOptions is repeated in each test. Consider refactoring this into a common setup method to avoid redundancy and make the code cleaner.

    Hardcoded URL
    The URL "https://www.selenium.dev/selenium/web/bidi/logEntryAdded.html" is hardcoded in multiple test methods. Consider using a constant or a configuration setting for this URL to make the tests more maintainable and flexible.

    Copy link

    netlify bot commented Aug 2, 2024

    ✅ Deploy Preview for selenium-dev ready!

    Name Link
    🔨 Latest commit 2d1e3f3
    🔍 Latest deploy log https://app.netlify.com/sites/selenium-dev/deploys/66aca08e1d61f9000950f888
    😎 Deploy Preview https://deploy-preview-1835--selenium-dev.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    Copy link
    Contributor

    qodo-merge-pro bot commented Aug 2, 2024

    PR Code Suggestions ✨

    CategorySuggestion Score
    Best practice
    Ensure resources are cleaned up reliably by using try-finally

    Use a try-with-resources or finally block to ensure that handlers are always removed
    even if an exception occurs during the test execution.

    examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/high_level/LogTest.java [35-44] 

     long id = ((RemoteWebDriver) driver).script().addConsoleMessageHandler(future::complete);
-...
-((RemoteWebDriver) driver).script().removeConsoleMessageHandler(id);
+try {
+ ...
+} finally {
+ ((RemoteWebDriver) driver).script().removeConsoleMessageHandler(id);
+}
    Suggestion importance[1-10]: 9

    Why: This suggestion ensures that resources are reliably cleaned up, which is crucial for preventing resource leaks and ensuring test reliability. It addresses a significant best practice.

    		9
    Add pre-action assertions to verify the correct state of elements

    Consider adding assertions to check the state before performing actions to ensure
    that the test environment is set up correctly.

    examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/high_level/LogTest.java [37-38] 

     driver.get("https://www.selenium.dev/selenium/web/bidi/logEntryAdded.html");
+Assertions.assertTrue(driver.findElement(By.id("consoleLog")).isDisplayed(), "Console log button should be visible");
 driver.findElement(By.id("consoleLog")).click();
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding pre-action assertions enhances the robustness of the tests by ensuring that the environment is correctly set up before actions are performed. This is a good practice for reliable test execution.

    		8
    Maintainability
    Improve flexibility and maintainability of the driver initialization

    Replace the direct instantiation of FirefoxDriver with a more flexible driver
    initialization. This allows for easier updates or changes to the browser driver
    without modifying the test setup method directly.

    examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/high_level/LogTest.java [25-27] 

     FirefoxOptions options = new FirefoxOptions();
 options.setCapability("webSocketUrl", true);
-driver = new FirefoxDriver(options);
+driver = createDriver(options);
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves the flexibility and maintainability of the code by abstracting the driver initialization. However, it is not a critical change and does not address any major issues.

    		7
    Use constants for repeated literal values to enhance maintainability

    Refactor repeated URL strings into a constant to avoid duplication and facilitate
    changes.

    examples/java/src/test/java/dev/selenium/bidirectional/webdriver_bidi/high_level/LogTest.java [37-84] 

    -driver.get("https://www.selenium.dev/selenium/web/bidi/logEntryAdded.html");
+private static final String TEST_PAGE_URL = "https://www.selenium.dev/selenium/web/bidi/logEntryAdded.html";
 ...
-driver.get("https://www.selenium.dev/selenium/web/bidi/logEntryAdded.html");
+driver.get(TEST_PAGE_URL);
+...
+driver.get(TEST_PAGE_URL);
    Suggestion importance[1-10]: 6

    Why: Using constants for repeated literal values improves maintainability by reducing duplication and making future changes easier. However, it is a minor improvement and not critical.

    		6

    @harsha509 harsha509 merged commit 1ef4dd1 into trunk Aug 3, 2024
    @harsha509 harsha509 deleted the add-log-bidi-example-java branch August 3, 2024 07:28
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @harsha509 harsha509 harsha509 approved these changes

    Assignees

    No one assigned

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    AltStyle によって変換されたページ (->オリジナル) /