Update to Selenium v.4.36.0, update core to migrate from Newthonsoft.Json to System.Text.Json +semver: feature #275
Conversation
…Json to System.Text.Json +semver: feature
WalkthroughBumps Aquality.Selenium.Core to 4.1.0, updates Selenium DevTools references from V138 → V140 (usings and XML docs), replaces versioned DevTools types in tests with non-versioned types, updates test SDK/adapters, relaxes some assertions and wait usage, and guards a cookie-consent click by visibility. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test
participant LocationForm
participant ConsentBtn as consentCookieInfoButton
participant GotIt as gotItCookieButton
participant CloseBtn as closeCookieSettingsButton
Test->>LocationForm: DismissCookieInfo()
rect rgba(200,230,255,0.25)
LocationForm->>ConsentBtn: IsDisplayed?
alt displayed
LocationForm->>ConsentBtn: Click
LocationForm->>ConsentBtn: Wait until not displayed
else not displayed
LocationForm-->>LocationForm: Skip click/wait
end
end
LocationForm->>GotIt: Click if present
LocationForm->>CloseBtn: Click if present
LocationForm-->>Test: Return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs (1)
27-27: Clarify and tighten the URL assertion
Does.StartWith("https://www.google.com/")accommodates Google’s redirects and query parameters but is less precise. Add a brief comment explaining this choice and consider assertingnew Uri(AqualityServices.Browser.CurrentUrl).Host == "www.google.com"to confirm the correct domain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj(1 hunks)Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml(4 hunks)Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs(1 hunks)Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csproj(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/TestApp/MyLocation/LocationForm.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#270
File: Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs:3-4
Timestamp: 2025-05-28T19:52:19.309Z
Learning: DevTools V137 namespaces (OpenQA.Selenium.DevTools.V137.DOM and OpenQA.Selenium.DevTools.V137.Emulation) are available and working in Selenium WebDriver 4.33.0 for .NET projects.
📚 Learning: 2025-05-28T19:52:19.309Z
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#270
File: Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs:3-4
Timestamp: 2025-05-28T19:52:19.309Z
Learning: DevTools V137 namespaces (OpenQA.Selenium.DevTools.V137.DOM and OpenQA.Selenium.DevTools.V137.Emulation) are available and working in Selenium WebDriver 4.33.0 for .NET projects.
Applied to files:
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xmlAquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.csAquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs
📚 Learning: 2024-10-09T11:11:38.254Z
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#264
File: Aquality.Selenium/src/Aquality.Selenium.Images/Aquality.Selenium.Images.csproj:41-48
Timestamp: 2024-10-09T11:11:38.254Z
Learning: In `Aquality.Selenium.Images.csproj`, different versions of `OpenCvSharp4` package references are acceptable because the packages are compatible.
Applied to files:
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj
🧬 Code graph analysis (2)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
Aquality.Selenium/src/Aquality.Selenium/Browsers/AqualityServices.cs (1)
AqualityServices(16-121)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs (1)
Aquality.Selenium/src/Aquality.Selenium/Browsers/Browser.cs (2)
Browser(21-478)Browser(36-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: aquality-automation.aquality-selenium-dotnet (test Analyze code with SonarQube)
- GitHub Check: aquality-automation.aquality-selenium-dotnet (test Run tests)
🔇 Additional comments (9)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
33-37: LGTM!The refactoring from
WaitForTruetoWaitForwith a direct predicate is correct and simplifies the code.Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/TestApp/MyLocation/LocationForm.cs (1)
26-30: LGTM!Adding the
WaitForDisplayedguard before clicking the consent cookie button improves test robustness by handling cases where the button may not appear on all pages.Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.cs (1)
1-1: LGTM!The DevTools namespace update from V138 to V140 is consistent with the broader DevTools API migration in this PR.
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml (4)
481-481: LGTM!XML documentation reference updated from V138 to V140 to match the code changes in
DevToolsEmulationExtensions.cs.
511-511: LGTM!XML documentation reference updated from V138 to V140 to match the code changes in
DevToolsEmulationExtensions.cs.
547-547: LGTM!XML documentation reference updated from V138 to V140 to match the code changes in
DevToolsEmulationExtensions.cs.
566-566: LGTM!XML documentation reference updated from V138 to V140 to match the code changes in
DevToolsEmulationExtensions.cs.Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj (1)
94-94: Confirm Core 4.1.0 JSON migration and issue #274 fix
No directNewtonsoft.Jsonreferences found; please verify that Aquality.Selenium.Core 4.1.0 fully migrated toSystem.Text.Jsonand resolves the JSON reading issues from issue #274 without introducing breaking changes.Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs (1)
3-4: Verify DevTools V140 support. Ensure Selenium.WebDriver 4.36.0 includes OpenQA.Selenium.DevTools.V140.DOM and .Emulation namespaces; update or revert DevTools version as needed.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs
Show resolved
Hide resolved
...ity.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
33-37: Update remainingWaitForTruecalls toWaitFor
All occurrences in tests (e.g., AqualityServicesTests, MouseActionsTests, JavaScriptHandlingTests, FileDownloadingTests, BrowserTests, JsActionsTests, SliderForm) and insrc/Aquality.Selenium/Browsers/Browser.cs:312must be replaced with the newWaitForAPI.Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (1)
8-8: Align DevTools namespace in tests
Replaceusing OpenQA.Selenium.DevTools.V139.Emulation;withusing OpenQA.Selenium.DevTools.V140.Emulation;inDevToolsEmulationTests.csto match the updated extension methods and ensure all command-settings types resolve to V140.
🧹 Nitpick comments (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/TestApp/MyLocation/LocationForm.cs (1)
26-30: Good defensive pattern, but consider consistency.The visibility check before clicking improves test stability by only attempting to click when the button appears. This aligns with the
gotItCookieButtonhandling at lines 31-35.However, note that
closeCookieSettingsButtonat lines 36-39 usesIsDisplayed(immediate check) rather thanWaitForDisplayed()(polling). Consider whether all three cookie buttons should follow the same pattern for consistency, or if the different approaches are intentional based on the specific button behaviors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj(1 hunks)Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml(4 hunks)Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs(1 hunks)Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csproj(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/TestApp/MyLocation/LocationForm.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#270
File: Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs:3-4
Timestamp: 2025-05-28T19:52:19.309Z
Learning: DevTools V137 namespaces (OpenQA.Selenium.DevTools.V137.DOM and OpenQA.Selenium.DevTools.V137.Emulation) are available and working in Selenium WebDriver 4.33.0 for .NET projects.
📚 Learning: 2024-10-09T11:11:38.254Z
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#264
File: Aquality.Selenium/src/Aquality.Selenium.Images/Aquality.Selenium.Images.csproj:41-48
Timestamp: 2024-10-09T11:11:38.254Z
Learning: In `Aquality.Selenium.Images.csproj`, different versions of `OpenCvSharp4` package references are acceptable because the packages are compatible.
Applied to files:
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj
📚 Learning: 2025-05-28T19:52:19.309Z
Learnt from: mialeska
PR: aquality-automation/aquality-selenium-dotnet#270
File: Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs:3-4
Timestamp: 2025-05-28T19:52:19.309Z
Learning: DevTools V137 namespaces (OpenQA.Selenium.DevTools.V137.DOM and OpenQA.Selenium.DevTools.V137.Emulation) are available and working in Selenium WebDriver 4.33.0 for .NET projects.
Applied to files:
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xmlAquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.csAquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.csAquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: aquality-automation.aquality-selenium-dotnet (test Analyze code with SonarQube)
- GitHub Check: aquality-automation.aquality-selenium-dotnet (test Run tests)
🔇 Additional comments (8)
Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsEmulationExtensions.cs (1)
3-4: LGTM! DevTools protocol updated to V140.The namespace update to V140 correctly aligns with the Selenium 4.36.0 upgrade mentioned in the PR title. This is a standard version bump that maintains compatibility with the latest DevTools protocol.
Based on learnings: DevTools V137 namespaces were available in Selenium 4.33.0, so V140 for Selenium 4.36.0 is consistent with the progression.
Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsPerformanceExtensions.cs (1)
1-1: LGTM! DevTools protocol updated to V140.The namespace update to V140 is consistent with the upgrade to Selenium 4.36.0 and matches the DevTools protocol version used in DevToolsEmulationExtensions.cs.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csproj (2)
33-33: LGTM! Minor version update.The NUnit3TestAdapter update from 5.1.0 to 5.2.0 is a minor version bump that should be compatible.
37-37: Verify test execution with Microsoft.NET.Test.Sdk 18.0.0.The update from 17.14.1 to 18.0.0 is a major version bump of the Test SDK, which could introduce breaking changes or behavioral differences in test discovery and execution.
Ensure that all tests run successfully with the new Test SDK version. If pipeline tests have passed, this verification is complete.
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj (1)
94-94: Remove JSON deserialization verification suggestion. This project doesn’t deserialize any JSON configuration or data files—JSON is only used internally for CDP command formatting via System.Text.Json. No further verification required.Likely an incorrect or invalid review comment.
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml (1)
481-481: LGTM! Documentation updated to reflect DevTools V140.The XML documentation references have been correctly updated from V138 to V140, aligning with the DevTools namespace version upgrade.
Also applies to: 511-511, 547-547, 566-566
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (1)
62-67: Migration to non-versioned types looks correct, but verify completeness.The migration from versioned (
OpenQA.Selenium.DevTools.V138.Emulation.*) to non-versioned public types is a good practice that will reduce maintenance burden. However, as noted in the comment on line 8, this migration appears incomplete:
- Lines 62-67, 186: Successfully using non-versioned types
- Lines 111, 119: Still using versioned types from V139 namespace
Consider whether
SetGeolocationOverrideCommandSettingsandClearGeolocationOverrideCommandSettingsshould also be migrated to non-versioned types for consistency.Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Usecases/CustomBrowserFactoryTests.cs (1)
27-27: Clarify necessity of prefix URL assertion
Changing from exact match (Is.EqualTo(url)) to prefix match (Does.StartWith(url)) onCurrentUrlmay hide unexpected query parameters, fragments, or redirects. Confirm if this relaxation is required due to Selenium 4.36.0 or environment-specific trailing-slash behavior; if not, consider trimming trailing slashes or retaining strict equality for full-URL validation.
|
fixes #274