SeleniumHQ / selenium

A browser automation framework and ecosystem.
https://selenium.dev
Apache License 2.0
29.73k stars 8.02k forks source link

EdgeOptions.useWebView to return "this" #14157

Closed vlad8x8 closed 1 week ago

vlad8x8 commented 1 week ago

User description

All ChromiumOptions methods return this This commit makes EdgeOptions.useWebView behave the same as other Options methods

Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

All ChromiumOptions methods return this This commit makes EdgeOptions.useWebView behave the same as other Options methods

Motivation and Context

All ChromiumOptions methods return this This commit makes EdgeOptions.useWebView behave the same as other Options methods

Types of changes

Checklist


PR Type

Enhancement


Description


Changes walkthrough 📝

Relevant files
Enhancement
EdgeOptions.java
Modify `useWebView` method to return `EdgeOptions` instance

java/src/org/openqa/selenium/edge/EdgeOptions.java
  • Modified useWebView method to return EdgeOptions instead of void.
  • Added a return statement to return this in the useWebView method.
  • +2/-1     

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

    codiumai-pr-agent-pro[bot] commented 1 week ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 1
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review None
    codiumai-pr-agent-pro[bot] commented 1 week ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add a method description to explain the purpose and behavior of the useWebView method ___ **Add a method description to explain the purpose and behavior of the useWebView method for
    better code readability.** [java/src/org/openqa/selenium/edge/EdgeOptions.java [65-69]](https://github.com/SeleniumHQ/selenium/pull/14157/files#diff-2a19705afa93cc28103a6935b7f549605f8bcc8dc6b45f351d0e1cc97d273593R65-R69) ```diff +/** + * Enables or disables the use of WebView2. + * + * @param enable boolean flag to enable or disable the 'webview2' usage + * @return the current EdgeOptions instance + */ public EdgeOptions useWebView(boolean enable) { String browserName = enable ? WEBVIEW2_BROWSER_NAME : EDGE.browserName(); setCapability(CapabilityType.BROWSER_NAME, browserName); return this; } ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: Adding a method description improves code readability and maintainability, which is a good practice, especially for public APIs.
    7
    Possible issue
    Ensure that the browser name is not null to prevent potential issues when setting capabilities ___ **Ensure that the WEBVIEW2_BROWSER_NAME and EDGE.browserName() are not null to prevent
    potential issues when setting capabilities.** [java/src/org/openqa/selenium/edge/EdgeOptions.java [65-69]](https://github.com/SeleniumHQ/selenium/pull/14157/files#diff-2a19705afa93cc28103a6935b7f549605f8bcc8dc6b45f351d0e1cc97d273593R65-R69) ```diff public EdgeOptions useWebView(boolean enable) { String browserName = enable ? WEBVIEW2_BROWSER_NAME : EDGE.browserName(); + if (browserName == null) { + throw new IllegalStateException("Browser name cannot be null"); + } setCapability(CapabilityType.BROWSER_NAME, browserName); return this; } ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 4 Why: While ensuring non-null values is generally good, the suggestion lacks context on whether `WEBVIEW2_BROWSER_NAME` or `EDGE.browserName()` could realistically be null, which affects the necessity of this check.
    4