[dotnet] [test] Clean driver fixture from expected page titles

[nvborisenko]

Tests are responsible for expected values. Hardcoded, not a big deal.

Contributes to #15536

🔧 Implementation Notes

Hardcode? No. Yes, but feasible.

🔄 Types of changes

• Cleanup (formatting, renaming)

[qodo-code-review]

PR Summary by Qodo

[dotnet][tests] Inline expected page titles and remove them from DriverTestFixture
🧪 Tests 🕐 10-20 Minutes

Description

• Remove expected page title constants from the shared driver test fixture.
• Inline expected titles directly in navigation-related test assertions.
• Reduce fixture coupling so tests own their expected values.

Diagram

graph TD
  NavTests["NavigationTests"] --> Fixture["DriverTestFixture"] --> EnvMgr["EnvironmentManager"] --> Urls["WebServer Urls"]
  SessTests["SessionHandlingTests"] --> Fixture
  NavTests --> Titles["Inline expected titles"]
  SessTests --> Titles

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move titles to a dedicated PageTitles constants class (test-only)

• ➕ Centralizes expected values without coupling them to driver lifecycle concerns
• ➕ Avoids repeating strings across multiple test classes
• ➖ Reintroduces indirection for simple assertions
• ➖ Can become a dumping ground for unrelated expectations if not curated

2. Adopt page-object helpers that expose expected title per page

• ➕ Scales better as assertions expand beyond titles (locators, helpers, common flows)
• ➕ Keeps expectations grouped with page semantics
• ➖ More code and structure than needed for a handful of title assertions
• ➖ Higher upfront refactor cost for minimal immediate benefit

Recommendation: The PR’s approach is appropriate here: expected values belong with the tests asserting them, and DriverTestFixture should remain focused on driver/environment concerns. If title reuse grows substantially, consider a small test-only PageTitles constants class (or page objects) to reduce duplication without reverting to fixture-level coupling.

Files changed (3) +13 / -23

Refactor (1) +0 / -10

DriverTestFixture.csRemove shared expected page title fields from the base fixture +0/-10

Remove shared expected page title fields from the base fixture

• Deletes several public string fields containing expected page titles. This keeps DriverTestFixture focused on driver setup/teardown and shared helpers rather than test expectations.

dotnet/test/webdriver/DriverTestFixture.cs

Tests (2) +13 / -13

NavigationTests.csInline expected page titles in navigation assertions +12/-12

Inline expected page titles in navigation assertions

• Replaces fixture-sourced title variables with string literals in multiple sync/async navigation tests. Keeps each test’s expected title local to the assertion.

dotnet/test/webdriver/NavigationTests.cs

SessionHandlingTests.csInline expected title assertion for SimpleTestPage +1/-1

Inline expected title assertion for SimpleTestPage

• Updates the repeated open/close browser loop test to assert the page title using a local string literal instead of a fixture field.

dotnet/test/webdriver/SessionHandlingTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 15 rules

1. Public titles removed without deprecation ✓ Resolved 📘 Rule violation ☼ Reliability

Description

Several public string title members were removed from DriverTestFixture without a deprecation
phase or guidance to a replacement, which violates the project deprecation/backward-compatibility
policy. This can immediately break downstream consumers (including derived test classes) that
referenced these public members and cause compile failures.

Code

dotnet/test/webdriver/DriverTestFixture.cs[L30-38]

-    public string macbethTitle = "Macbeth: Entire Play";
-
-    public string simpleTestTitle = "Hello WebDriver";
-
-    public string framesTitle = "This page has frames";
-
-    public string iframesTitle = "This page has iframes";
-
-    public string formsTitle = "We Leave From Here";

Evidence

Rule 389271 requires that any removed/broken public API first be deprecated and include an explicit
alternative, and Rule 389266 prohibits removing/renaming existing public symbols; the cited diff
removes multiple previously-available public string fields from DriverTestFixture (e.g.,
macbethTitle, simpleTestTitle) and updates tests to use hardcoded string literals instead of
those members, demonstrating that the public surface was changed without an [Obsolete(...)]
deprecation period or replacement guidance.

Rule 389271: Deprecate public APIs with guidance before removal
Rule 389266: Maintain backward-compatible public API and ABI
dotnet/test/webdriver/NavigationTests.cs[42-50]
dotnet/test/webdriver/SessionHandlingTests.cs[149-157]
dotnet/test/webdriver/DriverTestFixture.cs[26-32]

Agent prompt

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

## Issue description
The PR removes public fields from `DriverTestFixture` (e.g., `macbethTitle`, `simpleTestTitle`) without first deprecating them and providing guidance to a replacement, which is a breaking change for any consumers compiled against the previous assembly.

## Issue Context
Project policy requires public APIs to be deprecated for a deprecation period before removal, and the deprecation should include an explicit message naming the replacement (commonly via `[Obsolete(...)]`). These identifiers were previously referenced from tests and could also be referenced by other test classes deriving from `DriverTestFixture`, so removing them can cause immediate compile failures.

## Fix Focus Areas
- dotnet/test/webdriver/DriverTestFixture.cs[30-38]

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

---

2. Unawaited GoToUrlAsync 🐞 Bug ☼ Reliability

Description

NavigationTests.ShouldGoToUrlUsingUriAsync calls GoToUrlAsync(...) without awaiting it (and the test
is not async), so the title assertions can execute before navigation completes and intermittently
fail or test the wrong page.

Code

dotnet/test/webdriver/NavigationTests.cs[R154-157]

        var navigation = driver.Navigate();

        navigation.GoToUrlAsync(new Uri(Urls.MacbethPage));
-        Assert.That(macbethTitle, Is.EqualTo(driver.Title));
+        Assert.That("Macbeth: Entire Play", Is.EqualTo(driver.Title));

Evidence

The test invokes GoToUrlAsync without awaiting it, then immediately reads driver.Title. The
underlying GoToUrlAsync implementation awaits a remote command (ExecuteAsync), so without
awaiting, the test can observe the old page title before navigation finishes.

dotnet/test/webdriver/NavigationTests.cs[140-160]
dotnet/src/webdriver/Navigator.cs[99-111]

Agent prompt

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

### Issue description
`ShouldGoToUrlUsingUriAsync` calls `navigation.GoToUrlAsync(...)` without awaiting the returned `Task`, but immediately asserts on `driver.Title`. Because `GoToUrlAsync` is truly asynchronous (it awaits `ExecuteAsync`), this can race and make the test flaky.

### Issue Context
In this repo, `Navigator.GoToUrlAsync(string url)` is `async` and awaits `driver.ExecuteAsync(...)`. In `NavigationTests`, other async navigation tests properly `await navigation.GoToUrlAsync(...)`, implying this test should too.

### Fix
- Change `ShouldGoToUrlUsingUriAsync` to `public async Task ShouldGoToUrlUsingUriAsync()`.
- `await navigation.GoToUrlAsync(new Uri(Urls.MacbethPage));` before asserting `driver.Title`.
- Optionally make both navigations consistent (use async for both, or use sync for both).

### Fix Focus Areas
- dotnet/test/webdriver/NavigationTests.cs[152-160]

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

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit ffbcfe0

[qodo-code-review]

Code Review by Qodo

No Changes in PR

Qodo reviewed your PR and found no changes in the code

[qodo-code-review]

Code review by qodo was updated up to the latest commit d700b80