Skip to content

[dotnet] [test] Eliminate stub driver for testing events in Support package - #17736

Merged
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-test-stub-driver
Jul 1, 2026
Merged

[dotnet] [test] Eliminate stub driver for testing events in Support package#17736
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-test-stub-driver

Conversation

@nvborisenko

Copy link
Copy Markdown
Member

Remove unnecessary StubDriver.

🔗 Related Issues

Contributes to #15536

🔧 Implementation Notes

Use mocking.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-support Issue or PR related to support classes labels Jul 1, 2026
@selenium-ci

Copy link
Copy Markdown
Member

Thank you, @nvborisenko for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

dotnet: Replace StubDriver with Moq in EventFiringWebDriver tests

🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Replace StubDriver usage with Mock in event-firing driver tests.
• Assert wrapped driver and event args reference the mocked driver instance.
• Remove the now-unused ChildDriver/stub-based test scaffolding.
Diagram

graph TD
  T["EventFiringWebDriverTests"] --> M["Moq Mock<IWebDriver>"] --> E["EventFiringWebDriver"] --> W["WrappedDriver"] --> A["WebDriver*EventArgs.Driver"]
Loading
High-Level Assessment

Current approach is appropriate: the test suite already uses Moq heavily, and replacing a bespoke StubDriver reduces maintenance while keeping assertions focused on identity (Is.SameAs) rather than behavior.

Files changed (1) +5 / -10

Tests (1) +5 / -10
EventFiringWebDriverTests.csUse Moq driver instead of StubDriver for wrapper/event identity assertions +5/-10

Use Moq driver instead of StubDriver for wrapper/event identity assertions

• Updates tests to build 'EventFiringWebDriver' from 'mockDriver.Object' and verifies 'WrappedDriver' and event args reference the same mocked instance. Removes the unused 'ChildDriver : StubDriver' test scaffolding.

dotnet/test/support/Events/EventFiringWebDriverTests.cs

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 15 rules

Grey Divider


Action required

1. mockDriver replaces StubDriver 📘 Rule violation ▣ Testability
Description
The updated tests replace a concrete IWebDriver implementation with Moq-based mocks, which
violates the requirement to avoid mocks in tests unless backed by a machine-checked contract or
real/contract-driven integration. This can reduce test fidelity and allow tests to pass while
diverging from real WebDriver behavior.
Code

dotnet/test/support/Events/EventFiringWebDriverTests.cs[R222-234]

+        EventFiringWebDriver testDriver = new EventFiringWebDriver(mockDriver.Object);
    }

    [Test]
    public void ShouldBeAbleToAccessWrappedInstanceFromEventCalls()
    {
-        var stubDriver = new StubDriver();
-        EventFiringWebDriver testDriver = new EventFiringWebDriver(stubDriver);
-        StubDriver wrapped = ((IWrapsDriver)testDriver).WrappedDriver as StubDriver;
-        Assert.That(wrapped, Is.EqualTo(stubDriver));
+        EventFiringWebDriver testDriver = new EventFiringWebDriver(mockDriver.Object);
+        IWebDriver wrapped = ((IWrapsDriver)testDriver).WrappedDriver;
+        Assert.That(wrapped, Is.SameAs(mockDriver.Object));
        testDriver.Navigating += new EventHandler<WebDriverNavigationEventArgs>((sender, e) =>
        {
-            Assert.That(stubDriver, Is.EqualTo(e.Driver));
+            Assert.That(e.Driver, Is.SameAs(mockDriver.Object));
        });
Evidence
PR Compliance ID 389270 disallows mocking-framework-based tests without a documented,
machine-checked contract. The changed tests now construct EventFiringWebDriver using
mockDriver.Object and assert event args against that mock, showing the PR moved away from a
concrete driver implementation toward Moq-based mocking.

Rule 389270: Avoid mocks in tests; use real or contract-driven integrations
dotnet/test/support/Events/EventFiringWebDriverTests.cs[218-234]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests in `EventFiringWebDriverTests.cs` use Moq mocks (e.g., `mockDriver.Object`) instead of a real/contract-driven integration or a simple in-memory fake implementation, which violates the "Avoid mocks in tests" compliance rule.

## Issue Context
This PR explicitly replaces `StubDriver` usage with mocking (`mockDriver.Object`). If these tests are meant to validate event wiring and wrapper behavior, a lightweight fake `IWebDriver`/`INavigation` implementation (in-memory, no mocking framework) can preserve deterministic behavior without relying on a mocking library.

## Fix Focus Areas
- dotnet/test/support/Events/EventFiringWebDriverTests.cs[218-237]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Public tests lack <summary> 📘 Rule violation ✧ Quality
Description
The modified public test methods do not have XML documentation comments with a non-empty <summary>
immediately preceding their declarations. This violates the requirement that all new or modified
public members include <summary> documentation.
Code

dotnet/test/support/Events/EventFiringWebDriverTests.cs[R219-230]

    public void ShouldBeAbleToWrapSubclassesOfSomethingImplementingTheWebDriverInterface()
    {
        // We should get this far
-        EventFiringWebDriver testDriver = new EventFiringWebDriver(new ChildDriver());
+        EventFiringWebDriver testDriver = new EventFiringWebDriver(mockDriver.Object);
    }

    [Test]
    public void ShouldBeAbleToAccessWrappedInstanceFromEventCalls()
    {
-        var stubDriver = new StubDriver();
-        EventFiringWebDriver testDriver = new EventFiringWebDriver(stubDriver);
-        StubDriver wrapped = ((IWrapsDriver)testDriver).WrappedDriver as StubDriver;
-        Assert.That(wrapped, Is.EqualTo(stubDriver));
+        EventFiringWebDriver testDriver = new EventFiringWebDriver(mockDriver.Object);
+        IWebDriver wrapped = ((IWrapsDriver)testDriver).WrappedDriver;
+        Assert.That(wrapped, Is.SameAs(mockDriver.Object));
Evidence
PR Compliance ID 389245 requires an XML doc comment with <summary> for all public members that are
new or modified in the diff. The touched public test methods shown in the cited region have [Test]
attributes and method declarations with no preceding /// <summary>... block.

Rule 389245: Require XML documentation with <summary> for all public API members
dotnet/test/support/Events/EventFiringWebDriverTests.cs[197-230]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Modified public members (test methods) are missing XML documentation blocks containing a non-empty `<summary>`.

## Issue Context
In `EventFiringWebDriverTests.cs`, the PR changes method bodies inside public test methods, making them "modified public members" for the purposes of the documentation rule. The declarations are preceded by `[Test]` but not by `///` XML docs.

## Fix Focus Areas
- dotnet/test/support/Events/EventFiringWebDriverTests.cs[197-237]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Subclass wrapping test weakened 🐞 Bug ⚙ Maintainability
Description
ShouldBeAbleToWrapSubclassesOfSomethingImplementingTheWebDriverInterface now constructs
EventFiringWebDriver with a Moq proxy instead of an actual subclass instance, so it no longer
exercises the inheritance/subclass scenario the test name claims to cover. This reduces regression
coverage for type-handling changes in the future.
Code

dotnet/test/support/Events/EventFiringWebDriverTests.cs[R219-223]

    public void ShouldBeAbleToWrapSubclassesOfSomethingImplementingTheWebDriverInterface()
    {
        // We should get this far
-        EventFiringWebDriver testDriver = new EventFiringWebDriver(new ChildDriver());
+        EventFiringWebDriver testDriver = new EventFiringWebDriver(mockDriver.Object);
    }
Evidence
The test name indicates it should cover wrapping subclasses, but its only action is instantiating
EventFiringWebDriver with mockDriver.Object, which is just a mocked IWebDriver interface
implementation, not a concrete subclass instance.

dotnet/test/support/Events/EventFiringWebDriverTests.cs[218-223]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A test named to validate wrapping *subclasses* now wraps `mockDriver.Object` (an interface proxy). This no longer validates that `EventFiringWebDriver` can be constructed with a real class derived from another concrete `IWebDriver` implementation.

### Issue Context
The PR removes `StubDriver`/`ChildDriver`, which previously provided a concrete inheritance chain for this test. The current test body only instantiates `EventFiringWebDriver` with a mock, which does not represent the original scenario.

### Fix Focus Areas
- dotnet/test/support/Events/EventFiringWebDriverTests.cs[218-223]

### Suggested fix
Either:
1) Reintroduce a *minimal* concrete base/child driver pair inside the test file (e.g., `private class BaseDriver : IWebDriver { ... }` and `private class ChildDriver : BaseDriver { }`) with no-op/throwing members as needed, and use `new ChildDriver()` in this test; **or**
2) Rename the test to reflect what it actually verifies (basic construction with an `IWebDriver` instance), since the subclass-specific intent is no longer covered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread dotnet/test/support/Events/EventFiringWebDriverTests.cs
Comment thread dotnet/test/support/Events/EventFiringWebDriverTests.cs
Comment thread dotnet/test/support/Events/EventFiringWebDriverTests.cs
@nvborisenko
nvborisenko merged commit 32aa93f into SeleniumHQ:trunk Jul 1, 2026
24 checks passed
@nvborisenko
nvborisenko deleted the dotnet-test-stub-driver branch July 1, 2026 19:25
This was referenced Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-support Issue or PR related to support classes C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants