You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
RtD builds the docs directly from a clone of the source repo, and can't use bazel. Since some of the Python modules are generated during the build, they don't exist in the repo and are not included in the docs on RtD.
This PR makes it so when it builds the docs, it will first download the latest nightly sdist tarball from TestPyPI and overlay the files from that onto the source repo. This way, the generated Python files from the nightly build will be included in the docs on RtD.
🤖 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
• Fetch latest nightly selenium sdist from TestPyPI during Read the Docs builds
• Overlay packaged/generated Python modules into the repo checkout before Sphinx runs
• Generate API module listing and build HTML docs with the augmented PYTHONPATH
Diagram
graph TD
A["Read the Docs build"] --> B[".readthedocs.yaml"] --> C["TestPyPI JSON API"] --> D["Nightly sdist (.tar.gz)"] --> E["Overlay into py/selenium"] --> F["Sphinx autogen+build"]
subgraph Legend
direction LR
_svc(["Build step"]) ~~~ _ext{{"External service"}} ~~~ _pkg["Artifact"]
end
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. Build docs from the installed nightly package
➕ Avoids copying files into the git checkout (less risk of stale/partial overlays)
➕ Closer to what end users actually install
➖ Docs generation likely depends on repo-local files (conf.py, api.rst, generation script), requiring extra wiring
➖ Some modules/docs content may not be shipped in the package
2. Commit generated Python modules to the repo (docs-only subset)
➕ RtD build becomes deterministic and offline (no TestPyPI dependency)
➕ Simpler RtD script
➖ Generated sources in VCS add churn/noise and can drift from build output
➖ Requires a clear policy and tooling to keep generated files updated
3. Publish a dedicated docs artifact (sdist/wheel) and have RtD consume it
➕ Deterministic inputs with explicit version pinning
➕ Separates release packaging from docs publishing concerns
➖ More CI/release pipeline complexity
➖ Requires extra hosting/version selection logic
Recommendation: The sdist overlay approach is a pragmatic fit for RtD’s no-bazel constraint and keeps docs sourced from the repo while importing generated modules. To reduce flakiness, consider pinning to a specific nightly version/commit (or adding retry/fallback logic) rather than always selecting the first parsed version/URL from the TestPyPI JSON responses.
Files changed (1) +40 / -6
Other (1) +40 / -6
.readthedocs.yamlDownload nightly sdist from TestPyPI and overlay generated modules before Sphinx+40/-6
Download nightly sdist from TestPyPI and overlay generated modules before Sphinx
• Replaces the simple RtD command list with a bash script that discovers the latest selenium nightly version on TestPyPI, downloads/extracts its sdist, and copies the packaged selenium/ tree into py/selenium/. Then regenerates the API module listing and runs sphinx-autogen and sphinx-build with PYTHONPATH pointing at the updated py/ tree.
The build downloads an sdist from TestPyPI and extracts it with tar -xzf into the repo working
directory, which can overwrite unexpected paths or introduce unsafe entries (e.g., symlinks) from a
compromised artifact. Those overlaid modules are then used as Sphinx autosummary/autodoc inputs, so
the downloaded package contents directly influence what code is imported/processed during doc
generation.
+ # fetch latest nightly sdist tarball from TestPyPI+ curl -fsSLOJ "${URL}"++ # overlay packaged/generated code from the sdist into the repo checkout+ tar -xzf "${SDIST}"+ cp -a "${SDIST%.tar.gz}/selenium/." "${REPO_DIR}/py/${PACKAGE}/"+
Evidence
The build config shows a remote download followed by direct tar extraction and copy into the
import path; the docs configuration uses autosummary/autodoc and api.rst contains `..
autosummary::` entries listing selenium modules, meaning the overlaid package content is actively
consumed by Sphinx during the build.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The docs build extracts a remotely downloaded tarball directly into the checkout directory without validating archive member paths/types.
### Issue Context
The archive is fetched from TestPyPI and extracted with `tar -xzf "${SDIST}"`, then its `selenium/` tree is copied into `py/selenium/`.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[36-45]
### Suggested fix
Mitigate archive/supply-chain hazards by:
1) Extracting into a temporary directory (e.g., `TMP=$(mktemp -d)`), not into the repo root.
2) Validating archive members before writing:
- reject absolute paths and any path containing `..`
- reject symlinks/hardlinks (or at least do not copy them)
3) Copy only the expected subtree (`selenium/`) into `py/selenium/` using a tool/mode that does not preserve unsafe links (e.g., `rsync -a --safe-links --copy-links ...`).
4) Consider pinning/verifying the artifact (e.g., enforce expected hostname for URL, or verify a checksum if you publish one).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The build script extracts VERSION/URL from JSON using sed patterns that only match the
no-whitespace form ("version":"...", "url":"..."), so any whitespace or formatting differences
can yield empty/wrong values and break the docs build. Because sed|head can still exit 0, the
failure can be non-obvious until later steps (download/extract) error out.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The RtD build parses JSON using regex-based `sed`, which is not a reliable JSON parser and can fail on innocuous formatting/whitespace changes.
### Issue Context
This is in the RtD build command block in `py/docs/.readthedocs.yaml`, where VERSION and the sdist URL are derived from TestPyPI JSON endpoints.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[21-30]
### Suggested fix
Replace the `sed` pipelines with Python JSON parsing (Python is already available), and fail fast if values are empty.
Example approach:
- `VERSION=$(python -c 'import json,urllib.request; print(json.load(urllib.request.urlopen(...))["info"]["version"])')`
- `URL=$(python -c '...; files=json.load(...)["urls"]; print(next(u["url"] for u in files if u.get("packagetype")=="sdist"))')`
- Add `test -n "$VERSION"` and `test -n "$URL"` checks.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Missing parsed value checks 🐞 Bug☼ Reliability⭐ New
Description
The build script derives VERSION and URL via sed/head but does not assert they are non-empty before
using them to construct the next TestPyPI request and the SDIST filename. If parsing yields empty
(e.g., response shape changes or no .tar.gz match), the subsequent curl/download fails with an
unclear error and breaks RTD builds.
+ VERSION="$(+ curl -fsSL "https://test.pypi.org/pypi/${PACKAGE}/json" |+ sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' |+ head -n1+ )"+ URL="$(+ curl -fsSL "https://test.pypi.org/pypi/${PACKAGE}/${VERSION}/json" |+ sed -n 's/.*"url"[[:space:]]*:[[:space:]]*"\([^"]*\.tar\.gz\)".*/\1/p' |+ head -n1+ )"+ SDIST="$(basename "${URL}")"
Evidence
VERSION and URL are extracted via sed/head and then used immediately to form the next URL and
compute SDIST, with no guard that parsing returned a value.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`VERSION` and `URL` are scraped from JSON output and then immediately used to form another request and a local filename, without checking that parsing actually produced values. When parsing fails, the script errors later in a confusing way.
### Issue Context
This runs in Read the Docs build logs, where clearer early failures reduce time-to-diagnose.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[21-31]
### Suggested fix
After each assignment, add explicit validation with a clear message, e.g.:
- `: "${VERSION:?Failed to determine selenium version from TestPyPI}"`
- `: "${URL:?Failed to find sdist (.tar.gz) URL for version ${VERSION}}"`
Ensure the `SDIST="$(basename "${URL}")"` line remains after URL validation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4. curl lacks diagnostics/retries 🐞 Bug☼ Reliability⭐ New
Description
The sdist download uses curl -fsSL (includes -s without -S) and has no retry policy, which
suppresses useful error output and makes transient network/HTTP failures harder to diagnose.
Although -f and set -e will fail the build, RTD failures can be flaky and opaque in logs.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The sdist download can fail with minimal diagnostics and no retries due to `curl -fsSL`.
### Issue Context
Read the Docs builds can be sensitive to transient network issues; better curl flags reduce flakiness and improve logs.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[37-37]
### Suggested fix
Change the download command to include error output and retries, e.g.:
```sh
curl -fL -sS --retry 3 --retry-delay 2 --retry-all-errors -o "${SDIST}" "${URL}"
```
(Adjust retry counts/delays as appropriate.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
5. curl output name mismatch✓ Resolved🐞 Bug☼ Reliability
Description
The script derives SDIST from basename(URL) but downloads using curl -OJ, which may save the
file using the server-provided Content-Disposition filename instead of the URL basename. If those
differ, tar -xzf "${SDIST}" will reference a non-existent file and fail the docs build.
+ SDIST="$(basename "${URL}")"++ # install dependencies+ pip install -r "${REPO_DIR}/py/requirements_lock.txt"++ # fetch latest nightly sdist tarball from TestPyPI+ curl -fsSLOJ "${URL}"++ # overlay packaged/generated code from the sdist into the repo checkout+ tar -xzf "${SDIST}"+ cp -a "${SDIST%.tar.gz}/selenium/." "${REPO_DIR}/py/${PACKAGE}/"
Evidence
The script sets SDIST from the URL basename, but uses curl’s content-disposition option -J and
then uses SDIST for extraction, which can diverge if curl writes a different filename.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`SDIST` is computed from the URL, but `curl -OJ` can write the file under a different name, making the subsequent `tar -xzf "${SDIST}"` fragile.
### Issue Context
This occurs in the RtD command script where the sdist is downloaded and then extracted.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[31-41]
### Suggested fix
Download to the exact expected filename:
- Replace `curl -fsSLOJ "${URL}"` with `curl -fsSL "${URL}" -o "${SDIST}"`
- Alternatively, remove `-J` and keep `-O` (or capture the actual output filename and use it consistently).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The build downloads an sdist from TestPyPI and extracts it with tar -xzf into the repo working
directory, which can overwrite unexpected paths or introduce unsafe entries (e.g., symlinks) from a
compromised artifact. Those overlaid modules are then used as Sphinx autosummary/autodoc inputs, so
the downloaded package contents directly influence what code is imported/processed during doc
generation.
+ # fetch latest nightly sdist tarball from TestPyPI+ curl -fsSLOJ "${URL}"++ # overlay packaged/generated code from the sdist into the repo checkout+ tar -xzf "${SDIST}"+ cp -a "${SDIST%.tar.gz}/selenium/." "${REPO_DIR}/py/${PACKAGE}/"+
Evidence
The build config shows a remote download followed by direct tar extraction and copy into the
import path; the docs configuration uses autosummary/autodoc and api.rst contains `..
autosummary::` entries listing selenium modules, meaning the overlaid package content is actively
consumed by Sphinx during the build.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The docs build extracts a remotely downloaded tarball directly into the checkout directory without validating archive member paths/types.
### Issue Context
The archive is fetched from TestPyPI and extracted with `tar -xzf "${SDIST}"`, then its `selenium/` tree is copied into `py/selenium/`.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[36-45]
### Suggested fix
Mitigate archive/supply-chain hazards by:
1) Extracting into a temporary directory (e.g., `TMP=$(mktemp -d)`), not into the repo root.
2) Validating archive members before writing:
- reject absolute paths and any path containing `..`
- reject symlinks/hardlinks (or at least do not copy them)
3) Copy only the expected subtree (`selenium/`) into `py/selenium/` using a tool/mode that does not preserve unsafe links (e.g., `rsync -a --safe-links --copy-links ...`).
4) Consider pinning/verifying the artifact (e.g., enforce expected hostname for URL, or verify a checksum if you publish one).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The build script extracts VERSION/URL from JSON using sed patterns that only match the
no-whitespace form ("version":"...", "url":"..."), so any whitespace or formatting differences
can yield empty/wrong values and break the docs build. Because sed|head can still exit 0, the
failure can be non-obvious until later steps (download/extract) error out.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The RtD build parses JSON using regex-based `sed`, which is not a reliable JSON parser and can fail on innocuous formatting/whitespace changes.
### Issue Context
This is in the RtD build command block in `py/docs/.readthedocs.yaml`, where VERSION and the sdist URL are derived from TestPyPI JSON endpoints.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[21-30]
### Suggested fix
Replace the `sed` pipelines with Python JSON parsing (Python is already available), and fail fast if values are empty.
Example approach:
- `VERSION=$(python -c 'import json,urllib.request; print(json.load(urllib.request.urlopen(...))["info"]["version"])')`
- `URL=$(python -c '...; files=json.load(...)["urls"]; print(next(u["url"] for u in files if u.get("packagetype")=="sdist"))')`
- Add `test -n "$VERSION"` and `test -n "$URL"` checks.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. curl output name mismatch✓ Resolved🐞 Bug☼ Reliability
Description
The script derives SDIST from basename(URL) but downloads using curl -OJ, which may save the
file using the server-provided Content-Disposition filename instead of the URL basename. If those
differ, tar -xzf "${SDIST}" will reference a non-existent file and fail the docs build.
+ SDIST="$(basename "${URL}")"++ # install dependencies+ pip install -r "${REPO_DIR}/py/requirements_lock.txt"++ # fetch latest nightly sdist tarball from TestPyPI+ curl -fsSLOJ "${URL}"++ # overlay packaged/generated code from the sdist into the repo checkout+ tar -xzf "${SDIST}"+ cp -a "${SDIST%.tar.gz}/selenium/." "${REPO_DIR}/py/${PACKAGE}/"
Evidence
The script sets SDIST from the URL basename, but uses curl’s content-disposition option -J and
then uses SDIST for extraction, which can diverge if curl writes a different filename.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`SDIST` is computed from the URL, but `curl -OJ` can write the file under a different name, making the subsequent `tar -xzf "${SDIST}"` fragile.
### Issue Context
This occurs in the RtD command script where the sdist is downloaded and then extracted.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[31-41]
### Suggested fix
Download to the exact expected filename:
- Replace `curl -fsSLOJ "${URL}"` with `curl -fsSL "${URL}" -o "${SDIST}"`
- Alternatively, remove `-J` and keep `-O` (or capture the actual output filename and use it consistently).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The RTD command block runs set -euo pipefail, which will error if the runner executes
build.commands with a POSIX /bin/sh (e.g., dash) that doesn’t support pipefail, breaking the
docs build before any work is done. The repo already treats shell choice as significant when using
this pattern (it pins shell: bash elsewhere), but the RTD config does not pin a shell.
The RTD config directly invokes set -euo pipefail. The repo demonstrates that this pattern is
coupled to bash by explicitly setting shell: bash before using it in workflows.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`py/docs/.readthedocs.yaml` uses `set -euo pipefail` inside the Read the Docs `build.commands` block. If RTD runs these commands using `/bin/sh` instead of bash, `pipefail` is unsupported and the build can fail immediately.
### Issue Context
The repo explicitly pins bash when using `set -euo pipefail` in GitHub workflows, indicating the pattern depends on bash semantics.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[15-18]
### Suggested fix
Adjust the RTD command to explicitly run under bash (e.g., wrap the script in `bash -lc '...script...'`), or remove `pipefail` and avoid pipelines where possible.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. RTD overlay lacks regression test 📘 Rule violation▣ Testability
Description
The new Read the Docs build logic downloads and overlays an sdist before generating docs, but there
is no corresponding CI smoke check to catch breakage before RTD runs. Existing Python docs linting
in CI uses Bazel and will not exercise this RTD-specific path.
+ - |+ set -euo pipefail++ REPO_DIR="$(pwd)"+ PACKAGE="selenium"+ VERSION="$(+ curl -fsSL "https://test.pypi.org/pypi/${PACKAGE}/json" |+ sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' |+ head -n1+ )"+ URL="$(+ curl -fsSL "https://test.pypi.org/pypi/${PACKAGE}/${VERSION}/json" |+ sed -n 's/.*"url"[[:space:]]*:[[:space:]]*"\([^"]*\.tar\.gz\)".*/\1/p' |+ head -n1+ )"+ SDIST="$(basename "${URL}")"++ # install dependencies+ pip install -r "${REPO_DIR}/py/requirements_lock.txt"++ # fetch latest nightly sdist tarball from TestPyPI+ curl -fsSL -o "${SDIST}" "${URL}"++ # overlay packaged/generated code from the sdist into the repo checkout+ tar -xzf "${SDIST}"+ cp -a "${SDIST%.tar.gz}/selenium/." "${REPO_DIR}/py/${PACKAGE}/"++ # remove extracted sdist contents+ rm -rf "${SDIST%.tar.gz}" "${SDIST}"++ # generate new .rst with API modules+ cd "${REPO_DIR}/py" && python3 "generate_api_module_listing.py" && cd ..++ export PYTHONPATH="${REPO_DIR}/py:${PYTHONPATH:-}"++ # generate doc stubs+ sphinx-autogen -o "${READTHEDOCS_OUTPUT}/html" "py/docs/source/api.rst"-sphinx:- configuration: py/docs/source/conf.py+ # build docs+ sphinx-build -b html -d build/docs/doctrees py/docs/source ${READTHEDOCS_OUTPUT}/html
Evidence
The changed RTD config introduces new network/overlay steps as part of the docs build commands, but
no CI job is updated/added to run these same commands. The existing lint-docs CI job runs
Bazel-based doc generation, so regressions in the RTD-specific process would go undetected until RTD
fails.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The PR adds RTD-specific build behavior (download/overlay sdist, then build docs) but does not add an automated check in CI to validate that this path keeps working.
## Issue Context
CI currently builds/lints Python docs using Bazel, which does not cover the new RTD script path. A lightweight GitHub Actions job that runs the same steps (or a minimal subset) would provide regression coverage.
## Fix Focus Areas
- py/docs/.readthedocs.yaml[16-55]
- .github/workflows/ci-python.yml[34-43]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
export PYTHONPATH="${REPO_DIR}/py:${PYTHONPATH:-}" produces a trailing : when PYTHONPATH is
unset/empty, introducing an empty path element and making import resolution depend more on the
current working directory than intended. This reduces reproducibility for Sphinx autodoc/autosummary
imports compared to setting a single, explicit path.
The RTD config constructs PYTHONPATH by always appending :${PYTHONPATH:-}, which can introduce an
empty element; the tox docs env demonstrates a simpler, explicit PYTHONPATH setup for docs
generation.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The RTD script builds `PYTHONPATH` as `${REPO_DIR}/py:${PYTHONPATH:-}` which can leave a trailing `:` when `PYTHONPATH` is empty/unset, adding an empty path element.
### Issue Context
The project’s tox docs environment sets `PYTHONPATH` to a single intended directory without appending an empty element.
### Fix Focus Areas
- py/docs/.readthedocs.yaml[49-49]
### Suggested fix
Build `PYTHONPATH` without a trailing separator, e.g.:
```sh
export PYTHONPATH="${REPO_DIR}/py${PYTHONPATH:+:$PYTHONPATH}"
```
(or simply `export PYTHONPATH="${REPO_DIR}/py"` if no pre-existing `PYTHONPATH` is needed).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💥 What does this PR do?
We publish the current Python API docs on Read the Docs with every commit: https://selenium-python-api-docs.readthedocs.io
RtD builds the docs directly from a clone of the source repo, and can't use bazel. Since some of the Python modules are generated during the build, they don't exist in the repo and are not included in the docs on RtD.
This PR makes it so when it builds the docs, it will first download the latest nightly sdist tarball from TestPyPI and overlay the files from that onto the source repo. This way, the generated Python files from the nightly build will be included in the docs on RtD.
🤖 AI assistance
🔄 Types of changes