Skip to content

[build] do not create targets for IE for browser tests#17548

Merged
titusfortner merged 1 commit into
trunkfrom
ie-tests
May 23, 2026
Merged

[build] do not create targets for IE for browser tests#17548
titusfortner merged 1 commit into
trunkfrom
ie-tests

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

This change is what prompted #17546

💥 What does this PR do?

Prevents IE test targets from being generated for browser tests
Results in a marginal improvement in building the graph, memory, cache and binary creation impacts depending on which binding

🔧 Implementation Notes

Didn't delete IE from the lists just added lines to skip them to the places that generate the targets

🤖 AI assistance

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

💡 Additional Considerations

These are already filtered out of ci-rbe and aren't running

@selenium-ci selenium-ci added C-py Python Bindings C-rb Ruby Bindings C-dotnet .NET Bindings C-java Java Bindings B-build Includes scripting, bazel and CI integrations labels May 22, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Exclude IE from browser test target generation

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Exclude IE browser from test target generation across multiple build systems
• Create DEFAULT_BROWSERS lists filtering out IE from BROWSERS definitions
• Update test suite functions to use DEFAULT_BROWSERS instead of all browsers
• Reduce build graph complexity and improve build performance
Diagram
flowchart LR
  A["BROWSERS definitions"] -->|filter out IE| B["DEFAULT_BROWSERS lists"]
  B -->|used in| C["Test suite generators"]
  C -->|skip IE targets| D["Reduced build graph"]
  D -->|faster builds| E["Improved performance"]

Loading

File Changes

1. dotnet/private/dotnet_nunit_test_suite.bzl ✨ Enhancement +5/-1

Filter IE from dotnet test targets

• Added _DEFAULT_BROWSERS list that filters out IE from _BROWSERS
• Updated all_browser_data collection to only include browsers in _DEFAULT_BROWSERS
• Added skip condition in test target generation loop to skip IE browsers

dotnet/private/dotnet_nunit_test_suite.bzl


2. java/private/suite.bzl ✨ Enhancement +2/-2

Use DEFAULT_BROWSERS for Java tests

• Changed import from BROWSERS to DEFAULT_BROWSERS in load statement
• Updated java_selenium_test_suite default browsers parameter to use DEFAULT_BROWSERS instead of
 BROWSERS.keys()

java/private/suite.bzl


3. rb/spec/tests.bzl ✨ Enhancement +3/-1

Filter IE from Ruby test targets

• Added DEFAULT_BROWSERS list that filters out IE from BROWSERS.keys()
• Updated rb_integration_test function default browsers parameter to use DEFAULT_BROWSERS

rb/spec/tests.bzl


View more (1)
4. py/BUILD.bazel ✨ Enhancement +4/-2

Filter IE from Python test targets

• Added DEFAULT_BROWSER_TESTS list that filters out IE from BROWSER_TESTS
• Updated non-bidi test target generation to iterate over DEFAULT_BROWSER_TESTS instead of
 BROWSER_TESTS.keys()
• Updated bidi test target generation to iterate over DEFAULT_BROWSER_TESTS instead of
 BROWSER_TESTS.keys()

py/BUILD.bazel


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. Default browser may be skipped 🐞 Bug ☼ Reliability
Description
In dotnet_nunit_test_suite, default_browser is chosen from the unfiltered browsers list while IE
(and now any browser not in the allow-list) is skipped later during wrapper generation, so an
IE-first (or IE-only) list can prevent the per-class native.test_suite(name = test_name, ...)
alias from being created and can yield zero wrapper tests. Additionally, unknown/typo browser
strings are silently ignored via continue rather than failing the build, which can make CI appear
green while intended browser-specific tests are never generated or run.
Code

dotnet/private/dotnet_nunit_test_suite.bzl[R214-215]

Evidence
The macro derives default_browser = browsers[0], but later wrapper targets and the per-class suite
alias are only created when iterating browsers and when browser == default_browser; since IE is
skipped in that later loop, an IE-first configuration means the condition to emit
native.test_suite(name = test_name, ...) is never met, and an IE-only list results in only the
manual csharp_test with no wrapper tests. The dotnet webdriver BUILD file documents/relies on
“first browser is default” semantics, making this order-dependent skip particularly problematic.
Separately, the newly added continue check happens before any _BROWSERS[browser] lookup, so
unknown browser strings are ignored instead of triggering an error; this differs from the repo’s
existing pattern (e.g., Java’s selenium_test macro) of failing fast on unrecognized browsers,
showing the current behavior can silently suppress expected test targets.

dotnet/private/dotnet_nunit_test_suite.bzl[187-226]
dotnet/test/webdriver/BUILD.bazel[34-42]
dotnet/private/dotnet_nunit_test_suite.bzl[187-218]
java/private/selenium_test.bzl[100-105]

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

## Issue description
`dotnet_nunit_test_suite` currently picks `default_browser` from the *unfiltered* `browsers` list, but later skips IE during wrapper generation; if `browsers[0]` is `ie`, the per-class alias `native.test_suite(name = test_name, ...)` is never created, and if the list is only `["ie"]` the macro emits the manual `csharp_test` but generates zero wrapper tests. In addition, the macro now skips any browser not in `_DEFAULT_BROWSERS` via `continue`, which unintentionally causes unknown/typo browser strings to be silently ignored rather than failing the build, leading to missing wrapper targets with no signal.

## Issue Context
BUILD files in this repo document/rely on the convention that the first browser in the list is treated as the default; once IE is filtered out later, that assumption becomes invalid depending on list order and can break suite alias generation. Other bindings/macros in the repo fail fast for unrecognized browser values (e.g., Java), so silently skipping unknown browsers is inconsistent and risks CI passing without running intended browser-specific tests.

## Fix Focus Areas
- dotnet/private/dotnet_nunit_test_suite.bzl[185-236]
- dotnet/test/webdriver/BUILD.bazel[34-42]

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


Grey Divider

Qodo Logo

@titusfortner titusfortner merged commit 5b65356 into trunk May 23, 2026
34 checks passed
@titusfortner titusfortner deleted the ie-tests branch May 23, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-dotnet .NET Bindings C-java Java Bindings C-py Python Bindings C-rb Ruby Bindings D-IE

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants