Skip to content

[dotnet] [test] Stabilize DevToolsTabsTests fixture - #18235

Merged
nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-test-stabilize-devtools
Oct 24, 2026
Merged

nvborisenko merged 2 commits into
SeleniumHQ:trunkfrom
nvborisenko:dotnet-test-stabilize-devtools

Conversation

@nvborisenko

Copy link
Copy Markdown
Member

Introduce synchronization mechanism before assertion.

🔗 Related Issues

DevTools event processing is concurrent, there is a race condition when detachedFromTarget event is not happening in time.

💥 What does this PR do?

This pull request improves the reliability of the ClosingTabDoesNotBreakDevToolsSession test in DevToolsTabsTests.cs by ensuring the test waits for the DevTools "detached from target" event before proceeding. This change addresses potential timing issues when closing a browser tab during DevTools protocol tests.

Test reliability improvements:

  • Added a TaskCompletionSource and event handler to wait for the DetachedFromTarget event before continuing the test, ensuring the test only proceeds after the tab is fully detached.
  • Subscribed and unsubscribed the DetachedFromTarget event handler within a try/finally block to ensure proper cleanup and avoid side effects on other tests.

General cleanup:

  • Removed an unnecessary blank line for code style consistency.

🔧 Implementation Notes

🤖 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

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_describe

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Oct 24, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Stabilize DevTools tab-close test with target-detachment synchronization

🧪 Tests 🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Wait for the closed tab’s DevTools detachment event before checking session usability.
• Bound the wait to five seconds and remove the event handler after the test.
Diagram

sequenceDiagram
    participant Test as Tabs test
    participant Target as Target domain
    participant Driver as WebDriver
    participant Browser as Browser
    participant Wait as Completion task
    participant Console as Console domain
    Test->>Target: Subscribe to detachment
    Test->>Driver: Close original tab
    Driver->>Browser: Close tab
    Browser-->>Target: DetachedFromTarget event
    Target->>Wait: Complete for matching ID
    Wait-->>Test: Resume within timeout
    Test->>Console: Enable and assert
    Test->>Target: Unsubscribe
Loading
High-Level Assessment

Keep the bounded, event-driven wait. It synchronizes on the condition the test needs, unlike an arbitrary delay, and the try/finally subscription cleanup limits effects on other tests.

Files changed (1) +21 / -8

Tests (1) +21 / -8
DevToolsTabsTests.csWait for target detachment before asserting session usability +21/-8

Wait for target detachment before asserting session usability

• The tab-close test now subscribes to DetachedFromTarget before closing the original tab and waits up to five seconds for an event with the matching target ID. It then checks that the Console domain can still be enabled and removes the handler in a finally block.

dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs

@qodo-code-review

qodo-code-review Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Informational

1. A blank line keeps trailing spaces ✓ Resolved
Description
The new blank line between the event subscription and try block contains trailing spaces. The
changed file therefore violates the repository's whitespace-formatting convention and adds avoidable
formatter noise to this test.
Code

dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50]

+        
Evidence
The changed line contains whitespace-only content after the indentation, while the repository's C#
editor configuration enables dotnet format whitespace checks.

AGENTS.md: Keep Changes Consistent With Repository Formatting
dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50-50]

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 new blank line contains trailing whitespace, which is inconsistent with the repository's formatting rules.

## Fix Focus Areas
- dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50-50]

## Recommended Fix
Remove the trailing spaces from the blank line and run the repository formatter or whitespace checks.

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


Grey Divider

Context sources
Review mode: Auto: ⏭️ Skipped: Pure whitespace-only formatting change with no semantic effect.
✅ Fix-loop round: fix loop

Grey Divider

Tip of the day
💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit ff7bfb2 ⏭️ Skipped

Results up to commit 212a6c5 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Informational
1. A blank line keeps trailing spaces ✓ Resolved
Description
The new blank line between the event subscription and try block contains trailing spaces. The
changed file therefore violates the repository's whitespace-formatting convention and adds avoidable
formatter noise to this test.
Code

dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50]

+        
Evidence
The changed line contains whitespace-only content after the indentation, while the repository's C#
editor configuration enables dotnet format whitespace checks.

AGENTS.md: Keep Changes Consistent With Repository Formatting
dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50-50]

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 new blank line contains trailing whitespace, which is inconsistent with the repository's formatting rules.

## Fix Focus Areas
- dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs[50-50]

## Recommended Fix
Remove the trailing spaces from the blank line and run the repository formatter or whitespace checks.

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


Grey Divider

Qodo Logo

Comment thread dotnet/test/webdriver/DevTools/DevToolsTabsTests.cs Outdated
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@nvborisenko
nvborisenko merged commit 301eec3 into SeleniumHQ:trunk Oct 24, 2026
29 checks passed
@nvborisenko
nvborisenko deleted the dotnet-test-stabilize-devtools branch October 24, 2026 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants