Skip to content

[dotnet] [bidi] Unignore some internal tests#16968

Merged
nvborisenko merged 3 commits intoSeleniumHQ:trunkfrom
nvborisenko:bidi-unignore-tests
Jan 22, 2026
Merged

[dotnet] [bidi] Unignore some internal tests#16968
nvborisenko merged 3 commits intoSeleniumHQ:trunkfrom
nvborisenko:bidi-unignore-tests

Conversation

@nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Jan 21, 2026

User description

💥 What does this PR do?

Include some tests into regular CI run.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Tests


Description

  • Remove browser ignore attributes from BiDi tests

  • Tests now run on Firefox, Chrome, and Edge

  • Update assertion to handle filename variations

  • Enable previously skipped test coverage


Diagram Walkthrough

flowchart LR
  A["BiDi Tests<br/>with IgnoreBrowser"] -- "Remove ignore<br/>attributes" --> B["Tests enabled<br/>on all browsers"]
  C["Assertion<br/>exact match"] -- "Update to<br/>flexible match" --> D["Assertion handles<br/>filename variations"]
  B --> E["Expanded test<br/>coverage"]
  D --> E
Loading

File Walkthrough

Relevant files
Tests
BrowsingContextEventsTest.cs
Enable Firefox tests with flexible assertions                       

dotnet/test/common/BiDi/BrowsingContext/BrowsingContextEventsTest.cs

  • Remove [IgnoreBrowser(Selenium.Browser.Firefox)] attributes from two
    test methods
  • Update assertion for SuggestedFilename to use Does.Contain() instead
    of exact equality
  • Allows tests to run on Firefox with flexible filename matching
+1/-3     
EmulationTest.cs
Enable emulation tests on Firefox and other browsers         

dotnet/test/common/BiDi/Emulation/EmulationTest.cs

  • Remove [IgnoreBrowser] attributes from 6 test methods across multiple
    browsers
  • Tests CanSetTimezoneOverride, CanSetTimezoneOverrideToDefault,
    CanSetUserAgentOverride, CanSetUserAgentOverrideToDefault,
    CanSetScreenOrientationOverride, and
    CanSetScreenOrientationOverrideToDefault now run on all browsers
  • Keep Firefox ignore only for CanSetScriptingEnabled and
    CanSetScriptingEnabledToDefault tests
  • Remove Chrome and Edge ignores from CanSetScreenSettingsOverride test
+0/-15   
NetworkTest.cs
Enable network extra headers test                                               

dotnet/test/common/BiDi/Network/NetworkTest.cs

  • Remove [IgnoreBrowser] attributes for Chrome, Edge, and Firefox from
    CanSetExtraHeaders test
  • Enables network extra headers test across all supported browsers
+0/-3     

@nvborisenko nvborisenko marked this pull request as draft January 21, 2026 21:11
@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Jan 21, 2026
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use regex for filename matching

Replace the Does.Contain assertion with a more precise Does.Match regex to
validate the filename format, including optional increments.

dotnet/test/common/BiDi/BrowsingContext/BrowsingContextEventsTest.cs [44]

-Assert.That(eventArgs.SuggestedFilename, Does.Contain("file_1")); // Avoid incremental prefix
+Assert.That(eventArgs.SuggestedFilename, Does.Match(@"^file_1(?:\(\d+\))?\.txt$"));
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The PR changes a strict equality check to a loose Does.Contain, and this suggestion correctly points out that a regex match would be more precise and robust, preventing potential false positives while still allowing for filename increments.

Low
Convert to async Task test

Refactor the test method to be an async Task and directly await the asynchronous
call, removing the need for Assert.That(..., Throws.Nothing).

dotnet/test/common/BiDi/Emulation/EmulationTest.cs [49-53]

-Assert.That(async () =>
+[Test]
+public async Task CanSetUserAgentOverride()
 {
     await bidi.Emulation.SetUserAgentOverrideAsync("MyUserAgent/1.0", new() { Contexts = [context] });
-},
-Throws.Nothing);
+}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that using Assert.That(async lambda, Throws.Nothing) is redundant for an async test and proposes a more modern and readable async Task pattern, which simplifies the test implementation.

Low
Learned
best practice
Guard nullable event fields

Assert eventArgs.SuggestedFilename is not null/empty before applying substring
matching so failures are clearer and don't depend on matcher behavior with
nulls.

dotnet/test/common/BiDi/BrowsingContext/BrowsingContextEventsTest.cs [44]

+Assert.That(eventArgs.SuggestedFilename, Is.Not.Null.And.Not.Empty);
 Assert.That(eventArgs.SuggestedFilename, Does.Contain("file_1")); // Avoid incremental prefix
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add explicit validation/null guards at integration boundaries (event/message fields) before using them.

Low
  • More

@nvborisenko nvborisenko marked this pull request as ready for review January 22, 2026 09:56
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use DoesNotThrowAsync for async

Replace Assert.That(async () => ..., Throws.Nothing) with await
Assert.DoesNotThrowAsync(...) for asserting no exceptions in asynchronous code,
improving readability and adhering to NUnit conventions.

dotnet/test/common/BiDi/Emulation/EmulationTest.cs [49-53]

-Assert.That(async () =>
-{
-    await bidi.Emulation.SetUserAgentOverrideAsync("MyUserAgent/1.0", new() { Contexts = [context] });
-},
-Throws.Nothing);
+await Assert.DoesNotThrowAsync(async () =>
+    await bidi.Emulation.SetUserAgentOverrideAsync("MyUserAgent/1.0", new() { Contexts = [context] }));

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly recommends using the more idiomatic Assert.DoesNotThrowAsync for asserting that an async operation does not throw an exception, which improves code readability and aligns with NUnit best practices.

Medium
Match exact filename suffix

Replace Does.Contain("file_1") with Does.EndWith("file_1.txt") to create a more
precise assertion that correctly validates the filename and extension while
allowing for browser-added prefixes.

dotnet/test/common/BiDi/BrowsingContext/BrowsingContextEventsTest.cs [44]

-Assert.That(eventArgs.SuggestedFilename, Does.Contain("file_1")); // Avoid incremental prefix
+Assert.That(eventArgs.SuggestedFilename, Does.EndWith("file_1.txt")); // Exact filename suffix
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that Does.Contain is too permissive for the test's intent and proposes a more robust assertion using Does.EndWith, improving the test's reliability.

Low
  • More

@nvborisenko nvborisenko merged commit 1a6c9f5 into SeleniumHQ:trunk Jan 22, 2026
19 checks passed
@nvborisenko nvborisenko deleted the bidi-unignore-tests branch January 22, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants