Skip to content

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Sep 18, 2025

User description

💥 What does this PR do?

Instead of ignoring some tests for all browsers, we can run them at least on Firefox. So ignoring for Chromium only.

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Tests


Description

  • Enable WebExtension tests for Firefox browser

  • Replace global test ignore with browser-specific ignores

  • Keep Chromium browsers (Chrome/Edge) ignored due to CDP conflicts


Diagram Walkthrough

flowchart LR
  A["Global Ignore"] --> B["Browser-Specific Ignores"]
  B --> C["Firefox Tests Enabled"]
  B --> D["Chrome/Edge Still Ignored"]
Loading

File Walkthrough

Relevant files
Tests
WebExtensionTest.cs
Replace global ignore with browser-specific ignores           

dotnet/test/common/BiDi/WebExtension/WebExtensionTest.cs

  • Removed global [Ignore] attribute from test class
  • Added browser-specific [IgnoreBrowser] attributes for Chrome and Edge
  • Extracted ignore reason into a constant for reusability
  • Removed unused System import
+10/-8   

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Sep 18, 2025
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Attribute Placement

Verify that the new [IgnoreBrowser] attributes at the class level correctly apply to all tests in this fixture for Chrome and Edge in your NUnit setup and that the attribute is available in the test project (namespace/usings). Some NUnit runners require the attribute type to be in scope.

[IgnoreBrowser(Selenium.Browser.Chrome, ChromiumIgnoreReason)]
[IgnoreBrowser(Selenium.Browser.Edge, ChromiumIgnoreReason)]
class WebExtensionTest : BiDiTestFixture
Const Visibility

Consider making ChromiumIgnoreReason private or internal if it isn't intended for external use, or moving it to a shared location if other test classes will reuse it.

public const string ChromiumIgnoreReason = """
    The following test suite wants to set driver arguments via Options, but it breaks CDP/DevTools tests.
    The desired arguments (for Chromium only?):
    --enable-unsafe-extension-debugging
    --remote-debugging-pipe
    Ignoring these tests for now. Hopefully https://github.com/SeleniumHQ/selenium/issues/15536 will be resolved soon.
    """;

Copy link
Contributor

qodo-merge-pro bot commented Sep 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Restrict constant visibility for better encapsulation
Suggestion Impact:The constant visibility was reduced by removing 'public', making it non-public (internal by default), aligning with the suggestion to restrict visibility.

code diff:

-    public const string ChromiumIgnoreReason = """
+    const string ChromiumIgnoreReason = """

Change the ChromiumIgnoreReason constant's visibility from public to private to
improve encapsulation, as it is only used within the WebExtensionTest class.

dotnet/test/common/BiDi/WebExtension/WebExtensionTest.cs [30-36]

-public const string ChromiumIgnoreReason = """
+private const string ChromiumIgnoreReason = """
     The following test suite wants to set driver arguments via Options, but it breaks CDP/DevTools tests.
     The desired arguments (for Chromium only?):
     --enable-unsafe-extension-debugging
     --remote-debugging-pipe
     Ignoring these tests for now. Hopefully https://github.com/SeleniumHQ/selenium/issues/15536 will be resolved soon.
     """;

[Suggestion processed]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies that the ChromiumIgnoreReason constant can be made private to improve encapsulation, as it is only used within its declaring class.

Low
  • Update

@nvborisenko nvborisenko merged commit 4e9ec56 into SeleniumHQ:trunk Sep 18, 2025
10 checks passed
@nvborisenko nvborisenko deleted the bidi-webextension-tests-ff branch September 18, 2025 16:57
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