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

added csharp code for alerts #2087

Closed
wants to merge 1 commit into from

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Nov 29, 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

added csharp code for alerts

Motivation and Context

added csharp code for alerts

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

enhancement, documentation


Description

  • Added a new C# test class AlertsTest to handle different types of alerts (simple, confirm, and prompt) using Selenium WebDriver.
  • Updated the documentation across multiple languages (English, Japanese, Portuguese, Chinese) to reference the new C# test class for alert interactions.
  • Replaced inline C# code examples with references to the newly added test class to ensure consistency and maintainability.

Changes walkthrough 📝

Relevant files
Enhancement
AlertsTest.cs
Add C# test class for handling alerts                                       

examples/dotnet/SeleniumDocs/Interactions/AlertsTest.cs

  • Added a new C# test class AlertsTest for handling alerts.
  • Implemented methods to test simple, confirm, and prompt alerts.
  • Utilized IJavaScriptExecutor to trigger alerts.
  • Incorporated assertions to verify alert text.
  • +76/-1   
    Documentation
    alerts.en.md
    Update C# alert examples in English documentation               

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

  • Updated C# code examples to reference new test class.
  • Replaced inline code with references to the new C# test class.
  • +9/-38   
    alerts.ja.md
    Update C# alert examples in Japanese documentation             

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

  • Updated C# code examples to reference new test class.
  • Replaced inline code with references to the new C# test class.
  • +9/-41   
    alerts.pt-br.md
    Update C# alert examples in Portuguese documentation         

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

  • Updated C# code examples to reference new test class.
  • Replaced inline code with references to the new C# test class.
  • +8/-30   
    alerts.zh-cn.md
    Update C# alert examples in Chinese documentation               

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

  • Updated C# code examples to reference new test class.
  • Replaced inline code with references to the new C# test class.
  • +9/-38   

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

    Copy link

    netlify bot commented Nov 29, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit d9ac99d

    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 Management
    WebDriver instance should be properly disposed using 'using' statement or try-finally block to ensure proper cleanup of resources

    Hardcoded Values
    Test uses hardcoded alert messages which could make tests brittle if the alert text changes. Consider moving these to constants or test data

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Ensure proper cleanup of WebDriver resources by using try-finally block

    Add a try-finally block to ensure the WebDriver is properly disposed even if an
    exception occurs during test execution.

    examples/dotnet/SeleniumDocs/Interactions/AlertsTest.cs [33-81]

     WebDriver driver = new ChromeDriver();
    -driver.Manage().Timeouts().ImplicitWait = TimeSpan.FromMilliseconds(500);
    -// ... test code ...
    -driver.Quit(); //close all windows
    +try {
    +    driver.Manage().Timeouts().ImplicitWait = TimeSpan.FromMilliseconds(500);
    +    // ... test code ...
    +}
    +finally {
    +    driver.Quit(); //close all windows
    +}
    Suggestion importance[1-10]: 9

    Why: Adding a try-finally block is crucial for proper resource cleanup, ensuring the WebDriver is disposed even if tests fail. This prevents resource leaks and orphaned browser processes.

    9
    Improve performance by reusing wait instance instead of creating multiple instances

    Reuse the WebDriverWait instance by declaring it once at the start of the test
    method rather than recreating it for each alert.

    examples/dotnet/SeleniumDocs/Interactions/AlertsTest.cs [44]

    -WebDriverWait wait = new WebDriverWait(driver, TimeSpan.FromSeconds(30));
    -wait.Until(ExpectedConditions.AlertIsPresent());
    -// ... more code with repeated wait declarations ...
    +private readonly TimeSpan TIMEOUT = TimeSpan.FromSeconds(30);
    +// ... in test method:
    +WebDriverWait wait = new WebDriverWait(driver, TIMEOUT);
    +// ... reuse wait instance for all alerts ...
    Suggestion importance[1-10]: 6

    Why: Creating a single WebDriverWait instance improves code efficiency and reduces object allocation overhead, though the performance impact is relatively minor in this context.

    6
    Properly manage alert resources with using statement to ensure proper disposal

    Store the alert reference in a using block to ensure proper disposal of IAlert
    resources.

    examples/dotnet/SeleniumDocs/Interactions/AlertsTest.cs [47-51]

    -IAlert alert = driver.SwitchTo().Alert();
    -string text = alert.Text;
    -Assert.AreEqual(text, "Sample Alert");
    -alert.Accept();
    +using (IAlert alert = driver.SwitchTo().Alert())
    +{
    +    string text = alert.Text;
    +    Assert.AreEqual(text, "Sample Alert");
    +    alert.Accept();
    +}
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: While using statements for IAlert can help with resource management, IAlert implementations typically don't require explicit disposal, making this suggestion's impact minimal.

    3

    💡 Need additional feedback ? start a PR chat

    @pallavigitwork pallavigitwork deleted the alert-csharp branch December 2, 2024 05:53
    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.

    1 participant