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

[dotnet] Migrate remaining NUnit assertions to Assert.That and Has.Count #14870

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

RenderMichael
Copy link
Contributor

@RenderMichael RenderMichael commented Dec 6, 2024

User description

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

Migrates remaining assertions to Assert.That, including improving some hand-rolled exception assertions

Motivation and Context

Contributes to #14852

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement, tests


Description

  • Migrated NUnit assertions to modern syntax using Assert.That with Has and Throws for improved readability and consistency.
  • Updated exception handling assertions to use Throws syntax for better clarity.
  • Modified JavaScript result conversion method for consistency.
  • Enhanced dictionary equivalence assertions in interaction tests.

Changes walkthrough 📝

Relevant files
Enhancement
19 files
BrowserTest.cs
Update assertion syntax for user context count check.       

dotnet/test/common/BiDi/Browser/BrowserTest.cs

  • Migrated assertion to use Has.Count.GreaterThanOrEqualTo.
+1/-1     
NetworkEventsTest.cs
Update assertion syntax for request cookies count check. 

dotnet/test/common/BiDi/Network/NetworkEventsTest.cs

  • Migrated assertion to use Has.Count.EqualTo.
+1/-1     
ScriptCommandsTest.cs
Update assertion syntax for realm count checks.                   

dotnet/test/common/BiDi/Script/ScriptCommandsTest.cs

  • Migrated assertions to use Has.Count.EqualTo.
+2/-2     
StorageTest.cs
Update assertion syntax for cookie count checks.                 

dotnet/test/common/BiDi/Storage/StorageTest.cs

  • Migrated assertions to use Has.Count.EqualTo.
+3/-3     
ClickScrollingTest.cs
Modify JavaScript result conversion method.                           

dotnet/test/common/ClickScrollingTest.cs

  • Changed conversion method for JavaScript result.
+1/-1     
DriverElementFindingTest.cs
Update assertion syntax for element count checks.               

dotnet/test/common/DriverElementFindingTest.cs

  • Migrated assertions to use Has.Count.EqualTo.
+7/-7     
ElementElementFindingTest.cs
Update assertion syntax for child element count checks.   

dotnet/test/common/ElementElementFindingTest.cs

  • Migrated assertions to use Has.Count.EqualTo.
+7/-7     
ExecutingAsyncJavascriptTest.cs
Update assertion syntax for async JavaScript tests.           

dotnet/test/common/ExecutingAsyncJavascriptTest.cs

  • Migrated assertions to use Has.Count.EqualTo.
  • Updated exception handling assertions.
  • +7/-5     
    ExecutingJavascriptTest.cs
    Update assertion syntax for JavaScript execution tests.   

    dotnet/test/common/ExecutingJavascriptTest.cs

  • Updated exception handling assertions.
  • Migrated assertion to use Has.Length.GreaterThan.
  • +7/-5     
    ActionBuilderTest.cs
    Update dictionary equivalence assertion syntax.                   

    dotnet/test/common/Interactions/ActionBuilderTest.cs

    • Updated dictionary equivalence assertions.
    +4/-4     
    FileLogHandlerTest.cs
    Update assertion syntax for log file content checks.         

    dotnet/test/common/Internal/Logging/FileLogHandlerTest.cs

    • Migrated assertions to use Has.Count.EqualTo.
    +4/-4     
    NavigationTest.cs
    Update navigation assertion syntax for async methods.       

    dotnet/test/common/NavigationTest.cs

    • Updated navigation assertions to use Throws.Nothing.
    +2/-2     
    PositionAndSizeTest.cs
    Update commented assertion syntax for position tests.       

    dotnet/test/common/PositionAndSizeTest.cs

    • Commented out assertion updated to new syntax.
    +2/-2     
    FirefoxDriverTest.cs
    Update exception handling assertion syntax.                           

    dotnet/test/firefox/FirefoxDriverTest.cs

    • Updated exception handling assertion.
    +3/-9     
    RemoteWebDriverSpecificTests.cs
    Update driver type check assertion syntax.                             

    dotnet/test/remote/RemoteWebDriverSpecificTests.cs

    • Updated assertion for driver type check.
    +1/-1     
    EventFiringWebDriverTest.cs
    Update exception handling assertion syntax.                           

    dotnet/test/support/Events/EventFiringWebDriverTest.cs

    • Updated exception handling assertion.
    +3/-9     
    LoadableComponentTests.cs
    Update exception handling assertion syntax.                           

    dotnet/test/support/UI/LoadableComponentTests.cs

    • Updated exception handling assertions.
    +9/-19   
    SlowLoadableComponentTest.cs
    Update exception handling assertion syntax.                           

    dotnet/test/support/UI/SlowLoadableComponentTest.cs

    • Updated exception handling assertions.
    +7/-18   
    WebDriverWaitTest.cs
    Update exception handling assertion syntax.                           

    dotnet/test/support/UI/WebDriverWaitTest.cs

    • Updated exception handling assertion.
    +3/-9     

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

    Copy link
    Contributor

    qodo-merge-pro bot commented Dec 6, 2024

    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

    Type Conversion
    The change from ChangeType to ToInt64 for JavaScript result conversion may affect type handling. Verify that ToInt64 handles all expected numeric types correctly.

    Dictionary Comparison
    The change in dictionary comparison from CollectionAssert to Is.EquivalentTo should be validated to ensure equivalent behavior, especially for nested dictionaries and collections.

    Copy link
    Contributor

    qodo-merge-pro bot commented Dec 6, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Use appropriate integer type conversion for scroll position values

    Use Convert.ToInt32() instead of Convert.ToInt64() since the JavaScript scroll
    position is typically returned as a 32-bit integer. Using Int64 could lead to
    overflow issues with very large scroll positions.

    dotnet/test/common/ClickScrollingTest.cs [47]

    -var yOffset = Convert.ToInt64(result);
    +var yOffset = Convert.ToInt32(result);
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: While both Int32 and Int64 would work, using Int32 is more appropriate for scroll position values as they rarely exceed 32-bit integer limits. However, this is a minor optimization with minimal impact on functionality.

    3

    💡 Need additional feedback ? start a PR chat

    @nvborisenko nvborisenko changed the title [dotnet] Migrate remaining NUnit assertions to modern syntax [dotnet] Migrate remaining NUnit assertions to Assert.That and Has.Count Dec 6, 2024
    @nvborisenko
    Copy link
    Member

    As usual, thanks @RenderMichael for contribution.

    @nvborisenko nvborisenko merged commit 40ea8a4 into SeleniumHQ:trunk Dec 6, 2024
    9 of 10 checks passed
    @RenderMichael RenderMichael deleted the dotnet-migrate-assert branch December 6, 2024 18:55
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Dec 7, 2024
    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