Skip to content
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

[java]: add java docs for setPermission for chrome and edge browsers #2063

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

navin772
Copy link
Contributor

@navin772 navin772 commented Nov 15, 2024

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

Add java docs for setPermission in chrome and edge browsers

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

documentation, tests


Description

  • Added Java test methods for setting and verifying browser permissions in Chrome and Edge.
  • Updated documentation to include new Java code examples for setting permissions in Chrome and Edge browsers.
  • Adjusted code block references in multiple language versions of the documentation.

Changes walkthrough 📝

Relevant files
Tests
2 files
ChromeTest.java
Add setPermission test for Chrome browser                               

examples/java/src/test/java/dev/selenium/browsers/ChromeTest.java

  • Added a test method setPermission for Chrome.
  • Set camera permission to 'denied' and verified the state.
  • +18/-0   
    EdgeTest.java
    Add setPermissions test for Edge browser                                 

    examples/java/src/test/java/dev/selenium/browsers/EdgeTest.java

  • Added a test method setPermissions for Edge.
  • Set camera permission to 'denied' and verified the state.
  • +18/-0   
    Documentation
    8 files
    chrome.en.md
    Update Java code block in Chrome documentation                     

    website_and_docs/content/documentation/webdriver/browsers/chrome.en.md

    • Updated Java code block reference for Chrome documentation.
    +1/-1     
    chrome.ja.md
    Update Java code block in Japanese Chrome documentation   

    website_and_docs/content/documentation/webdriver/browsers/chrome.ja.md

  • Updated Java code block reference for Japanese Chrome documentation.
  • +1/-1     
    chrome.pt-br.md
    Update Java code block in Portuguese Chrome documentation

    website_and_docs/content/documentation/webdriver/browsers/chrome.pt-br.md

  • Updated Java code block reference for Portuguese Chrome documentation.

  • +1/-1     
    chrome.zh-cn.md
    Update Java code block in Chinese Chrome documentation     

    website_and_docs/content/documentation/webdriver/browsers/chrome.zh-cn.md

  • Updated Java code block reference for Chinese Chrome documentation.
  • +1/-1     
    edge.en.md
    Update Java code block in Edge documentation                         

    website_and_docs/content/documentation/webdriver/browsers/edge.en.md

    • Updated Java code block reference for Edge documentation.
    +1/-1     
    edge.ja.md
    Update Java code block in Japanese Edge documentation       

    website_and_docs/content/documentation/webdriver/browsers/edge.ja.md

    • Updated Java code block reference for Japanese Edge documentation.
    +1/-1     
    edge.pt-br.md
    Update Java code block in Portuguese Edge documentation   

    website_and_docs/content/documentation/webdriver/browsers/edge.pt-br.md

  • Updated Java code block reference for Portuguese Edge documentation.
  • +1/-1     
    edge.zh-cn.md
    Update Java code block in Chinese Edge documentation         

    website_and_docs/content/documentation/webdriver/browsers/edge.zh-cn.md

    • Updated Java code block reference for Chinese Edge documentation.
    +1/-1     

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

    Copy link

    netlify bot commented Nov 15, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit a53a70b

    Copy link
    Contributor

    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

    Resource Cleanup
    The driver.quit() call should be in a finally block to ensure browser cleanup even if assertions fail

    Resource Cleanup
    The driver.quit() call should be in a finally block to ensure browser cleanup even if assertions fail

    Error Handling
    The permission query JavaScript execution should include error handling for unsupported browsers or denied permissions

    Copy link
    Contributor

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

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure proper resource cleanup by using try-finally block for driver management

    Add a try-finally block to ensure the driver is always closed, even if an assertion
    or other exception occurs during the test.

    examples/java/src/test/java/dev/selenium/browsers/ChromeTest.java [185-199]

     @Test
     public void setPermission() {
         ChromeDriver driver = new ChromeDriver();
    -    driver.get("https://www.selenium.dev");
    -    
    -    driver.setPermission("camera", "denied");
    -    
    -    String script = """
    -            return navigator.permissions.query({ name: 'camera' })
    -                .then(permissionStatus => permissionStatus.state);
    -        """;
    -    String permissionState = (String) driver.executeScript(script);
    -    
    -    Assertions.assertEquals("denied", permissionState);
    -    driver.quit();
    +    try {
    +        driver.get("https://www.selenium.dev");
    +        
    +        driver.setPermission("camera", "denied");
    +        
    +        String script = """
    +                return navigator.permissions.query({ name: 'camera' })
    +                    .then(permissionStatus => permissionStatus.state);
    +            """;
    +        String permissionState = (String) driver.executeScript(script);
    +        
    +        Assertions.assertEquals("denied", permissionState);
    +    } finally {
    +        driver.quit();
    +    }
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding a try-finally block is crucial for ensuring proper resource cleanup, preventing potential memory leaks and browser process hangs if an exception occurs during test execution.

    8
    Possible issue
    Add error handling for JavaScript Promise execution to improve failure diagnostics

    Add error handling for the JavaScript execution to handle potential Promise
    rejection or execution failures.

    examples/java/src/test/java/dev/selenium/browsers/ChromeTest.java [192-196]

     String script = """
             return navigator.permissions.query({ name: 'camera' })
    -            .then(permissionStatus => permissionStatus.state);
    +            .then(permissionStatus => permissionStatus.state)
    +            .catch(error => { throw new Error('Failed to query permission state: ' + error); });
         """;
     String permissionState = (String) driver.executeScript(script);
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding error handling for Promise rejection provides better diagnostics and clearer error messages when permission queries fail, though the base functionality would still work without it.

    6

    💡 Need additional feedback ? start a PR chat

    Copy link
    Member

    @harsha509 harsha509 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thank you @navin772 !

    @harsha509 harsha509 merged commit 0ec7656 into SeleniumHQ:trunk Nov 15, 2024
    9 checks passed
    @navin772 navin772 deleted the java_setPermission branch November 15, 2024 18:19
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants