#2550 Implement downloading files via CDP #2567

britka · 2023-11-27T12:44:36Z

Proposed changes

Add new download method CDP. It works similar to FOLDER approach but now this process is controlled by browser.
To use this download method you need to use snippet

$("#someId").download(DownloadOptions.using(FileDownloadMode.CDP);

Note

This method works only for Chromium based browsers

For more information see:

Checklist

[*] Checkstyle and unit tests are passed locally with my changes by running gradlew check chrome_headless firefox_headless command
[*] I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java

BorisOsipov

Please add integration tests as in integration.FileDownloadToFolderTest

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java

BorisOsipov

Please add integration tests as in integration.FileDownloadToFolderTest

…cdp-file-download

britka · 2023-11-28T11:56:25Z

Add tests to integration.FileDownloadToFolderWithCdpTest

asolntsev · 2023-12-01T08:39:15Z

@britka Few tests failed in Firefox:

FileDownloadToFolderWithCdpTest > downloadLargeFile() FAILED
    java.lang.IllegalArgumentException: The browser you selected "firefox" doesn't have Chrome Devtools protocol functionality.
        at com.codeborne.selenide.impl.DownloadFileToFolderCdp.initDevTools(DownloadFileToFolderCdp.java:78)
        at com.codeborne.selenide.impl.DownloadFileToFolderCdp.prepareDownloadWithCdp(DownloadFileToFolderCdp.java:83)
        at com.codeborne.selenide.impl.DownloadFileToFolderCdp.download(DownloadFileToFolderCdp.java:49)
        at com.codeborne.selenide.commands.DownloadFile.execute(DownloadFile.java:75)
        at com.codeborne.selenide.commands.DownloadFile.execute(DownloadFile.java:29)
        at com.codeborne.selenide.commands.Commands.execute(Commands.java:161)
        at integration.FileDownloadToFolderWithCdpTest.downloadLargeFile(FileDownloadToFolderWithCdpTest.java:265)

You can add assumeThat(isFirefox()).isFalse() or something similar.

britka · 2023-12-01T09:41:41Z

Ok. I'll add that

britka · 2023-12-04T06:25:30Z

@asolntsev @BorisOsipov I will revert some changes.
In our case we should give file to user anyway. So I suggest to do the next
Variant 1.
When User chose FOLDER method we should check if it is chromium based browser then we apply CDP method
otherwise we continue using FOLDER .

Variant 2.
The same but for CDP method.

What do you think?

BorisOsipov · 2023-12-04T21:56:06Z

@britka

When User chose FOLDER method we should check if it is chromium based browser then we apply CDP method
otherwise we continue using FOLDER .

If the user has chosen FOLDER, we should use the FOLDER method and not confuse the user with some of our predictions about how we can make the file available to them.

…ipped if browser is firefox

sonarcloud · 2023-12-05T06:31:40Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

britka · 2023-12-05T06:33:10Z

@BorisOsipov agree.

britka · 2023-12-05T06:33:59Z

@asolntsev I fixed the tests according to your comments

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java

asolntsev · 2024-01-11T14:56:50Z

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java

+  }
+
+  private void initDevTools(Driver driver) {
+    if (driver.browser().isChromium()) {


I would replace driver.browser().isChromium() by driver.getWebDriver() instanceof HasDevTools

@asolntsev I disagree with you. For example, FirefoxDriver is also implementing HasDevTools interface. But it doesn't support this functionality. It uses devtools v.85

I just tried - Firefox does download the file. Only command setDownloadBehavior doesn't work.

I prefer to leave current implementation if you do not mind

I don't understand why we forbid this functionality in Firefox if it works.

asolntsev · 2024-01-11T14:58:22Z

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java

+      Browser.downloadProgress(),
+      e -> {
+        downloadComplete.set(e.getState() == COMPLETED);
+        log.debug("Download is in progress");


I would log "Download is completed" if e.getState() == COMPLETED`.

I check this in other piece of code.

Yes, but this log is lying. It says Download is in progress, but the download is actually completed.

src/test/java/com/codeborne/selenide/impl/DownloadFileToFolderCdpTest.java

statics/src/test/java/integration/FileDownloadToFolderWithCdpTest.java

…ownload

asolntsev

LGTM

@britka Checkstyle warning:

Unused import at FileDownloadToFolderWithCdpTest.java:14:8

…ownload

britka · 2024-01-22T11:04:41Z

LGTM

@britka Checkstyle warning:

Unused import at FileDownloadToFolderWithCdpTest.java:14:8

@asolntsev Fixed unused imports

sonarcloud · 2024-01-22T14:28:43Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

…e it got "completed" it seems that CDP continues sending events "inProgress" (14 of 14 bytes) even AFTER it already sent the "completed" event. We will just ignore them.

…for N ms this feature ("increment timout") was implemented only in FOLDER mode, now in CDP as well.

CDP may send "in progress" events about multiple files simultaneously. We need to track of all them, and pick the right file using given FileFilter.

e.g. when Chrome is running in Selenoid

…e it got "completed" it seems that CDP continues sending events "inProgress" (14 of 14 bytes) even AFTER it already sent the "completed" event. We will just ignore them.

…for N ms this feature ("increment timout") was implemented only in FOLDER mode, now in CDP as well.

CDP may send "in progress" events about multiple files simultaneously. We need to track of all them, and pick the right file using given FileFilter.

e.g. when Chrome is running in Selenoid

…er on Selenoid)

selenide#2550 Implement downloading files via CDP

66bf770

github-advanced-security bot found potential problems Nov 27, 2023

View reviewed changes

src/main/java/com/codeborne/selenide/impl/DownloadFileToFolderCdp.java Fixed Show fixed Hide fixed

sbritprovectus and others added 2 commits November 27, 2023 16:12

selenide#2550 Implement downloading files via CDP. Update exceptions

904206d

Merge branch 'main' into add-cdp-file-download

dd4c041

BorisOsipov reviewed Nov 27, 2023

View reviewed changes

sbritprovectus added 2 commits November 28, 2023 14:50

selenide#2550 Update code according to code review comments

226102d

Merge remote-tracking branch 'origin/add-cdp-file-download' into add-…

59fd255

…cdp-file-download

britka requested a review from BorisOsipov November 28, 2023 11:56

sbritprovectus added 2 commits November 28, 2023 18:02

selenide#2550 Update exception according to SonarCube error

f5b4b75

selenide#2550 Extends DownloadFileToFolderCdp from DownloadFileToFolder

ec98561

asolntsev linked an issue Dec 1, 2023 that may be closed by this pull request

Implement downloading files via CDP or BiDi #2550

Closed

asolntsev added this to the 7.0.4 milestone Dec 1, 2023

selenide#2550 Update test for FileDownloadToFolderWithCdp. Will be sk…

b6634e6

…ipped if browser is firefox

asolntsev modified the milestones: 7.0.4, 7.1.0 Dec 7, 2023

asolntsev requested changes Jan 11, 2024

View reviewed changes

britka added 3 commits January 14, 2024 21:03

Merge branch 'main' of github.com:britka/selenide into add-cdp-file-d…

1340890

…ownload

Merge branch 'main' of github.com:britka/selenide into add-cdp-file-d…

0586e98

…ownload

selenide#2550 Update code according to comments to PR

d791ee7

britka requested a review from asolntsev January 17, 2024 07:53

asolntsev self-assigned this Jan 18, 2024

asolntsev added feature roadmap downloading files labels Jan 18, 2024

selenide#2550 Fix incrementtimout

06b3642

asolntsev approved these changes Jan 19, 2024

View reviewed changes

britka added 2 commits January 22, 2024 13:54

selenide#2550 Fix unused import

e82e431

Merge branch 'main' of github.com:britka/selenide into add-cdp-file-d…

d5d2289

…ownload

Delete unnecessary parentheses

b8597bc

asolntsev modified the milestones: 7.0.5, 7.1.0 Jan 25, 2024

asolntsev merged commit 72ba3ee into selenide:main Feb 2, 2024
11 of 13 checks passed

asolntsev added a commit that referenced this pull request Feb 2, 2024

#2550 fixup for PR #2567: clear DevTools listeners from previous tests

21c216c

asolntsev added a commit that referenced this pull request Feb 2, 2024

#2550 #2567 add debugging info when downloading files via CDP

0a9c889

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 to make logs more readable, extract lambdas to named classes

4c29a10

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 remove CDP listeners after downloading a file

cff9d3e

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 fail fast if downloading via CDP hasn't get any progress …

373fb12

…for N ms this feature ("increment timout") was implemented only in FOLDER mode, now in CDP as well.

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 support parallel downloading of multiple files

49e19e0

CDP may send "in progress" events about multiple files simultaneously. We need to track of all them, and pick the right file using given FileFilter.

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 don't try to cast remote ChromeDriver

21bd9a8

e.g. when Chrome is running in Selenoid

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 to make logs more readable, extract lambdas to named classes

d03c21d

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 remove CDP listeners after downloading a file

860cb1f

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 fail fast if downloading via CDP hasn't get any progress …

e9e44cb

…for N ms this feature ("increment timout") was implemented only in FOLDER mode, now in CDP as well.

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 support parallel downloading of multiple files

a892fc8

CDP may send "in progress" events about multiple files simultaneously. We need to track of all them, and pick the right file using given FileFilter.

asolntsev added a commit that referenced this pull request Feb 3, 2024

#2550 #2567 don't try to cast remote ChromeDriver

058618e

e.g. when Chrome is running in Selenoid

asolntsev added a commit that referenced this pull request Feb 6, 2024

#2550 #2567 properly unwrap webdriver (e.g. in case of remote webdriv…

e861333

…er on Selenoid)

asolntsev added a commit that referenced this pull request Feb 7, 2024

#2550 #2567 properly unwrap webdriver (e.g. in case of remote webdriv…

817e5f7

…er on Selenoid)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2550 Implement downloading files via CDP #2567

#2550 Implement downloading files via CDP #2567

britka commented Nov 27, 2023 •

edited

BorisOsipov left a comment

BorisOsipov left a comment

britka commented Nov 28, 2023

asolntsev commented Dec 1, 2023

britka commented Dec 1, 2023

britka commented Dec 4, 2023

BorisOsipov commented Dec 4, 2023

sonarcloud bot commented Dec 5, 2023

britka commented Dec 5, 2023

britka commented Dec 5, 2023

asolntsev Jan 11, 2024

britka Jan 12, 2024

asolntsev Jan 13, 2024

britka Jan 17, 2024

asolntsev Jan 18, 2024

asolntsev Jan 11, 2024

britka Jan 17, 2024

asolntsev Jan 18, 2024

asolntsev left a comment •

edited

britka commented Jan 22, 2024

sonarcloud bot commented Jan 22, 2024

#2550 Implement downloading files via CDP #2567

#2550 Implement downloading files via CDP #2567

Conversation

britka commented Nov 27, 2023 • edited

Proposed changes

Checklist

BorisOsipov left a comment

Choose a reason for hiding this comment

BorisOsipov left a comment

Choose a reason for hiding this comment

britka commented Nov 28, 2023

asolntsev commented Dec 1, 2023

britka commented Dec 1, 2023

britka commented Dec 4, 2023

BorisOsipov commented Dec 4, 2023

sonarcloud bot commented Dec 5, 2023

britka commented Dec 5, 2023

britka commented Dec 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asolntsev left a comment • edited

Choose a reason for hiding this comment

britka commented Jan 22, 2024

sonarcloud bot commented Jan 22, 2024

Quality Gate passed

britka commented Nov 27, 2023 •

edited

asolntsev left a comment •

edited