Skip to content

[java] publish selenium-devtools-latest artifact#17562

Open
titusfortner wants to merge 3 commits into
trunkfrom
devtools_latest
Open

[java] publish selenium-devtools-latest artifact#17562
titusfortner wants to merge 3 commits into
trunkfrom
devtools_latest

Conversation

@titusfortner
Copy link
Copy Markdown
Member

@titusfortner titusfortner commented May 24, 2026

🔗 Related Issues

Alternative implementation to #17492

💥 What does this PR do?

Creates a jar for devtools-latest so users do not have to keep bumping versions in their code and dependencies to have the most recent version.

🔧 Implementation Notes

  • Let the cdp-client-generator do all the heavy lifting
  • Mostly just need the bazel build file and everything else works
  • Avoids all shims/sed/substitutions
  • No hardcoded constants or lists of implementations
  • Updates will propagate automatically
  • no cross-package visibility issues
  • no spotbugs exemptions
  • No duplicate ServiceLoader registration

At runtime, when a user's classpath has both selenium-devtools-v148 and selenium-devtools-latest, the ServiceLoader.load(CdpInfo.class) call inside CdpVersionFinder finds exactly one entry — v148CdpInfo — from the v148 jar's service file. The latest jar contributes only generated protocol types to the classpath; it's invisible to the version finder. No duplicate registration, no non-determinism, no maintenance.

🤖 AI assistance

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

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@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 May 24, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add selenium-devtools-latest stable alias artifact

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Introduces selenium-devtools-latest artifact as stable alias
• Automatically computes latest CDP version from versions list
• Users can import from org.openqa.selenium.devtools.latest package
• Eliminates need to update imports on Chrome version bumps
• Includes comprehensive documentation and build configuration
Diagram
flowchart LR
  A["CDP_VERSIONS list"] -->|"compute max version"| B["LATEST_CDP_VERSION"]
  B -->|"reference in BUILD"| C["latest/BUILD.bazel"]
  C -->|"generate sources"| D["cdp-client-generator"]
  D -->|"create package"| E["org.openqa.selenium.devtools.latest"]
  E -->|"export & publish"| F["selenium-devtools-latest artifact"]
  B -->|"include in deps"| G["versions.bzl CDP_DEPS"]
  G -->|"add to release"| H["rake_tasks/java.rake"]

Loading

File Changes

1. java/src/org/openqa/selenium/devtools/versions.bzl ✨ Enhancement +5/-1

Compute latest CDP version automatically

• Added LATEST_CDP_VERSION variable that automatically computes the maximum version from
 CDP_VERSIONS
• Updated CDP_DEPS to include the new latest devtools module

java/src/org/openqa/selenium/devtools/versions.bzl


2. rake_tasks/java.rake ⚙️ Configuration changes +1/-0

Add latest devtools to release targets

• Added //java/src/org/openqa/selenium/devtools/latest:latest.publish to JAVA_RELEASE_TARGETS
• Enables publishing of the latest devtools artifact during release process

rake_tasks/java.rake


3. java/src/org/openqa/selenium/devtools/README.md 📝 Documentation +15/-1

Document latest devtools alias artifact

• Clarified that LATEST_CDP_VERSION is computed automatically and doesn't require manual editing
• Added comprehensive documentation explaining the selenium-devtools-latest artifact purpose and
 usage
• Documented how users can import from org.openqa.selenium.devtools.latest package
• Explained that the alias is rebuilt automatically when CDP versions change

java/src/org/openqa/selenium/devtools/README.md


View more (1)
4. java/src/org/openqa/selenium/devtools/latest/BUILD.bazel ✨ Enhancement +70/-0

Create latest devtools module build configuration

• Created new Bazel build file for the latest devtools module
• Defined java_export target that publishes selenium-devtools-latest Maven artifact
• Configured genrule to generate CDP protocol sources using cdp-client-generator with latest
 package name
• Set up copy_file rules to fetch browser and JavaScript protocol definitions from latest CDP
 version
• Exports and depends on the dynamically resolved latest versioned devtools module

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2)

Grey Divider


Remediation recommended

1. selenium-devtools-latest missing tests 📘 Rule violation ☼ Reliability
Description
This PR introduces a new user-facing artifact (selenium-devtools-latest) but does not add any
tests to verify the alias package compiles/links as advertised. Without a small unit/compile test,
regressions in generation/packaging may only be caught late (or by users) when CDP versions bump.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[R7-29]

Evidence
PR Compliance ID 5 requires behavior-changing features to include tests when feasible. The PR adds a
new published selenium-devtools-latest artifact and documents it as a stable alias for users, but
no corresponding test changes are included to validate the new artifact/package contract.

AGENTS.md: Add or update tests for fixes/features, favoring small unit tests and avoiding mocks
java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[7-29]
java/src/org/openqa/selenium/devtools/README.md[18-29]

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

## Issue description
A new published artifact (`selenium-devtools-latest`) is added, but there is no test that locks in the expected contract (that `org.openqa.selenium.devtools.latest.*` generated types exist and can be referenced).

## Issue Context
The README documents that users can import `org.openqa.selenium.devtools.latest.page.Page`. Adding a small unit/compile test that depends on `//java/src/org/openqa/selenium/devtools/latest` and references a well-known generated type will detect broken generation/packaging during CI.

## Fix Focus Areas
- java/test/org/openqa/selenium/devtools/BUILD.bazel[4-20]
- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[31-45]

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


2. Unquoted $@ in genrule 📘 Rule violation ⛨ Security
Description
The new genrule cmd uses unquoted shell expansions (e.g., $@ and $(location ...)), which can
cause word-splitting/globbing issues and makes the build step less robust. This violates the
requirement to harden build/scripts with safe argument handling.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54]

Evidence
PR Compliance ID 14 requires safe argument handling (quoting paths/args) in scripts/build steps. The
genrule command at the cited location passes multiple unquoted $(location ...) expansions and an
unquoted $@ output argument.

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54-54]
Best Practice: Learned patterns

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 genrule `cmd` is assembled without quoting `$(location ...)` expansions and the output arg (`$@`). In shell contexts this can lead to word-splitting/globbing and reduces robustness.

## Issue Context
Compliance requires safe argument handling in build/CI/scripts (quoting paths/args, using safer command forms).

## Fix Focus Areas
- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54-54]

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


3. Unguarded latest version compute 🐞 Bug ☼ Reliability
Description
LATEST_CDP_VERSION is computed via max(int(...)) over CDP_VERSIONS; if the list is ever empty
or contains a non-v### entry, loading versions.bzl will fail and break any Bazel target that
loads it.
Code

java/src/org/openqa/selenium/devtools/versions.bzl[7]

Evidence
versions.bzl computes LATEST_CDP_VERSION directly from CDP_VERSIONS at load time, and the new
devtools/latest BUILD loads that symbol, so an evaluation error here breaks the latest artifact
(and any other consumer of versions.bzl).

java/src/org/openqa/selenium/devtools/versions.bzl[1-11]
java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[1-29]

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

### Issue description
`LATEST_CDP_VERSION` is derived from `CDP_VERSIONS` using `max([int(v[1:]) ...])`, which hard-fails at load time if `CDP_VERSIONS` is empty or an entry is not numeric after the leading `v`.

### Issue Context
This is evaluated when `versions.bzl` is loaded, so the failure prevents Bazel from analyzing/building any target that loads this file (including the new `devtools/latest` target).

### Fix Focus Areas
- Add a defensive check + clear failure message (or fallback) before computing `LATEST_CDP_VERSION`.
- Optionally validate each entry is `v` + digits.

- java/src/org/openqa/selenium/devtools/versions.bzl[1-11]

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



Advisory comments

4. Redundant exports/runtime_deps 🐞 Bug ⚙ Maintainability
Description
latest/BUILD.bazel lists the same two targets in both exports and runtime_deps, which is
redundant and makes the dependency intent harder to reason about.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[R21-28]

Evidence
The new latest target duplicates entries in exports and runtime_deps. The java_export macro
and module rule already process both attributes, so duplicating the same labels provides no added
behavior and increases configuration noise.

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[21-28]
java/private/export.bzl[58-67]
java/private/module.bzl[98-107]

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 `latest` `java_export` repeats identical labels in both `exports` and `runtime_deps`. Given `java_export` already passes `exports` to `java_module`, repeating them in `runtime_deps` is unnecessary and obscures what is meant to be exported vs only needed at runtime.

### Issue Context
In `java_export`, `java_module` receives `deps = deps + runtime_deps` and also receives `exports` separately; and module construction accounts for both `deps` and `exports` when building the module jar/module path.

### Fix Focus Areas
- Keep each dependency in exactly one place based on intent (exported vs runtime-only).

- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[21-28]
- java/private/export.bzl[58-67]
- java/private/module.bzl[98-107]

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


Grey Divider

Previous review results

Review updated until commit 884776a

Results up to commit 4fb200b


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


Remediation recommended
1. Unquoted $@ in genrule 📘 Rule violation ⛨ Security
Description
The new genrule cmd uses unquoted shell expansions (e.g., $@ and $(location ...)), which can
cause word-splitting/globbing issues and makes the build step less robust. This violates the
requirement to harden build/scripts with safe argument handling.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54]

Evidence
PR Compliance ID 14 requires safe argument handling (quoting paths/args) in scripts/build steps. The
genrule command at the cited location passes multiple unquoted $(location ...) expansions and an
unquoted $@ output argument.

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54-54]
Best Practice: Learned patterns

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 genrule `cmd` is assembled without quoting `$(location ...)` expansions and the output arg (`$@`). In shell contexts this can lead to word-splitting/globbing and reduces robustness.

## Issue Context
Compliance requires safe argument handling in build/CI/scripts (quoting paths/args, using safer command forms).

## Fix Focus Areas
- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[54-54]

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


2. Unguarded latest version compute 🐞 Bug ☼ Reliability
Description
LATEST_CDP_VERSION is computed via max(int(...)) over CDP_VERSIONS; if the list is ever empty
or contains a non-v### entry, loading versions.bzl will fail and break any Bazel target that
loads it.
Code

java/src/org/openqa/selenium/devtools/versions.bzl[7]

Evidence
versions.bzl computes LATEST_CDP_VERSION directly from CDP_VERSIONS at load time, and the new
devtools/latest BUILD loads that symbol, so an evaluation error here breaks the latest artifact
(and any other consumer of versions.bzl).

java/src/org/openqa/selenium/devtools/versions.bzl[1-11]
java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[1-29]

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

### Issue description
`LATEST_CDP_VERSION` is derived from `CDP_VERSIONS` using `max([int(v[1:]) ...])`, which hard-fails at load time if `CDP_VERSIONS` is empty or an entry is not numeric after the leading `v`.

### Issue Context
This is evaluated when `versions.bzl` is loaded, so the failure prevents Bazel from analyzing/building any target that loads this file (including the new `devtools/latest` target).

### Fix Focus Areas
- Add a defensive check + clear failure message (or fallback) before computing `LATEST_CDP_VERSION`.
- Optionally validate each entry is `v` + digits.

- java/src/org/openqa/selenium/devtools/versions.bzl[1-11]

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



Advisory comments
3. Redundant exports/runtime_deps 🐞 Bug ⚙ Maintainability
Description
latest/BUILD.bazel lists the same two targets in both exports and runtime_deps, which is
redundant and makes the dependency intent harder to reason about.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[R21-28]

Evidence
The new latest target duplicates entries in exports and runtime_deps. The java_export macro
and module rule already process both attributes, so duplicating the same labels provides no added
behavior and increases configuration noise.

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[21-28]
java/private/export.bzl[58-67]
java/private/module.bzl[98-107]

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 `latest` `java_export` repeats identical labels in both `exports` and `runtime_deps`. Given `java_export` already passes `exports` to `java_module`, repeating them in `runtime_deps` is unnecessary and obscures what is meant to be exported vs only needed at runtime.

### Issue Context
In `java_export`, `java_module` receives `deps = deps + runtime_deps` and also receives `exports` separately; and module construction accounts for both `deps` and `exports` when building the module jar/module path.

### Fix Focus Areas
- Keep each dependency in exactly one place based on intent (exported vs runtime-only).

- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[21-28]
- java/private/export.bzl[58-67]
- java/private/module.bzl[98-107]

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


Results up to commit 9dbf21d


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


Remediation recommended
1. selenium-devtools-latest missing tests 📘 Rule violation ☼ Reliability
Description
This PR introduces a new user-facing artifact (selenium-devtools-latest) but does not add any
tests to verify the alias package compiles/links as advertised. Without a small unit/compile test,
regressions in generation/packaging may only be caught late (or by users) when CDP versions bump.
Code

java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[R7-29]

Evidence
PR Compliance ID 5 requires behavior-changing features to include tests when feasible. The PR adds a
new published selenium-devtools-latest artifact and documents it as a stable alias for users, but
no corresponding test changes are included to validate the new artifact/package contract.

AGENTS.md: Add or update tests for fixes/features, favoring small unit tests and avoiding mocks
java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[7-29]
java/src/org/openqa/selenium/devtools/README.md[18-29]

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

## Issue description
A new published artifact (`selenium-devtools-latest`) is added, but there is no test that locks in the expected contract (that `org.openqa.selenium.devtools.latest.*` generated types exist and can be referenced).

## Issue Context
The README documents that users can import `org.openqa.selenium.devtools.latest.page.Page`. Adding a small unit/compile test that depends on `//java/src/org/openqa/selenium/devtools/latest` and references a well-known generated type will detect broken generation/packaging during CI.

## Fix Focus Areas
- java/test/org/openqa/selenium/devtools/BUILD.bazel[4-20]
- java/src/org/openqa/selenium/devtools/latest/BUILD.bazel[31-45]

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


Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 9dbf21d

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit c26b1fb

@asolntsev asolntsev added this to the 4.45.0 milestone May 24, 2026
Copy link
Copy Markdown
Contributor

@asolntsev asolntsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to build it locally from branch and run my existing tests - seems it's fully working!

@titusfortner titusfortner requested a review from diemol May 24, 2026 11:15
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 9609c4b

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 884776a

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.

3 participants