Skip to content

[build] remove obsolete third_party dependencies and dead build references - #18072

Merged
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:remove-obsolete-third-party
Oct 1, 2026
Merged

titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:remove-obsolete-third-party

Conversation

@titusfortner

@titusfortner titusfortner commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

Cleans up unused items in third_party/

🔧 Implementation Notes

  • All bindings now use the extensions in common/extensions, so the ones in third_party go (firebug/, backspace.crx), along with Sizzle and the Selenium 3 webdriver-3.141.59.xpi
  • 2 bazel patch files were created but never used or had references removed (aspect_rules_jest_runfiles and hermetic_llvm_windows_exec_toolchains), and the third_party.iml IntelliJ module pointed at a third_party/src that does not exist
  • Remove references to all these from the bazel and IntelliJ files that still named them
  • Remove unused webdriver.json
  • Sizzle was the environment appserver's only third_party/js runfile, and CommonWebResources locates that directory unconditionally, so the target now depends on //third_party/js/qunit — which is what the atoms tests actually serve from there

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5)
    • What was generated: the reachability analysis, the deletions, and this description
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Sep 23, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Remove obsolete third-party assets and build references

⚙️ Configuration changes ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Removes obsolete browser extensions, Selenium 3 artifacts, Sizzle, and unused Bazel patches.
• Drops stale Bazel dependencies and Java resource-staging references to deleted assets.
• Removes IntelliJ module entries targeting nonexistent or obsolete third-party sources.
Diagram

graph TD
  Tests["Java Test Targets"] --> Extensions["Common Extensions"]
  Resources["Static Resources"] --> Atoms["JavaScript Atoms"]
  IDE["IDE Metadata"] --> Sources["Active Sources"]
  Tests -. "dependency removed" .-> Removed["Obsolete Assets"]
  Resources -. "staging removed" .-> Removed
  IDE -. "module removed" .-> Removed
Loading
High-Level Assessment

Direct deletion is the appropriate approach because the assets and patches are unreachable, current bindings already use common/extensions, and the remaining changes remove their stale consumers. Retaining or quarantining these files would preserve maintenance, licensing, and repository-size costs without providing a supported fallback.

Files changed (7) +0 / -24

Refactor (1) +0 / -10
StaticResources.javaStop staging legacy Firefox WebDriver resources +0/-10

Stop staging legacy Firefox WebDriver resources

• Removes copying and Bazel-building of Selenium 3 Firefox preferences and the legacy WebDriver XPI. Static resource setup now stages only currently supported generated resources.

java/test/org/openqa/selenium/testing/StaticResources.java

Other (6) +0 / -14
modules.xmlRemove the obsolete third-party IntelliJ module +0/-1

Remove the obsolete third-party IntelliJ module

• Removes the project-level reference to third_party/third_party.iml, which declared a nonexistent source root and is deleted by this PR.

.idea/modules.xml

java-dev.imlStop indexing deleted Sizzle resources +0/-3

Stop indexing deleted Sizzle resources

• Removes the Sizzle directory from the Java development module's test-resource roots now that the vendored library is gone.

java/java-dev.iml

BUILD.bazelRemove the legacy extension from BiDi input tests +0/-3

Remove the legacy extension from BiDi input tests

• Drops the backspace.crx runtime data dependency from the BiDi input test suite because tests no longer consume the third-party extension.

java/test/org/openqa/selenium/bidi/input/BUILD.bazel

BUILD.bazelRemove the legacy extension from BiDi permission tests +0/-3

Remove the legacy extension from BiDi permission tests

• Drops the backspace.crx runtime data dependency from the BiDi permissions test suite because tests no longer consume the third-party extension.

java/test/org/openqa/selenium/bidi/permissions/BUILD.bazel

BUILD.bazelRemove the environment library's Sizzle dependency +0/-1

Remove the environment library's Sizzle dependency

• Eliminates the dependency on the deleted vendored Sizzle target while retaining the active JavaScript atoms and Closure resources.

java/test/org/openqa/selenium/environment/BUILD.bazel

BUILD.bazelRemove the legacy extension from interaction tests +0/-3

Remove the legacy extension from interaction tests

• Drops the backspace.crx runtime data dependency from the interactions test suite because the extension is obsolete.

java/test/org/openqa/selenium/interactions/BUILD.bazel

@qodo-code-review

qodo-code-review Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Vendored files are deleted directly 📘 Rule violation ⚙ Maintainability
Description
third_party/chrome_ext/BUILD.bazel is deleted directly, alongside the extension binary and other
files under third_party/. The cleanup spans vendored binaries, source, licenses, patches, and
build metadata, so the direct modification reaches every protected artifact type included in this
change.
Code

third_party/chrome_ext/BUILD.bazel[L1-3]

-exports_files([
-    "backspace.crx",
-])
Evidence
PR Compliance ID 3 expressly prohibits direct modifications under third_party/. The diff deletes
the complete third_party/chrome_ext/BUILD.bazel file and similarly removes numerous other files
from that directory.

AGENTS.md: Do Not Modify Third-Party or Generated Output Directories

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 PR directly deletes files under the protected `third_party/` directory, contrary to the repository compliance requirement.

## Fix Focus Areas
- third_party/chrome_ext/BUILD.bazel[1-3]

## Recommended Fix
Revert direct modifications under `third_party/` and perform the cleanup through the repository's approved dependency-management or vendoring workflow, while retaining the removal of obsolete references outside that directory.

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

Dismiss ↗ | View ↗


2. Environment server fails under Bazel ✓ Resolved
Description
The environment target removes its only third_party/js runfile even though CommonWebResources
still unconditionally locates that directory. Running the environment appserver or a test without an
independent QUnit data dependency makes InProject.locate throw during handler construction before
the server starts.
Code

java/test/org/openqa/selenium/environment/BUILD.bazel[36]

-        "//third_party/js/sizzle",
Evidence
The environment target's data now contains no resource below third_party/js, while
CommonWebResources unconditionally resolves that path. InProject.locate throws when the path is
absent, and NettyAppServer constructs HandlersForTests, which installs CommonWebResources,
before starting; the repository's debug-server alias directly exposes this affected appserver.

java/test/org/openqa/selenium/environment/BUILD.bazel[26-36]
java/test/org/openqa/selenium/environment/webserver/CommonWebResources.java[38-44]
java/test/org/openqa/selenium/build/InProject.java[39-63]
java/test/org/openqa/selenium/environment/webserver/HandlersForTests.java[78-84]
java/test/org/openqa/selenium/environment/webserver/NettyAppServer.java[108-114]
BUILD.bazel[92-95]

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

## Issue description
Removing the Sizzle data target leaves `third_party/js` absent from the environment server's Bazel runfiles, but `CommonWebResources` still requires that directory during construction.

## Fix Focus Areas
- java/test/org/openqa/selenium/environment/BUILD.bazel[26-36]
- java/test/org/openqa/selenium/environment/webserver/CommonWebResources.java[38-44]

## Recommended Fix
Add `//third_party/js/qunit` to the environment target's `data` so the remaining JavaScript resource directory is present in every environment-server runfiles tree. Alternatively, make the JavaScript resource layer optional and avoid calling `locate("third_party/js")` when no such runfile exists.

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is primarily dependency/build cleanup, but it changes Bazel runtime resource dependencies and removes test/build artifacts across multiple independent paths, warranting a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit bc410eb

Results up to commit 7c61112 ⚖️ Balanced


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


Action required
1. Vendored files are deleted directly 📘 Rule violation ⚙ Maintainability
Description
third_party/chrome_ext/BUILD.bazel is deleted directly, alongside the extension binary and other
files under third_party/. The cleanup spans vendored binaries, source, licenses, patches, and
build metadata, so the direct modification reaches every protected artifact type included in this
change.
Code

third_party/chrome_ext/BUILD.bazel[L1-3]

-exports_files([
-    "backspace.crx",
-])
Evidence
PR Compliance ID 3 expressly prohibits direct modifications under third_party/. The diff deletes
the complete third_party/chrome_ext/BUILD.bazel file and similarly removes numerous other files
from that directory.

AGENTS.md: Do Not Modify Third-Party or Generated Output Directories

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 PR directly deletes files under the protected `third_party/` directory, contrary to the repository compliance requirement.

## Fix Focus Areas
- third_party/chrome_ext/BUILD.bazel[1-3]

## Recommended Fix
Revert direct modifications under `third_party/` and perform the cleanup through the repository's approved dependency-management or vendoring workflow, while retaining the removal of obsolete references outside that directory.

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

Dismiss ↗ | View ↗


2. Environment server fails under Bazel ✓ Resolved
Description
The environment target removes its only third_party/js runfile even though CommonWebResources
still unconditionally locates that directory. Running the environment appserver or a test without an
independent QUnit data dependency makes InProject.locate throw during handler construction before
the server starts.
Code

java/test/org/openqa/selenium/environment/BUILD.bazel[36]

-        "//third_party/js/sizzle",
Evidence
The environment target's data now contains no resource below third_party/js, while
CommonWebResources unconditionally resolves that path. InProject.locate throws when the path is
absent, and NettyAppServer constructs HandlersForTests, which installs CommonWebResources,
before starting; the repository's debug-server alias directly exposes this affected appserver.

java/test/org/openqa/selenium/environment/BUILD.bazel[26-36]
java/test/org/openqa/selenium/environment/webserver/CommonWebResources.java[38-44]
java/test/org/openqa/selenium/build/InProject.java[39-63]
java/test/org/openqa/selenium/environment/webserver/HandlersForTests.java[78-84]
java/test/org/openqa/selenium/environment/webserver/NettyAppServer.java[108-114]
BUILD.bazel[92-95]

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

## Issue description
Removing the Sizzle data target leaves `third_party/js` absent from the environment server's Bazel runfiles, but `CommonWebResources` still requires that directory during construction.

## Fix Focus Areas
- java/test/org/openqa/selenium/environment/BUILD.bazel[26-36]
- java/test/org/openqa/selenium/environment/webserver/CommonWebResources.java[38-44]

## Recommended Fix
Add `//third_party/js/qunit` to the environment target's `data` so the remaining JavaScript resource directory is present in every environment-server runfiles tree. Alternatively, make the JavaScript resource layer optional and avoid calling `locate("third_party/js")` when no such runfile exists.

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


Grey Divider

Qodo Logo

Comment thread third_party/chrome_ext/BUILD.bazel
Comment thread java/test/org/openqa/selenium/environment/BUILD.bazel
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner force-pushed the remove-obsolete-third-party branch from c2b5e53 to bc410eb Compare October 1, 2026 16:32
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner merged commit 8e92f06 into SeleniumHQ:trunk Oct 1, 2026
26 checks passed
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 B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants