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

Moved alert/prompts and confirmations examples of Java tab to example sections and updated the … #1900

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

Anilkumar-Shrestha
Copy link

@Anilkumar-Shrestha Anilkumar-Shrestha commented Aug 27, 2024

User description

Description

  1. moved alert/prompts and confirmations examples of java.
  2. updated alerts_{lang}.md files.

Motivation and Context

I wanted to ensure that users can get real example of using the alerts pop up and they can execute it easily by copying the test files.

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, Documentation


Description

  • Added comprehensive test cases for handling various alert scenarios in Java, including information alerts, prompts, and confirmation dialogs.
  • Introduced setup and teardown methods for managing WebDriver sessions in the test class.
  • Updated documentation across multiple languages to reference Java test examples using code references instead of inline snippets.

Changes walkthrough 📝

Relevant files
Tests
AlertsTest.java
Add comprehensive alert handling test cases in Java           

examples/java/src/test/java/dev/selenium/interactions/AlertsTest.java

  • Added multiple test cases for handling alerts and prompts.
  • Introduced setup and teardown methods for WebDriver sessions.
  • Utilized assertions to verify alert text and actions.
  • +175/-0 
    Documentation
    alerts.en.md
    Update Java alert examples with code references                   

    website_and_docs/content/documentation/webdriver/interactions/alerts.en.md

  • Updated Java examples to use code references.
  • Removed inline Java code snippets.
  • +10/-44 
    alerts.ja.md
    Update Java alert examples with code references                   

    website_and_docs/content/documentation/webdriver/interactions/alerts.ja.md

  • Updated Java examples to use code references.
  • Removed inline Java code snippets.
  • +11/-42 
    alerts.pt-br.md
    Update Java alert examples with code references                   

    website_and_docs/content/documentation/webdriver/interactions/alerts.pt-br.md

  • Updated Java examples to use code references.
  • Removed inline Java code snippets.
  • +11/-42 
    alerts.zh-cn.md
    Update Java alert examples with code references                   

    website_and_docs/content/documentation/webdriver/interactions/alerts.zh-cn.md

  • Updated Java examples to use code references.
  • Removed inline Java code snippets.
  • +11/-42 

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

    Copy link

    netlify bot commented Aug 27, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit a1fede3

    @CLAassistant
    Copy link

    CLAassistant commented Aug 27, 2024

    CLA assistant check
    All committers have signed the CLA.

    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added documentation Improvements or additions to documentation tests Review effort [1-5]: 3 labels Aug 27, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

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

    Code Duplication
    There is some code duplication in the test methods, particularly in setting up the driver, navigating to the page, and handling alerts. Consider extracting common setup code to a separate method or using @before annotations.

    Incomplete Assertion
    In the promptDefaultInputTest method, there's a comment indicating that implementation is needed to check if the default value is accepted. This test is incomplete and should be fully implemented.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Implement the check for default value acceptance in promptDefaultInputTest

    In the promptDefaultInputTest, implement the check for the default value being
    accepted as mentioned in the comment.

    examples/java/src/test/java/dev/selenium/interactions/AlertsTest.java [84-85]

     alert.accept();
    -// Implementation needed to check teh default value is accepted.
    +WebElement result = driver.findElement(By.id("prompt-with-default-result"));
    +Assertions.assertEquals("Default value", result.getText());
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Implementing the check for default value acceptance addresses a missing implementation noted in the comment, improving the completeness and accuracy of the test.

    9
    Add verification for alert closure after accepting it

    Consider adding a check to verify if the alert is closed after accepting it, to
    ensure the action was successful.

    examples/java/src/test/java/dev/selenium/interactions/AlertsTest.java [39-41]

     Alert alert = driver.switchTo().alert();
     Assertions.assertEquals("cheese", alert.getText());
     alert.accept();
    +Assertions.assertThrows(NoAlertPresentException.class, () -> driver.switchTo().alert());
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding a check to verify if the alert is closed after accepting it enhances the robustness of the test by ensuring that the alert handling was successful.

    8
    Add verification for switching back to default content after handling alerts in iframes

    Consider adding a check to verify if the driver has successfully switched back to
    the default content after handling alerts in iframe tests.

    examples/java/src/test/java/dev/selenium/interactions/AlertsTest.java [157]

     alert.accept();
    +driver.switchTo().defaultContent();
    +Assertions.assertEquals("Alerts", driver.getTitle());
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Verifying that the driver has successfully switched back to the default content after handling alerts in iframes ensures that subsequent tests are not affected by the iframe context, enhancing test reliability.

    8
    Best practice
    Use a constant for timeout duration in WebDriverWait initialization

    Consider using a constant for the timeout duration in the WebDriverWait
    initialization to improve maintainability.

    examples/java/src/test/java/dev/selenium/interactions/AlertsTest.java [23]

    -wait = new WebDriverWait(driver, Duration.ofSeconds(10));
    +private static final Duration TIMEOUT = Duration.ofSeconds(10);
    +wait = new WebDriverWait(driver, TIMEOUT);
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using a constant for the timeout duration improves code maintainability and readability by making it easier to update the timeout value in the future.

    7

    @Anilkumar-Shrestha
    Copy link
    Author

    @harsha509 Can you please guide me on the failed check case Run Java examples / tests (windows, nightly) (pull_request) . I am getting hard to understand what check it has been failed with the failed log details.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation Review effort [1-5]: 3 tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants