[py][bidi]: add set_download_behavior command#16556
[py][bidi]: add set_download_behavior command#16556navin772 merged 4 commits intoSeleniumHQ:trunkfrom
set_download_behavior command#16556Conversation
PR Compliance Guide 🔍(Compliance updated until commit 0e88b8c)Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label Previous compliance checksCompliance check up to commit 79a445c
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
The AI bot's |
cgoldberg
left a comment
There was a problem hiding this comment.
nice work.
just 2 small things:
-
we should reset the default download behavior in a fixture or at least in a try/finally, so if the test fails it doesn't leave the browser in a non-default state. (this doesn't matter since we close the browser between BiDi tests, but good to have for the future).
-
pytest has a built-in
tmp_pathfixture that you can use instead of managing them yourself withtempfile.TemporaryDirectory(): https://docs.pytest.org/en/stable/how-to/tmp_path.html#the-tmp-path-fixture
cgoldberg
left a comment
There was a problem hiding this comment.
one more thing.. these tests should be skipped if run with remote webdriver since os.listdir() won't find the downloaded files on the local system.
yea, I think that's a good idea. I'd probably wait 3-4 seconds. |
They actually passed in CI-RBE. IDK why they are failing on chrome and edge for windows in Browser tests. The error says that the command is not supported but we have |
I was thinking they would fail if you used Also, we don't run the BiDi tests against remote browsers in CI, so it doesn't matter. |
cgoldberg
left a comment
There was a problem hiding this comment.
just this: #16556 (comment)
... then it's good to go.
|
I'm not sure what to about the failing tests on Chrome/Edge on Windows. Should we skip/xfail them for now, or hold off merging this to figure it out? It's up to you. |
I have done this using
I think we should hold it, I will try to debug what's happening in the CI. |
in
I think that should be:
|
This resets the behavior for Doing |
|
Seems like windows CI is using the old browsers only (the ones which are present in GH actions by default), chrome is on It is not honoring the |
|
All tests are passing now, merging. |
User description
🔗 Related Issues
💥 What does this PR do?
Adds the
set_download_behaviorcommand to the browser module - https://w3c.github.io/webdriver-bidi/#command-browser-setDownloadBehavior🔧 Implementation Notes
I have used keyword-only params since
destination_folderis conditional (only needed whenallowed=True). It will be easier to use and understand instead of guessing order in positional args.💡 Additional Considerations
Not supported in Firefox stable yet.
🔄 Types of changes
PR Type
Enhancement
Description
Adds
set_download_behaviorcommand to browser moduleSupports allowing, denying, or clearing download behavior
Enables per-user-context download behavior configuration
Includes comprehensive validation and test coverage
Diagram Walkthrough
File Walkthrough
browser.py
Implement set_download_behavior commandpy/selenium/webdriver/common/bidi/browser.py
set_download_behaviormethod with keyword-only parametersallowed=True)
behavior
bidi_browser_tests.py
Add comprehensive tests for download behaviorpy/test/selenium/webdriver/common/bidi_browser_tests.py