Enable managed downloads in Dynamic Grid and add file upload test - #3207
Merged
VietND96 merged 7 commits intoAug 13, 2026
Merged
Conversation
…rieval Add a remote file upload test using LocalFileDetector and extend the download test to actually retrieve the managed download to the client. Both exercise the file-operation forwarding path from SeleniumHQ/selenium#17914, where the Node forwards upload/downloadFile to the session whenever the browser does not share the Node filesystem. These tests run in the shared Selenium suite, so they cover Dynamic Grid on Docker and Kubernetes and Relay, where the browser runs in a separate container/Pod/endpoint. Static nodes and standalone keep working since LocalFileDetector transfer is a no-op against the local filesystem. The upload test is skipped for the Android emulator relay, where HTML file upload via LocalFileDetector does not apply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
Enable managed downloads on the desktop relay path (NodeFirefox relaying into the standalone browser) so the upload and download tests actually exercise the relay session file-operation forwarding from SeleniumHQ/selenium#17914. Previously the relay test disabled managed downloads, so test_download_file returned early and the relay download path was never verified. Now the relay target standalone runs with managed downloads enabled and the client requests downloads, so get_downloadable_files/downloadFile are forwarded through the relay node to the standalone browser, and LocalFileDetector uploads are forwarded the same way. This confirms the relay session reports a remote filesystem. Kept off for the Android emulator relay, where managed downloads and HTML file upload via LocalFileDetector do not apply. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
…rified Turn on managed downloads for the Dynamic Grid deployments that spawn the browser away from the Node, so the upload/download suite verifies the remote file forwarding from SeleniumHQ/selenium#17914 instead of skipping it. - Docker Dynamic Grid: default SELENIUM_ENABLE_MANAGED_DOWNLOADS to true in the test_node_docker target. It previously defaulted to false and only checked the legacy shared download volume; now the managed download API is retrieved through the DockerSession, exercising the forwarding path. - Kubernetes Dynamic Grid: enable managed downloads on the standalone-kubernetes and node-kubernetes deployments and default the test flag to true. The Node forwards downloadFile to the browser Job Pod, so managed downloads are now retrievable; the outdated comment saying they are not is updated. Enabling it on the Node mirrors the node-docker setup, where the dynamic-grid Node handles retrieval for the browsers it spawns without extra per-browser configuration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
|
|
Contributor
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
…ents CI failed on the Relay and Kubernetes Dynamic Grid lanes after enabling remote downloads: those sessions only forward file upload/download to the browser with SeleniumHQ/selenium#17914 (Selenium 4.48.0). On the current release base, enabling managed downloads there makes get_downloadable_files time out and the LocalFileDetector upload land where the browser cannot see it. Gate the remote verification behind TEST_UPLOAD_DOWNLOAD_REMOTE (default true) and turn it off for the Relay and Kubernetes harnesses until the Grid base includes the fix. Docker Dynamic Grid and standalone keep verifying it, since DockerSession forwards on every Selenium version. Also revert the managed downloads enablement for the Relay standalone target and the Kubernetes Node deployments so those lanes return to their known-good state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
…atch The Relay lane failed with managed downloads on because only the relay target (standalone) and the client had it enabled, not the relay Node itself. With the Node's slot not advertising se:downloadsEnabled, a session created with enable_downloads had no matching slot and failed, taking the whole lane down. Enable managed downloads on node-relay-standalone too, so its slot advertises se:downloadsEnabled, the request matches, and the Node forwards the file operations to the standalone browser it relays into (SeleniumHQ/selenium#17914). Also drop the TEST_UPLOAD_DOWNLOAD_REMOTE version gate added earlier: CI builds on the Selenium nightly, which already carries the fix, so the remote upload/download verification runs on Relay and Kubernetes as intended. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
The Relay lane still failed session creation with "could not find matching slot": every Firefox test errored in setUp. Enabling managed downloads on the relay Node is not enough, because Selenium's slot matcher requires the slot stereotype to carry se:downloadsEnabled when the request asks for it, and a relay stereotype is taken verbatim from relay_config.toml rather than augmented by --enable-managed-downloads the way a regular Node's slots are. Add se:downloadsEnabled to the relay stereotype, templated from SELENIUM_ENABLE_MANAGED_DOWNLOADS so it is true for the desktop Firefox relay and false for the Android relay. The client's managed-download request now matches the relay slot; the session is created and the Node forwards the file operations to the standalone browser it relays into. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
Make the SELENIUM_ENABLE_MANAGED_DOWNLOADS fallback true across the relay compose services so an unset value enables managed downloads by default, matching the client default; the Android relay still sets it false explicitly. Remove verbose explanatory comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VxGB7ifcYFULc8hDiwPfUf
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Motivation and Context
Types of changes
Checklist