Skip to content

add csharpwindowscommands #1774

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
merged 6 commits into from
Jul 5, 2024

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Jun 22, 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 example file and code lines for the windows csharp code

Motivation and Context

example code was missing. to improve documentation

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


Description

  • Added a new C# example file WindowsTest.cs to demonstrate window handling in Selenium.
  • Updated English, Japanese, Portuguese, and Chinese documentation to include new C# code examples for window handling.
  • Improved the clarity and accuracy of the C# code examples in the documentation.

Changes walkthrough 📝

Relevant files
Enhancement
WindowsTest.cs
Add C# example for window handling in Selenium                     

examples/dotnet/SeleniumDocs/Interactions/WindowsTest.cs

  • Added a new test method TestWindowCommands to demonstrate window
    handling in Selenium.
  • Included examples for opening new windows and tabs, switching between
    them, and closing them.
  • Added assertions to verify the titles of the new windows and tabs.
  • +41/-1   
    Documentation
    windows.en.md
    Update English documentation for C# window handling           

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

  • Updated C# code examples to reference new lines in WindowsTest.cs.
  • Improved documentation for window handling in C#.
  • +21/-40 
    windows.ja.md
    Update Japanese documentation for C# window handling         

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

  • Updated C# code examples to reference new lines in WindowsTest.cs.
  • Improved Japanese documentation for window handling in C#.
  • +23/-39 
    windows.pt-br.md
    Update Portuguese documentation for C# window handling     

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

  • Updated C# code examples to reference new lines in WindowsTest.cs.
  • Improved Portuguese documentation for window handling in C#.
  • +18/-37 
    windows.zh-cn.md
    Update Chinese documentation for C# window handling           

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

  • Updated C# code examples to reference new lines in WindowsTest.cs.
  • Improved Chinese documentation for window handling in C#.
  • +19/-38 

    💡 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 Jun 22, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit e6dda92

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 22, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The code does not handle exceptions that might be thrown by the WebDriver, such as NoSuchElementException or WebDriverException. This could lead to unhandled exceptions during runtime.
    Resource Management:
    The WebDriver instance is created and disposed within the same method. While this is generally okay, it might be more efficient to manage the WebDriver lifecycle at a higher level if multiple tests are run in sequence, to avoid the overhead of starting up and tearing down the WebDriver repeatedly.
    Hardcoded URL:
    The URL "https://www.selenium.dev/selenium/web/window_switching_tests/page_with_frame.html" is hardcoded in the TestWindowCommands method. It's a good practice to avoid hardcoding URLs and instead, retrieve them from configuration files or environment variables, making the tests more flexible and environment-independent.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a try-finally block to ensure WebDriver quits properly even if an exception occurs

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

    examples/dotnet/SeleniumDocs/Interactions/WindowsTest.cs [14-46]

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

    Why: This suggestion addresses a significant best practice in ensuring resources are cleaned up properly, which is crucial for preventing resource leaks.

    8
    Possible issue
    Add a check to ensure the new window handle is different from the current window handle before switching

    Add a check to ensure that the new window handle is different from the current window
    handle before switching to it.

    examples/dotnet/SeleniumDocs/Interactions/WindowsTest.cs [27]

    -driver.SwitchTo().Window(windowHandles[1]);
    +if (windowHandles[1] != currHandle)
    +{
    +    driver.SwitchTo().Window(windowHandles[1]);
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion potentially prevents a logical error where switching to the same window could occur, enhancing the robustness of the code.

    7
    Enhancement
    Use var for local variable declarations where the type is obvious to make the code more concise

    Use var instead of explicit type declaration for local variables where the type is obvious
    from the right-hand side to make the code more concise.

    examples/dotnet/SeleniumDocs/Interactions/WindowsTest.cs [14-29]

    -WebDriver driver = new ChromeDriver();
    -String currHandle = driver.CurrentWindowHandle;
    -IList<string> windowHandles = new List<string>(driver.WindowHandles);
    -String title = driver.Title;
    +var driver = new ChromeDriver();
    +var currHandle = driver.CurrentWindowHandle;
    +var windowHandles = new List<string>(driver.WindowHandles);
    +var title = driver.Title;
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion improves code conciseness and readability by using type inference, although it's more of a style preference than a critical change.

    6
    Maintainability
    Add consistent spacing between tab headers and their content

    Ensure consistent spacing between the tab headers and their content to improve readability
    and maintainability.

    website_and_docs/content/documentation/webdriver/interactions/windows.en.md [26-28]

     {{< tab header="CSharp" text=true >}}
    +
     {{< gh-codeblock path="examples/dotnet/SeleniumDocs/Interactions/WindowsTest.cs#L17-L21" >}}
    +
     {{< /tab >}}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion correctly identifies a minor readability improvement in the markdown documentation, but it's not a critical change.

    5

    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 @pallavigitwork !

    @harsha509 harsha509 merged commit db69292 into SeleniumHQ:trunk Jul 5, 2024
    8 checks passed
    selenium-ci added a commit that referenced this pull request Jul 5, 2024
    * add csharpwindowscommands
    
    * Update WindowsTest.cs
    
    ---------
    
    Co-authored-by: Sri Harsha <[email protected]> db69292
    @pallavigitwork
    Copy link
    Member Author

    pallavigitwork commented Jul 5, 2024

    Thank you Harsha! @harsha509

    @pallavigitwork pallavigitwork deleted the windowscsharpcode-branch branch September 27, 2024 12:09
    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 enhancement New feature or request Review effort [1-5]: 3
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants