Update Selenium version to v4.35.0 +semver: feature #273
Conversation
WalkthroughRemoves the obsolete GetDevToolsSession(int) overload, introduces/uses GetDevToolsSession(DevToolsOptions), updates XML docs accordingly, bumps Aquality.Selenium.Core to 3.5.0, updates NUnit dependencies, adds retry categorization and constants for flaky tests, and adjusts DevTools Emulation test code to newer protocol versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test/Client
participant DH as DevToolsHandling
participant DTP as DevToolsProvider
participant DTS as DevToolsSession
rect rgb(235,245,255)
note over Test,DH: Default session retrieval (no options)
Test->>DH: GetDevToolsSession()
DH->>DTP: GetDevToolsSession() // provider default/autodetect
DTP-->>DH: DevToolsSession
DH-->>Test: DevToolsSession
end
rect rgb(240,255,240)
note over Test,DH: Options-based session retrieval
Test->>DH: GetDevToolsSession(DevToolsOptions options)
DH->>DTP: GetDevToolsSession(options)
DTP-->>DH: DevToolsSession
DH-->>Test: DevToolsSession
end
note over Test,DH: Obsolete int overload removed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj (1)
94-94: Aquality.Selenium.Core 3.5.0 bump: confirm Selenium.WebDriver 4.35.0 alignmentThe Core bump makes sense for the DevTools API changes. Please verify that Aquality.Selenium.Core 3.5.0 pins or ranges to Selenium.WebDriver 4.35.0 (or newer) so this library can’t accidentally resolve an older WebDriver via transitive deps. If Core doesn’t enforce that, consider explicitly pinning here.
If needed, add an explicit reference:
<ItemGroup> - <PackageReference Include="Aquality.Selenium.Core" Version="3.5.0" /> + <PackageReference Include="Aquality.Selenium.Core" Version="3.5.0" /> + <!-- Ensure WebDriver 4.35.0 is used if not enforced by Core --> + <PackageReference Include="Selenium.WebDriver" Version="4.35.0" /> <PackageReference Include="WebDriverManager" Version="2.17.6" /> </ItemGroup>Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/UITest.cs (1)
11-12: Centralized retry config: good; consider broader accessibilityDefining RetriesCount/Group once is clean. If you ever need to use these constants from fixtures that don’t inherit UITest, switch to protected internal (or internal) and reference statically from a shared static class.
- protected const int RetriesCount = 5; - protected const string RetriesGroup = "Flaky"; + protected internal const int RetriesCount = 5; + protected internal const string RetriesGroup = "Flaky";Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTabsTests.cs (1)
145-153: Retry added; reduce flakiness at the source with deterministic waits in helperAdding Retry is fine, but the flake likely comes from timing around tab creation. Strengthen CheckSwitchingBy to wait for the new handle(s) rather than relying on immediate availability, and you can likely drop the Retry.
Apply this refactor to the helper (outside the changed hunk) to wait for handles both after opening the external link and after the switchMethod runs:
- private void CheckSwitchingBy(int expectedTabCount, Action switchMethod) - { - var tabHandle = Tabs.CurrentHandle; - Assert.That(tabHandle, Is.Not.Empty); - WelcomeForm.ClickElementalSelenium(); - var newTabHandle = Tabs.Handles.Last(); - switchMethod.Invoke(); - Assert.That(Tabs.CurrentHandle, Is.EqualTo(newTabHandle), "Browser should be switched to correct tab"); - Assert.That(Tabs.Handles.Count, Is.EqualTo(expectedTabCount), "Number of tabs should be correct"); - } + private void CheckSwitchingBy(int expectedTabCount, Action switchMethod) + { + var originalHandle = Tabs.CurrentHandle; + Assert.That(originalHandle, Is.Not.Empty); + + var initialCount = Tabs.Handles.Count; + WelcomeForm.ClickElementalSelenium(); + + // Wait until the link actually opens a new tab + Assert.That(() => Tabs.Handles.Count, Is.EqualTo(initialCount + 1).After(5000).PollEvery(100)); + var newTabHandle = Tabs.Handles.Last(); + + switchMethod.Invoke(); + + // If switchMethod opens/closes tabs, wait for the final expected count + Assert.That(() => Tabs.Handles.Count, Is.EqualTo(expectedTabCount).After(5000).PollEvery(100)); + Assert.That(Tabs.CurrentHandle, Is.EqualTo(newTabHandle), "Browser should be switched to correct tab"); + }If this stabilizes the test, consider removing Retry on this method to keep signal-to-noise high in CI.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Elements/MultiChoiceBoxTests.cs (1)
37-37: Retry categorized as Flaky: consider applying at class level (if most tests here are flaky)If multiple tests in this fixture exhibit the same instability pattern, you can reduce duplication and make intent clearer by decorating the class with [Category(RetriesGroup)] and keep per-test Retry only where needed. Otherwise, current selective use on this single test is fine.
- internal class MultiChoiceBoxTests : UITest + [Category(RetriesGroup)] + internal class MultiChoiceBoxTests : UITestAquality.Selenium/tests/Aquality.Selenium.Tests/Integration/HiddenElementsTests.cs (1)
52-71: Retries added for flaky hidden-elements check — consider lightweight logging before refresh to aid triageThe retry metadata aligns with the PR pattern. Since the test already has an inline recovery (Refresh + second wait), a small log entry when the first wait fails would help diagnose sporadic failures without changing behavior.
Apply this diff to add a debug line before the refresh:
{ - var result = element.State.WaitForNotDisplayed(); + var result = element.State.WaitForNotDisplayed(); if (!result) { + TestContext.WriteLine($"Element still displayed after initial wait. Refreshing and retrying... Element: {element}"); AqualityServices.Browser.Refresh(); result = element.State.WaitForNotDisplayed(); } return result; }));Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTests.cs (1)
229-253: Use TotalSeconds in message; optionally reset implicit wait to avoid side-effects
- The failure message uses
elapsedTime.Seconds, which prints only the seconds component, not the full duration. PreferTotalSeconds.- Consider restoring the implicit wait to a baseline in a finally block so subsequent tests aren’t influenced.
Apply these diffs:
- Improve the assertion message:
- Assert.That(elapsedTime, Is.LessThan(waitTime.Add(TimeSpan.FromSeconds(2))), - $"Elapsed time should be less than implicit timeout + 2 sec(accuracy). Elapsed time: {elapsedTime.Seconds}"); + Assert.That(elapsedTime, Is.LessThan(waitTime.Add(TimeSpan.FromSeconds(2))), + $"Elapsed time should be less than implicit timeout + 2 sec (accuracy). Elapsed time: {elapsedTime.TotalSeconds:F2}s");
- Reset implicit wait after the check:
try { browser.Driver.FindElement(By.Id("not_exist_element")); } catch (NoSuchElementException) { elapsedTime = stopwatch.Elapsed; } + finally + { + // Reset to baseline to avoid leaking this setting into other tests + browser.SetImplicitWaitTimeout(TimeSpan.Zero); + }Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (1)
62-67: Reduce verbosity when mixing V139 default with V138-specific paramsYou intentionally pass V138-specific settings to validate cross-version handling. To keep it readable, add a namespace alias and use it here.
Apply these diffs:
- Add an alias near the existing using directives:
+using V138Emulation = OpenQA.Selenium.DevTools.V138.Emulation;
- Use the alias in the parameter construction:
- var parameters = new OpenQA.Selenium.DevTools.V138.Emulation.SetDeviceMetricsOverrideCommandSettings + var parameters = new V138Emulation.SetDeviceMetricsOverrideCommandSettings { - DisplayFeature = new OpenQA.Selenium.DevTools.V138.Emulation.DisplayFeature + DisplayFeature = new V138Emulation.DisplayFeature { - Orientation = OpenQA.Selenium.DevTools.V138.Emulation.DisplayFeatureOrientationValues.Horizontal + Orientation = V138Emulation.DisplayFeatureOrientationValues.Horizontal }, Width = width, Height = height, Mobile = isMobile, DeviceScaleFactor = scaleFactor };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.csproj(1 hunks)Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml(0 hunks)Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsHandling.cs(0 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csproj(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTabsTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTests.cs(2 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs(5 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Elements/MultiChoiceBoxTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/HiddenElementsTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs(1 hunks)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/UITest.cs(1 hunks)
💤 Files with no reviewable changes (2)
- Aquality.Selenium/src/Aquality.Selenium/Browsers/DevToolsHandling.cs
- Aquality.Selenium/src/Aquality.Selenium/Aquality.Selenium.xml
🧰 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/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csprojAquality.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/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs
🧬 Code graph analysis (5)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/Elements/MultiChoiceBoxTests.cs (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTabsTests.cs (15)
Test(21-29)Test(31-39)Test(41-49)Test(51-56)Test(58-64)Test(66-79)Test(81-94)Test(96-103)Test(105-112)Test(114-121)Test(123-132)Test(134-143)Test(145-153)Test(155-163)Test(165-169)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (11)
Test(30-40)Test(42-48)Test(50-55)Test(57-78)Test(96-103)Test(105-120)Test(147-159)Test(161-176)Test(178-189)Test(191-205)Test(207-214)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTabsTests.cs (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (11)
Test(30-40)Test(42-48)Test(50-55)Test(57-78)Test(96-103)Test(105-120)Test(147-159)Test(161-176)Test(178-189)Test(191-205)Test(207-214)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/HiddenElementsTests.cs (1)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTests.cs (16)
Test(16-23)Test(25-32)Test(34-52)Test(54-62)Test(64-77)Test(79-85)Test(87-92)Test(94-101)Test(103-122)Test(124-130)Test(137-144)Test(146-154)Test(156-185)Test(187-201)Test(203-217)Test(219-227)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (2)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTabsTests.cs (13)
Test(21-29)Test(31-39)Test(41-49)Test(51-56)Test(58-64)Test(66-79)Test(81-94)Test(96-103)Test(105-112)Test(114-121)Test(123-132)Test(134-143)Test(145-153)Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
Test(14-41)
⏰ 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 Run tests)
- GitHub Check: aquality-automation.aquality-selenium-dotnet (test Analyze code with SonarQube)
🔇 Additional comments (7)
Aquality.Selenium/tests/Aquality.Selenium.Tests/Aquality.Selenium.Tests.csproj (1)
32-33: NUnit 4.4.0 + Adapter 5.1.0 – Manual Verification NeededBefore merging, please manually confirm that:
- RetryAttribute is still exposed in
nunit.frameworkin NUnit 4.4.0.- NUnit3TestAdapter 5.1.0 can discover and run NUnit 4.4.0 tests targeting .NET 8 under Microsoft.NET.Test.Sdk 17.14.1 without any discovery or execution errors.
- There are no breaking changes in NUnit 4.4.0 that affect CategoryAttribute or delayed constraints (e.g.
Assert.That(...).After(...)).If you encounter any issues:
- Bump the Test SDK or NUnit3TestAdapter version as needed.
- Adjust or remove any unsupported attributes or APIs.
To understand the full impact, scan for Retry usage across the suite:
#!/bin/bash # List all tests using Retry across the solution rg -nP '\[(Test|TestCase)[^\]]*Retry\(' -g 'Aquality.Selenium/tests/**' -C1—otherwise, we risk intermittent failures slipping through CI.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/ImageLocatorTests.cs (1)
14-41: Add Retry/Category to image-based test — good stability improvementMarking this image-recognition test as Flaky with retries matches the typical instability of visual assertions across environments. No logic concerns found.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/BrowserTests.cs (1)
103-122: Async JS timing test: Retry/Category addition looks goodThe retry metadata is appropriate for timing-sensitive assertions. The current tolerances (±1s op slack) are reasonable.
Aquality.Selenium/tests/Aquality.Selenium.Tests/Integration/DevToolsEmulationTests.cs (4)
96-103: GeoLocation override test — Retry/Category addition is appropriateGood call to mark as Flaky and add retries for a test that depends on browser + site permissions timing.
147-159: UserAgent/Language override test — Retry/Category addition looks goodFits the broader retry rollout; the assertions remain clear and side-effect free.
8-8: Please verify DevTools V139 support in Selenium.WebDriver 4.35.0I’m not able to confirm offline whether the
OpenQA.Selenium.DevTools.V139.Emulationnamespace is shipped in the 4.35.0 .NET package. Could you:
- Inspect the contents of your
Selenium.WebDriver.dll(e.g., via ILSpy/dnSpy) to see if there’s anOpenQA.Selenium.DevTools.V139folder withEmulation.- Check the official release notes or API docs for Selenium 4.35.0 on GitHub to confirm which CDP versions are bundled.
- If V139 isn’t present, fall back to the highest available CDP version (e.g., V138) and update the using/import accordingly.
This will ensure the test references a namespace that actually exists in the shipped package.
105-121: C# 12 empty collection expression is supported by net8.0The test project targets net8.0, which defaults to C# 12 and therefore supports the
[]collection expression for an empty argument list. No change is needed.
No description provided.