Skip to content

[adr] static install method downloads the driver and browser#17704

Open
titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:adr-install
Open

[adr] static install method downloads the driver and browser#17704
titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:adr-install

Conversation

@titusfortner

Copy link
Copy Markdown
Member

📄 The decision, its rationale, considered options, and consequences are in the record this PR adds; read it there. The sections below are proposal notes and review logistics.

🔗 Related

📝 Proposal notes

One record bundles three tightly-related sub-choices that share a single rationale: a static install on the driver class (browser + driver), the same on the service class (driver), and an optional options instance on the driver-class method.

Deliberately out of scope, as separate and higher-risk decisions: changing how Selenium Manager resolves binaries, and whether an explicit driver path should suppress SM browser management.

🗣 Discussion

  • Should the service-class method also accept an options instance, or stay driver-only as drafted?
  • Is install the right name across all ecosystems, or is setup/download preferable in some?

📌 Tracking

Tracking issue: (linked on acceptance)

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add ADR proposing static install to pre-download driver and browser
📝 Documentation 🕐 10-20 Minutes

Grey Divider

Description

• Add an ADR proposing a public static install API to warm Selenium Manager cache.
• Document optional options parameter and separate driver vs service install responsibilities.
• Capture alternatives, rationale, and consequences without changing Selenium Manager behavior.
Diagram

graph TD
  A(["CI / Test suite"]) --> B["Driver.install (browser+driver)"] --> D["Selenium Manager"] --> E[("SM cache")]
  A --> C["Service.install (driver)"] --> D
  subgraph Legend
    direction LR
    _a(["Caller"]) ~~~ _m["API entrypoint"] ~~~ _c[("Cache")]
  end
Loading
High-Level Assessment

The ADR’s proposed approach is appropriate: a simple, public static entrypoint avoids relying on internal DriverFinder APIs and avoids path pinning that can inadvertently disable Selenium Manager browser management. The record already considers and rejects the main alternatives (no public method, instance method, defaults-only, or only-driver/only-service entrypoint).

Files changed (1) +63 / -0

Documentation (1) +63 / -0
install-driver-and-browser.mdAdd ADR proposing static 'install' for pre-downloading binaries +63/-0

Add ADR proposing static 'install' for pre-downloading binaries

• Introduces a new Architecture Decision Record documenting a proposed static 'install' API on driver and service classes to trigger Selenium Manager downloads ahead of session creation. Captures context, decision details (including optional options parameter), considered alternatives, and consequences.

docs/decisions/install-driver-and-browser.md

@qodo-code-review

qodo-code-review Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 11 rules

Grey Divider


Remediation recommended

1. No binding-status table 🐞 Bug ⚙ Maintainability
Description
The decision record lacks the required "Binding status" section/table, so there is no standard place
to track per-binding implementation convergence as defined by the decisions process.
Code

docs/decisions/17704-install-driver-and-browser.md[R50-63]

+## Considered options
+
+- **No public method (Rejected).** Leaves callers reaching into internal APIs.
+- **An instance method (Rejected).** Warming happens before a session exists; a static is
+  where users start one.
+- **Defaults only, no options instance (Rejected).** Environment variables can adjust
+  download behavior but can't select a specific browser version or binary.
+- **A single method on only the driver or only the service class (Rejected).** Each is the
+  natural entry point for the binaries it owns; offering only one makes a caller holding the
+  other construct it first.
+
+## Consequences
+
+- Provisioning binaries ahead of a session becomes a supported, one-call operation.
Evidence
The ADR template includes a dedicated Binding status section/table, and the decisions README states
that per-binding convergence is tracked in that table; the new record does not include it.

docs/decisions/17704-install-driver-and-browser.md[50-63]
docs/decisions/0000-template.md[39-51]
docs/decisions/README.md[43-44]

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 ADR ends after "Consequences" and does not include the standard `## Binding status` section/table, which the decisions process uses to track rollout across Java/Python/Ruby/.NET/JavaScript.

## Issue Context
The decisions template includes a Binding status table, and `docs/decisions/README.md` states each binding tracks convergence in that table.

## Fix Focus Areas
- docs/decisions/17704-install-driver-and-browser.md[50-63]
- docs/decisions/0000-template.md[39-51]
- docs/decisions/README.md[43-44]

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


2. Missing ADR date 🐞 Bug ⚙ Maintainability
Description
The decision record omits the required - Date: YYYY-MM-DD line, so the status-change timestamp is
not recorded and the document does not conform to the decisions template.
Code

docs/decisions/17704-install-driver-and-browser.md[R1-5]

+# 17704. A static `install` method downloads the driver and browser
+
+- Status: Proposed
+- Discussion: https://github.com/SeleniumHQ/selenium/pull/17704
+
Evidence
The new ADR header includes Status and Discussion but no Date line, while the repository’s decisions
template explicitly requires the Date field under Status.

docs/decisions/17704-install-driver-and-browser.md[1-5]
docs/decisions/0000-template.md[6-9]

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 new ADR is missing the required `- Date: YYYY-MM-DD` metadata line (date the status last changed), which is part of the standard decisions template.

## Issue Context
The decisions template in `docs/decisions/0000-template.md` specifies `Status`, `Date`, and `Discussion` header metadata.

## Fix Focus Areas
- docs/decisions/17704-install-driver-and-browser.md[1-5]
- docs/decisions/0000-template.md[6-9]

ⓘ 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 1169db0

Results up to commit 29f0dc0


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


Remediation recommended
1. Missing ADR date 🐞 Bug ⚙ Maintainability
Description
The decision record omits the required - Date: YYYY-MM-DD line, so the status-change timestamp is
not recorded and the document does not conform to the decisions template.
Code

docs/decisions/17704-install-driver-and-browser.md[R1-5]

+# 17704. A static `install` method downloads the driver and browser
+
+- Status: Proposed
+- Discussion: https://github.com/SeleniumHQ/selenium/pull/17704
+
Evidence
The new ADR header includes Status and Discussion but no Date line, while the repository’s decisions
template explicitly requires the Date field under Status.

docs/decisions/17704-install-driver-and-browser.md[1-5]
docs/decisions/0000-template.md[6-9]

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 new ADR is missing the required `- Date: YYYY-MM-DD` metadata line (date the status last changed), which is part of the standard decisions template.

## Issue Context
The decisions template in `docs/decisions/0000-template.md` specifies `Status`, `Date`, and `Discussion` header metadata.

## Fix Focus Areas
- docs/decisions/17704-install-driver-and-browser.md[1-5]
- docs/decisions/0000-template.md[6-9]

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


2. No binding-status table 🐞 Bug ⚙ Maintainability
Description
The decision record lacks the required "Binding status" section/table, so there is no standard place
to track per-binding implementation convergence as defined by the decisions process.
Code

docs/decisions/17704-install-driver-and-browser.md[R50-63]

+## Considered options
+
+- **No public method (Rejected).** Leaves callers reaching into internal APIs.
+- **An instance method (Rejected).** Warming happens before a session exists; a static is
+  where users start one.
+- **Defaults only, no options instance (Rejected).** Environment variables can adjust
+  download behavior but can't select a specific browser version or binary.
+- **A single method on only the driver or only the service class (Rejected).** Each is the
+  natural entry point for the binaries it owns; offering only one makes a caller holding the
+  other construct it first.
+
+## Consequences
+
+- Provisioning binaries ahead of a session becomes a supported, one-call operation.
Evidence
The ADR template includes a dedicated Binding status section/table, and the decisions README states
that per-binding convergence is tracked in that table; the new record does not include it.

docs/decisions/17704-install-driver-and-browser.md[50-63]
docs/decisions/0000-template.md[39-51]
docs/decisions/README.md[43-44]

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 ADR ends after "Consequences" and does not include the standard `## Binding status` section/table, which the decisions process uses to track rollout across Java/Python/Ruby/.NET/JavaScript.

## Issue Context
The decisions template includes a Binding status table, and `docs/decisions/README.md` states each binding tracks convergence in that table.

## Fix Focus Areas
- docs/decisions/17704-install-driver-and-browser.md[50-63]
- docs/decisions/0000-template.md[39-51]
- docs/decisions/README.md[43-44]

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


Qodo Logo

@titusfortner titusfortner changed the title [docs] ADR: a static install method downloads the driver and browser [adr] static install method downloads the driver and browser Jun 22, 2026
@titusfortner titusfortner added the A-needs decision TLC needs to discuss and agree label Jun 22, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 1169db0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-needs decision TLC needs to discuss and agree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant