Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions docs/decisions/17741-selenium-manager-released-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# 17741. Selenium Manager ships as an official, independently released tool

- Status: Proposed
- Discussion: https://github.com/SeleniumHQ/selenium/pull/17741

Comment on lines +1 to +5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Informational

1. Missing decision date 🐞 Bug βš™ Maintainability

The decision record omits the required - Date: YYYY-MM-DD metadata line, so the record doesn’t
state when its status last changed and doesn’t conform to the decision template used in this repo.
Agent Prompt
### Issue description
The new decision record is missing the required `Date` metadata line (date the status last changed).

### Issue Context
The decision template includes `Status`, `Date`, and `Discussion` fields at the top of the file.

### Fix Focus Areas
- docs/decisions/17741-selenium-manager-released-api.md[1-5]

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

## Context

Releasing Selenium Manager (SM) independently of the bindings means a binding and the SM it
invokes can differ in version. This record settles what that commits us to. A decision belongs
here only if it either cannot change after 1.0 without a breaking change β€” the forward-compatibility
behavior and every committed default β€” or is an obligation the independent release itself creates.
Bugs and additive enhancements (new browsers, warning wording) are out of scope.

## Decision

**Versioning and distribution.** SM takes its own semantic version from `1.0.0` and is released
standalone as well as bundled. Bindings ship and default to a known-good SM, but already let a user
point at a different binary, so a binding and the SM it runs may differ in version.

**Output.** The default follows standard CLI convention β€” result on stdout, logs on stderr β€” so
`$(selenium-manager …)` captures only the result. Bindings request output explicitly and are
unaffected, so only the standalone audience is impacted.
Comment on lines +21 to +22

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remediation recommended

1. Output default contradicts cli 🐞 Bug ≑ Correctness

The ADR claims Selenium Manager’s default output puts the result on stdout and logs/diagnostics on
stderr, but the current CLI default is --output LOGGER, which logs to stdout; only `--output
mixed` targets stderr. This makes the proposed stable contract for standalone usage inaccurate
unless the ADR also commits to changing the CLI default (or explicitly names mixed as the default
output mode).
Agent Prompt
## Issue description
The ADR states that Selenium Manager’s *default* output follows the β€œresult on stdout, logs on stderr” convention, but the current Rust CLI default output mode is `LOGGER`, which sends logs to stdout. This makes the ADR’s contract misleading for standalone users and for shell command-substitution examples.

## Issue Context
- The ADR currently describes the stdout/stderr split as a default behavior and reiterates that choice in the β€œConsidered options” section.
- In `rust/`, the CLI default output is `LOGGER`, and the stdout/stderr split behavior corresponds to `--output mixed`.

## Fix Focus Areas
- docs/decisions/17741-selenium-manager-released-api.md[20-22]
- docs/decisions/17741-selenium-manager-released-api.md[59-61]

## What to change
Update the ADR to match one of these consistent positions:
1) **Document current behavior**: state that the CLI default is `LOGGER` (logs on stdout), and that bindings explicitly request structured output.
2) **Document intended contract change**: explicitly name the output mode (e.g., `mixed`) as the default from SM 1.0 onward and/or note that the Rust CLI default will be changed to implement this contract.

Also update the β€œConsidered options” bullet to use the same terminology (e.g., mention `LOGGER` vs `mixed`) so readers can map the decision to the actual CLI flags.

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


**Forward compatibility.** Neither the bindings nor the binary may fail solely because of version
skew. How an older SM treats an input it does not recognize depends on the input:

- *Unknown switch* β†’ warn (tell the user to update SM) and ignore; the binding still gets valid
paths. A switch only adjusts resolution, so dropping it is safe.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
- *Unknown value of a switch that has a default* (output format, log level) β†’ use the default and warn.
- *Unknown value of a switch with no default* β†’ error, naming the value and telling the user to
update SM (e.g. `Selenium Manager 1.2 does not support browser 'chrome-for-testing'`). There is
nothing to fall back to; for `--browser` these are distinct browsers where substituting one would
be the wrong result. Erroring is safe because such values are only ever requested explicitly.
- New switches must be optional β€” an older binding will omit them.
- Nothing in the contract β€” switches, values, output fields β€” is removed or renamed without a
deprecation cycle.
- Current defaults must not change once released, and must all be documented
([configuration reference](https://www.selenium.dev/documentation/selenium_manager/#configuration)).
Comment on lines +37 to +38

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remediation recommended

1. Defaults not enumerated 🐞 Bug βš™ Maintainability

The ADR says β€œcurrent defaults” are locked as part of the stable contract, but it doesn’t actually
list which defaults are being locked and instead points to an external reference. This conflicts
with the repo’s rule that decision records must be understandable without following links and leaves
the contract ambiguous when defaults change.
Agent Prompt
### Issue description
The ADR locks β€œcurrent defaults” but doesn’t enumerate them, requiring readers to follow an external link to know what is actually committed.

### Issue Context
`docs/decisions/README.md` states decision records must stand alone without requiring links. The ADR currently references an external configuration page instead of listing the defaults it intends to lock.

### Fix Focus Areas
- docs/decisions/17741-selenium-manager-released-api.md[8-12]
- docs/decisions/17741-selenium-manager-released-api.md[37-38]
- docs/decisions/17741-selenium-manager-released-api.md[86-87]
- docs/decisions/README.md[63-64]

### Suggested change
Add an explicit, in-record list of the defaults that are being made part of the immutable contract (and keep the link as β€œbackground/further reading” if desired).

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


**Resolution.**

- When provided a driver location, SM validates the correct browser and downloads it if necessary,
instead of being bypassed. This is not backward compatible, so a toggle is necessary to control
desired behavior.
- A `PATH` driver whose version does not match the browser is not used; SM fetches a matching one.

**Telemetry.**

- The binding must pass in the Selenium version and language (name and version).
- SM must pass in the SM version.
- Each time SM sends data, it logs what was sent and how to disable it.
Comment on lines +16 to +51

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remediation recommended

1. No cross-binding comparison evidence πŸ“˜ Rule violation ≑ Correctness

This ADR defines user-visible contract details impacting bindings (e.g., output behavior and
telemetry requirements) but does not identify any other binding used for comparison or provide
evidence of a cross-binding search. Without an explicit comparison reference, the PR does not meet
the checklist requirement to validate consistency across bindings when specifying user-visible
behavior.
Agent Prompt
## Issue description
The ADR specifies binding-related, user-visible behavior (e.g., `--output` handling and telemetry fields) but does not document any comparison to at least one other language binding or provide evidence of a project-wide search.

## Issue Context
Compliance requires showing which other binding (Java/Python/Ruby/.NET/JS) was checked to ensure the specified behavior is logically consistent across bindings, or documenting any intentional divergence.

## Fix Focus Areas
- docs/decisions/17741-selenium-manager-released-api.md[16-22]
- docs/decisions/17741-selenium-manager-released-api.md[49-51]

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


## Considered options

- **Leaving Beta.**
- Stay in Beta β€” contract stays mutable.
- Drop the label but keep versioning coupled to the bindings β€” empty releases, no standalone use.
- Independently versioned, standalone-released β€” **selected**.
- **Default output.**
- Keep logs on stdout β€” pollutes `$(selenium-manager …)`.
- Follow standard CLI convention: result on stdout, diagnostics on stderr β€” **selected**.
- **Unknown switch, or unknown value of a switch with a default.**
- Error β€” turns expected skew into a broken session.
- Warn and ignore the switch, or use the default value β€” **selected**.
- **Unknown value of a switch with no default** (e.g. `--browser`).
- Warn and substitute a fallback β€” silently does something other than what was asked.
- Error, directing the user to update SM β€” **selected**.
- **A provided driver.**
- Keep bypassing SM β€” the browser is left unmanaged.
- Always run SM β€” breaks backward compatibility.
- Run it behind a toggle, default unchanged β€” **selected**.
- **A mismatched `PATH` driver.**
- Use it with a warning β€” usually still fails at session start.
- Do not use it, fetch a matching one β€” **selected**.

## Consequences

- Once released, the contract constrains every `rust/` change: no switch, value, or output field
changes without a deprecation cycle, and the current defaults are locked.
Comment on lines +76 to +79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

2. Missing binding status table 🐞 Bug βš™ Maintainability

The record lacks a ## Binding status section/table, which the documented process uses to track
per-binding implementation and which is the only section intended to be editable after acceptance.
Agent Prompt
### Issue description
The decision record is missing the `## Binding status` section and tracking table.

### Issue Context
The template includes a binding-status table, and the process doc says bindings track convergence there and that updating that table is the only post-acceptance edit allowed (besides the status line).

### Fix Focus Areas
- docs/decisions/17741-selenium-manager-released-api.md[82-85]
- docs/decisions/0000-template.md[39-51]
- docs/decisions/README.md[28-45]

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