Skip to content

[dotnet] [test] Return safari back#17280

Merged
nvborisenko merged 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:dotnet-test-safari
Mar 30, 2026
Merged

[dotnet] [test] Return safari back#17280
nvborisenko merged 1 commit intoSeleniumHQ:trunkfrom
nvborisenko:dotnet-test-safari

Conversation

@nvborisenko
Copy link
Copy Markdown
Member

@nvborisenko nvborisenko commented Mar 30, 2026

Return safari webdriver tests back as bazel targets.

💥 What does this PR do?

This pull request makes a small change to the browser configuration in the dotnet/test/webdriver/BUILD.bazel file by enabling Safari browser tests, which were previously skipped.

  • Enabled Safari browser in the list of browsers for the dotnet_nunit_test_suite, allowing tests to run on Safari.

🔄 Types of changes

  • Cleanup (formatting, renaming)

@nvborisenko nvborisenko marked this pull request as draft March 30, 2026 21:39
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Re-enable Safari webdriver tests in dotnet bazel configuration

🧪 Tests

Grey Divider

Walkthroughs

Description
• Re-enable Safari browser testing in dotnet webdriver test suite
• Uncomment Safari from bazel test targets configuration

Grey Divider

File Changes

1. dotnet/test/webdriver/BUILD.bazel 🧪 Tests +1/-1

Re-enable Safari in dotnet webdriver test targets

• Uncommented safari from the browsers list in dotnet_nunit_test_suite
• Removed the skip comment indicating Safari tests are now active

dotnet/test/webdriver/BUILD.bazel


Grey Divider

Qodo Logo

@selenium-ci selenium-ci added C-dotnet .NET Bindings B-build Includes scripting, bazel and CI integrations labels Mar 30, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Mar 30, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Advisory comments

1. Safari tests skipped on CI 🐞 Bug ⛯ Reliability
Description
The re-enabled Safari Bazel test targets won’t execute in the default GitHub Actions CI run because
CI uses --config=rbe-ci which inherits test:rbe --test_tag_filters=-skip-rbe, and Safari tests
are tagged skip-rbe. As a result, Safari coverage remains absent in the default CI job despite the
targets being restored.
Code

dotnet/test/webdriver/BUILD.bazel[34]

+        "safari",
Evidence
The WebDriver test suite now includes "safari" in its browsers list. The .NET NUnit test suite
macro assigns Safari tests the skip-rbe tag. The rbe Bazel config excludes any skip-rbe tests
via --test_tag_filters=-skip-rbe, and the CI script runs tests with --config=rbe-ci (which
composes rbe), so Safari targets are filtered out during CI test runs.

dotnet/test/webdriver/BUILD.bazel[25-38]
dotnet/private/dotnet_nunit_test_suite.bzl[83-94]
.bazelrc.remote[35-37]
.bazelrc.remote[59-63]
scripts/github-actions/ci-build.sh[18-24]

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

### Issue description
Safari WebDriver Bazel test targets are restored, but they will not run in the default CI because `--config=rbe-ci` inherits `test:rbe --test_tag_filters=-skip-rbe`, and Safari tests are tagged `skip-rbe`.

### Issue Context
- Safari wrapper tests are tagged `skip-rbe`.
- The CI test invocation uses `--config=rbe-ci`.
- The `rbe` config filters out `skip-rbe` tests, so Safari targets won’t execute under this CI configuration.

### Fix Focus Areas
- scripts/github-actions/ci-build.sh[18-24]
- .bazelrc.remote[35-37]
- dotnet/private/dotnet_nunit_test_suite.bzl[83-94]

### Suggested fix
Add a separate CI job (typically macOS runner) that runs the Safari targets without the `rbe` config (or with tag filters that include Safari), so Safari tests actually execute somewhere in CI. Alternatively, if the intent is only to restore the targets for local execution, explicitly document in the PR/CI docs that Safari targets are excluded from `rbe-ci` runs.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@nvborisenko nvborisenko marked this pull request as ready for review March 30, 2026 22:05
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Re-enable Safari webdriver tests in dotnet bazel configuration

🧪 Tests

Grey Divider

Walkthroughs

Description
• Re-enable Safari browser testing in dotnet webdriver test suite
• Uncomment Safari from bazel test targets configuration

Grey Divider

File Changes

1. dotnet/test/webdriver/BUILD.bazel 🧪 Tests +1/-1

Re-enable Safari in dotnet webdriver test targets

• Uncommented safari from the browsers list in dotnet_nunit_test_suite
• Removed the skip comment indicating Safari was temporarily disabled
• Safari is now included in the test targets alongside firefox, ie, edge, and chrome

dotnet/test/webdriver/BUILD.bazel


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Mar 30, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Safari tests lack exclusivity 🐞 Bug ⛯ Reliability ⭐ New
Description
Adding "safari" to the .NET webdriver Bazel suite generates many Safari wrapper test targets that
Bazel may run concurrently on macOS. Safari is treated as non-parallelizable elsewhere in the repo
(tagged "exclusive-if-local"), but the .NET Safari targets are only tagged "skip-rbe", making
concurrent SafariDriver sessions likely to cause flakes/timeouts.
Code

dotnet/test/webdriver/BUILD.bazel[34]

+        "safari",
Evidence
The PR enables Safari in the browsers list for the dotnet_nunit_test_suite, which creates one
wrapper test per test class per browser. The .NET suite’s Safari browser config adds only "skip-rbe"
(no exclusivity), while Ruby and Java explicitly tag Safari tests as exclusive/non-parallel.

dotnet/test/webdriver/BUILD.bazel[25-38]
dotnet/private/dotnet_nunit_test_suite.bzl[83-94]
dotnet/private/dotnet_nunit_test_suite.bzl[193-217]
rb/spec/tests.bzl[142-149]
java/private/selenium_test.bzl[53-72]

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

## Issue description
Enabling Safari adds many `*-safari` Bazel test targets for .NET WebDriver tests. These targets are not marked as exclusive/non-parallelizable, so Bazel can schedule multiple Safari targets at once on macOS, leading to flaky/hanging SafariDriver sessions.

## Issue Context
Other language suites in this repo already tag Safari tests as `exclusive-if-local` (and comment that Safari cannot run in parallel). The .NET Bazel macro currently tags Safari only with `skip-rbe`.

## Fix Focus Areas
- dotnet/private/dotnet_nunit_test_suite.bzl[83-94]
- dotnet/private/dotnet_nunit_test_suite.bzl[193-217]

## Suggested change
Add an exclusivity tag for Safari wrapper tests generated by `dotnet_nunit_test_suite`, e.g. include `"exclusive"` (Bazel-native) and/or align with repo convention by adding `"exclusive-if-local"` to `_BROWSERS["safari"]["tags"]`. Ensure the tag is applied to the wrapper test targets (not just the compiled binary).

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



Advisory comments

2. Safari tests skipped on CI 🐞 Bug ⛯ Reliability
Description
The re-enabled Safari Bazel test targets won’t execute in the default GitHub Actions CI run because
CI uses --config=rbe-ci which inherits test:rbe --test_tag_filters=-skip-rbe, and Safari tests
are tagged skip-rbe. As a result, Safari coverage remains absent in the default CI job despite the
targets being restored.
Code

dotnet/test/webdriver/BUILD.bazel[34]

+        "safari",
Evidence
The WebDriver test suite now includes "safari" in its browsers list. The .NET NUnit test suite
macro assigns Safari tests the skip-rbe tag. The rbe Bazel config excludes any skip-rbe tests
via --test_tag_filters=-skip-rbe, and the CI script runs tests with --config=rbe-ci (which
composes rbe), so Safari targets are filtered out during CI test runs.

dotnet/test/webdriver/BUILD.bazel[25-38]
dotnet/private/dotnet_nunit_test_suite.bzl[83-94]
.bazelrc.remote[35-37]
.bazelrc.remote[59-63]
scripts/github-actions/ci-build.sh[18-24]

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

### Issue description
Safari WebDriver Bazel test targets are restored, but they will not run in the default CI because `--config=rbe-ci` inherits `test:rbe --test_tag_filters=-skip-rbe`, and Safari tests are tagged `skip-rbe`.

### Issue Context
- Safari wrapper tests are tagged `skip-rbe`.
- The CI test invocation uses `--config=rbe-ci`.
- The `rbe` config filters out `skip-rbe` tests, so Safari targets won’t execute under this CI configuration.

### Fix Focus Areas
- scripts/github-actions/ci-build.sh[18-24]
- .bazelrc.remote[35-37]
- dotnet/private/dotnet_nunit_test_suite.bzl[83-94]

### Suggested fix
Add a separate CI job (typically macOS runner) that runs the Safari targets without the `rbe` config (or with tag filters that include Safari), so Safari tests actually execute somewhere in CI. Alternatively, if the intent is only to restore the targets for local execution, explicitly document in the PR/CI docs that Safari targets are excluded from `rbe-ci` runs.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@nvborisenko nvborisenko merged commit 95014e1 into SeleniumHQ:trunk Mar 30, 2026
21 of 22 checks passed
@nvborisenko nvborisenko deleted the dotnet-test-safari branch March 30, 2026 22:18
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants