Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Selenium downloadFile command #12778

Merged
merged 1 commit into from May 23, 2024

Conversation

ccharnkij
Copy link
Contributor

Proposed changes

This change adds all 3 of Selenium Grid download command. For downloadFile, the logic is pretty much straight from Selenium JS binding

Types of changes

  • Polish (an improvement for an existing feature)
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improvements to the project's docs)
  • Specification changes (updates to WebDriver command specifications)
  • Internal updates (everything related to internal scripts, governance documentation and CI files)

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Further comments

If needed and this is good, I can make the same change for main as well.

#12671

Reviewers: @webdriverio/project-committers

@christian-bromann
Copy link
Member

christian-bromann commented May 2, 2024

Thanks for raising the PR!

If needed and this is good, I can make the same change for main as well.

Yes please!

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Some more comments

@ccharnkij
Copy link
Contributor Author

@christian-bromann I've made the changes you suggested. Please check again. For Safari, it doesn't seem like this feature is available. I've tried to setup local grid on my mac and noticed that Safari didn't have the download capability like the other 3 browsers.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Two more comments.

packages/webdriverio/src/commands/browser/downloadFile.ts Outdated Show resolved Hide resolved
packages/webdriverio/src/commands/browser/downloadFile.ts Outdated Show resolved Hide resolved
@ccharnkij
Copy link
Contributor Author

ccharnkij commented May 3, 2024

@christian-bromann better?

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

👍

@christian-bromann
Copy link
Member

Mind providing the same PR for the main branch?

@ccharnkij ccharnkij mentioned this pull request May 23, 2024
11 tasks
@christian-bromann christian-bromann added the PR: New Feature 🚀 PRs that contain new features label May 23, 2024
@christian-bromann
Copy link
Member

@ccharnkij mind rebasing the branch?

@christian-bromann christian-bromann merged commit 4da9867 into webdriverio:v8 May 23, 2024
7 of 8 checks passed
@christian-bromann
Copy link
Member

Thanks again for helping us backport this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: New Feature 🚀 PRs that contain new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants