Skip to content

Commit

Permalink
#2550 #2567 fail fast if downloading via CDP hasn't get any progress …
Browse files Browse the repository at this point in the history
…for N ms

this feature ("increment timout") was implemented only in FOLDER mode, now in CDP as well.
  • Loading branch information
asolntsev committed Feb 3, 2024
1 parent cff9d3e commit 373fb12
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public File execute(SelenideElement selenideElement, WebElementSource linkWithHr
}
case CDP: {
return downloadFileToFolderCdp
.download(linkWithHref, link, timeout, options.getFilter(), options.getAction());
.download(linkWithHref, link, timeout, incrementTimeout, options.getFilter(), options.getAction());
}
default: {
throw new IllegalArgumentException("Unknown file download mode: " + options.getMethod());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.function.Consumer;

import static com.codeborne.selenide.impl.FileHelper.moveFile;
import static java.lang.System.currentTimeMillis;
import static java.util.Collections.emptyMap;
import static org.openqa.selenium.devtools.v120.browser.Browser.downloadProgress;
import static org.openqa.selenium.devtools.v120.browser.Browser.downloadWillBegin;
Expand All @@ -49,7 +50,7 @@ public DownloadFileToFolderCdp() {
@CheckReturnValue
@Nonnull
public File download(WebElementSource anyClickableElement,
WebElement clickable, long timeout,
WebElement clickable, long timeout, long incrementTimeout,
FileFilter fileFilter,
DownloadAction action) {

Expand All @@ -58,16 +59,18 @@ public File download(WebElementSource anyClickableElement,

AtomicBoolean downloadComplete = new AtomicBoolean(false);
AtomicReference<String> fileName = new AtomicReference<>();
AtomicLong lastModifiedAt = new AtomicLong(currentTimeMillis());

// Init download behaviour and listeners
prepareDownloadWithCdp(driver, devTools, fileName, downloadComplete, timeout);
prepareDownloadWithCdp(driver, devTools, fileName, downloadComplete, lastModifiedAt, timeout);

// Perform action an element that begins download process
action.perform(anyClickableElement.driver(), clickable);

try {
// Wait until download
File file = waitUntilDownloadsCompleted(anyClickableElement.driver(), timeout, downloadComplete, fileName);
File file = waitUntilDownloadsCompleted(anyClickableElement.driver(), fileFilter,
timeout, incrementTimeout, lastModifiedAt, downloadComplete, fileName);

//
if (!fileFilter.match(new DownloadedFile(file, emptyMap()))) {
Expand All @@ -93,17 +96,23 @@ protected File archiveFile(Driver driver, File downloadedFile) {
return archivedFile;
}

private File waitUntilDownloadsCompleted(Driver driver, long timeout,
AtomicBoolean downloadComplete, AtomicReference<String> fileName) {
private File waitUntilDownloadsCompleted(Driver driver, FileFilter fileFilter,
long timeout, long incrementTimeout,
AtomicLong lastModifiedAt, AtomicBoolean downloadComplete,
AtomicReference<String> fileName) {
long pollingInterval = Math.max(driver.config().pollingInterval(), 100);
Stopwatch stopwatch = new Stopwatch(timeout);
do {
if (downloadComplete.get()) {
log.debug("File {} download is complete after {} ms.", fileName, stopwatch.getElapsedTimeMs());
return new File(driver.browserDownloadsFolder().toString(), fileName.get());
}
else {
failFastIfNoChanges(driver, lastModifiedAt.get(), fileFilter, timeout, incrementTimeout);
}
stopwatch.sleep(pollingInterval);
} while (!stopwatch.isTimeoutReached());
}
while (!stopwatch.isTimeoutReached());

String message = "Failed to download file in %d ms".formatted(timeout);
throw new FileNotDownloadedError(driver, message, timeout);
Expand All @@ -116,35 +125,39 @@ private DevTools initDevTools(Driver driver) {
devTools.createSessionIfThereIsNotOne();
devTools.send(Page.enable());
return devTools;
} else {
}
else {
throw new IllegalArgumentException("The browser you selected \"%s\" doesn't have Chrome Devtools protocol functionality."
.formatted(driver.browser().name));
}
}

private void prepareDownloadWithCdp(Driver driver, DevTools devTools,
AtomicReference<String> fileName, AtomicBoolean downloadComplete, long timeout) {
AtomicReference<String> fileName, AtomicBoolean downloadComplete, AtomicLong lastModifiedAt,
long timeout) {
devTools.send(Browser.setDownloadBehavior(
Browser.SetDownloadBehaviorBehavior.ALLOW,
Optional.empty(),
Optional.of(driver.browserDownloadsFolder().toString()),
Optional.of(true)));

devTools.clearListeners();
devTools.addListener(downloadWillBegin(), new DownloadWillBeginListener(id(), fileName));
devTools.addListener(downloadProgress(), new DownloadProgressListener(id(), driver, downloadComplete, timeout));
devTools.addListener(downloadWillBegin(), new DownloadWillBeginListener(id(), fileName, lastModifiedAt));
devTools.addListener(downloadProgress(), new DownloadProgressListener(id(), driver, downloadComplete, lastModifiedAt, timeout));
}

private static long id() {
return SEQUENCE.incrementAndGet();
}

private record DownloadWillBeginListener(long id, AtomicReference<String> fileName) implements Consumer<DownloadWillBegin> {
private record DownloadWillBeginListener(long id, AtomicReference<String> fileName, AtomicLong lastModifiedAt)
implements Consumer<DownloadWillBegin> {
@Override
public void accept(DownloadWillBegin e) {
log.debug("[{}] Download will begin with suggested file name \"{}\" (url: \"{}\", frameId: {}, guid: {})",
id, e.getSuggestedFilename(), e.getUrl(), e.getFrameId(), e.getGuid());
fileName.set(e.getSuggestedFilename());
lastModifiedAt.set(currentTimeMillis());
}

@Override
Expand All @@ -153,7 +166,8 @@ public String toString() {
}
}

private record DownloadProgressListener(long id, Driver driver, AtomicBoolean downloadComplete, long timeout)
private record DownloadProgressListener(long id, Driver driver,
AtomicBoolean downloadComplete, AtomicLong lastModifiedAt, long timeout)
implements Consumer<DownloadProgress> {
@Override
public void accept(DownloadProgress e) {
Expand All @@ -167,8 +181,7 @@ public void accept(DownloadProgress e) {
throw new FileNotDownloadedError(driver, message, timeout);
}
case COMPLETED -> downloadComplete.set(true);
case INPROGRESS -> {
}
case INPROGRESS -> lastModifiedAt.set(currentTimeMillis());
}
}

Expand All @@ -177,4 +190,18 @@ public String toString() {
return getClass().getSimpleName() + "#" + id;
}
}

protected void failFastIfNoChanges(Driver driver, long lastModifiedAt, FileFilter filter,
long timeout, long incrementTimeout) {
long now = currentTimeMillis();
long filesHasNotBeenUpdatedForMs = now - lastModifiedAt;
if (filesHasNotBeenUpdatedForMs > incrementTimeout) {
String message = String.format(
"Failed to download file%s in %d ms: file hasn't been modified for %s ms. " +
"(lastFileUpdate: %s, now: %s, incrementTimeout: %s)",
filter.description(), timeout, filesHasNotBeenUpdatedForMs,
lastModifiedAt, now, incrementTimeout);
throw new FileNotDownloadedError(driver, message, timeout);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,14 @@ void downloadsFileWithCrdownloadExtension() {
public void canSpecifyTimeoutForFileIncrement_downloadNotEvenStarted() {
var shortIncrementTimeout = using(FOLDER)
.withTimeout(ofSeconds(10))
.withIncrementTimeout(ofMillis(100))
.withIncrementTimeout(ofMillis(1001))
.withFilter(withName("hello_world.txt"));
assertThatThrownBy(() -> $("h1")
.download(shortIncrementTimeout))
.isInstanceOf(FileNotDownloadedError.class)
.hasMessageStartingWith("Failed to download file with name \"hello_world.txt\" in 10000 ms")
.hasMessageMatching(Pattern.compile("(?s).+files in .+ haven't been modified for \\d+ ms\\. +" +
"\\(started at: \\d+, lastFileUpdate: -?\\d+, now: \\d+, incrementTimeout: \\d+\\)\\s*" +
"\\(started at: \\d+, lastFileUpdate: -?\\d+, now: \\d+, incrementTimeout: 1001\\)\\s*" +
"Modification times: \\{.*}.*", DOTALL));

closeWebDriver();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.regex.Pattern;

import static com.codeborne.selenide.Configuration.downloadsFolder;
import static com.codeborne.selenide.Configuration.timeout;
Expand All @@ -29,6 +30,7 @@
import static java.nio.file.Files.createTempDirectory;
import static java.time.Duration.ofMillis;
import static java.time.Duration.ofSeconds;
import static java.util.regex.Pattern.DOTALL;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assumptions.assumeThat;
Expand Down Expand Up @@ -223,12 +225,17 @@ void downloadsFileWithCrdownloadExtension() {
public void canSpecifyTimeoutForFileIncrement_downloadNotEvenStarted() {
var shortIncrementTimeout = using(CDP)
.withTimeout(ofSeconds(10))
.withIncrementTimeout(ofMillis(100))
.withIncrementTimeout(ofMillis(201))
.withFilter(withName("hello_world.txt"));
assertThatThrownBy(() -> $("h1")
.download(shortIncrementTimeout))
.isInstanceOf(FileNotDownloadedError.class)
.hasMessageStartingWith("Failed to download file in 10000 ms");
.hasMessageStartingWith("""
Failed to download file with name "hello_world.txt" in 10000 ms
""".trim())
.hasMessageMatching(Pattern.compile("""
(?s).+: file hasn't been modified for \\d+ ms\\. +\\(lastFileUpdate: -?\\d+, now: \\d+, incrementTimeout: 201\\).*
""".trim(), DOTALL));

closeWebDriver();
}
Expand Down

0 comments on commit 373fb12

Please sign in to comment.