Closed ksingha161 closed 1 year ago
@ksingha161, thank you for creating this issue. We will troubleshoot it as soon as we can.
Triage this issue by using labels.
If information is missing, add a helpful comment and then I-issue-template
label.
If the issue is a question, add the I-question
label.
If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted
label.
If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C),
add the applicable G-*
label, and it will provide the correct link and auto-close the
issue.
After troubleshooting the issue, please add the R-awaiting answer
label.
Thank you!
Java has a way to do it via addListener
method in DevTools class https://github.com/SeleniumHQ/selenium/blob/trunk/java/src/org/openqa/selenium/devtools/DevTools.java.
After going through the code, I don't think it is possible in JS currently to listen to events. Though you can send CDP commands.
Thanks for the response @pujagani java example is what I'm not able to find
A simple example would be:
import org.openqa.selenium.chrome.ChromeDriver;
import org.openqa.selenium.devtools.DevTools;
import org.openqa.selenium.devtools.v116.browser.Browser;
public class ChromeDevToolsBrowserDownload {
public static void main(String [] args) throws Exception {
ChromeDriver driver = new ChromeDriver();
DevTools chromeDevTools = driver.getDevTools();
chromeDevTools.createSession();
chromeDevTools.addListener( Browser.downloadProgress(), downloadProgress -> {
downloadProgress.getState();
});
driver.quit();
}
}
Ooh, this would be a good real world example for our documentation....
You also need to set the download behavior.
Working example:
driver.get("https://www.selenium.dev/selenium/web/downloads/download.html");
devTools = ((HasDevTools) driver).getDevTools();
devTools.createSession();
CountDownLatch latch = new CountDownLatch(1);
devTools.send(Browser.setDownloadBehavior(Browser.SetDownloadBehaviorBehavior.DEFAULT,
Optional.empty(), Optional.of(""), Optional.of(true)));
devTools.addListener( Browser.downloadProgress(), progress -> {
if (Objects.equals(progress.getState().toString(), "completed")) {
latch.countDown();
}
});
driver.findElement(By.id("file-2")).click();
Assertions.assertTrue(latch.await(10, TimeUnit.SECONDS));
Thank you @titusfortner Works like a charm. 🚀
I guess selenium can have an api that checks for downloads this way 😉
@titusfortner Will the above code for for multiple downloads? I mean if I have 5 downloads in a test suite, how does the CountDownLatch work because the state of previous downloads is already complete.
You will need to set the countdown latch accordingly.
@pujagani If I understand correctly we click on download and the countdown latch is a counter until that particular download is complete right? Now the next test case starts and clicks on download and we use the countdown latch again, so when it checks for the state 'completed', the previously downloaded file is still there in the browser, so the countdown latch will check that and assert as completed while the original download is still in progress.
It depends on how your tests are configured and how the latch is initialized. The countdown latch is simply keeping a count. We have a listener in place, so each time the Browser.downloadProgress() event occurs, the listener is invoked and in the example above it counts down the latch. The listener is simply reacting each time the browser fires the event. You can do any operations inside it. The example shows using a countdown latch to keep track of one download, the same can be extended to keep track of 5 downloads.
@pujagani In my case it works like this, I have a common method that deals with all download cases in all tests.
public static ImmutablePair<String, String> getDownloadedFileByGrid( long sleepInSeconds ) { // grid download code here }
Now if I use CountDownLatch
here it sort of causes an issue because if latch is set to 5 and countdown is set to 15 -
Test 1 - download file, works fine
Test 2 - download file, latch will throw timeout
Any insights on how to handle this?
@pujagani - This is just for my understanding.
Here's what I have inferred using the sample shared above.
devTools.addListener()
is invoked then we add the event object and a callback into a Multimap.org.openqa.selenium.devtools.Event
does not implement a hashCode
+ equals
combo. So I am allowed to add multiple events, with the same method into the callback multimap.With all the above assumptions, I noticed that the below test fails.
Now I am not sure if the failure is hinting at a bug, or if we should explicitly call out in our documentation that once a user adds up an event with devtools, that event listener will keep getting invoked through out that session and if a user does not want that to happen, then they should ensure that they clear the event listeners before setting up something new.
The above failure goes away if between the downloads I invoke devTools.clearListeners();
Also I noticed that because Event
does not implement a hashcode and equals, we are adding duplicate events into the multi map org.openqa.selenium.devtools.Connection#eventCallbacks
. Is this fine ?
@ksingha161 - Take a look at the sample that I shared above and see if that helps.
Basically, for a given Webdriver backed CDP session, you are free to add any number of event listeners. All the event listeners that are targetting browser downloading, will all be handled one after the other. So within a session, lets say you are doing 5 sequential file downloads, then you can add a Countdownlatch aware listener 5 times and it will all work.
The catch is that for the 5th file download, the earlier 4 file download event listeners will still be executed, but you wont see any errors because latch.countDown()
does not throw any errors if its count goes beyond zero.
"we should explicitly call out in our documentation that once a user adds up an event with devtools, that event listener will keep getting invoked through out that session and if a user does not want that to happen, then they should ensure that they clear the event listeners before setting up something new." - This part is correct. We are adding an event listener, that would be valid through the devtools session. Any event listener would work such in my understanding. Basically, we are subscribing to an event and the only way to stop doing that is to unsubscribe. I don't think that is a bug.
I think it is valid to add duplicate events. If a user wants two different consumers for the same event, they should have the freedom to do so. These are CDP low-level APIs that are essentially used to create high-level APIs. The reason they are exposed is to allow users to take advantage of all CDP methods and events as suitable to their requirements.
We are adding an event listener, that would be valid through the devtools session. Any event listener would work such in my understanding. Basically, we are subscribing to an event and the only way to stop doing that is to unsubscribe. I don't think that is a bug.
True that. For an event driven approach that would be the approach.
But in this case, we are confining to handling the file download to a specific instance of that event. So in that case, I think we should perhaps at-least call this out to people that if they are expecting a "Per file download event driven behaviour" then they should perhaps be aware of this. Maybe we could add up this as a sample perhaps? Just a suggestion.
I think it is valid to add duplicate events. If a user wants two different consumers for the same event, they should have the freedom to do so.
Yes. But the events themselves don't need to be duplicated. I think we only need to add up the duplicate consumer which is what the multi map is being used here I guess.
So when a user does
devTools.addListener(Browser.downloadProgress(), progress -> {
if (Objects.equals(progress.getState().toString(), "completed")) {
latch.countDown();
}
});
We should be adding one key called "Browser.downloadProgress"
but with "n" values (1 for each unique consumer)
I agree with Puja, user should be able to do all the things here. CountDownLatch
isjust the wrong tool for the above use case.
Also, keep in mind, this is the CDP implementation. It is going away once we get BiDi. It's offered at all as a convenient stopgap.
On that note, @pujagani do we need to make sure w3c is including downloadProgress in the spec? https://w3c.github.io/webdriver-bidi/ I haven't dug into how they are defining things, yet.
I'd try this:
private final AtomicInteger actionCount = new AtomicInteger(0);
// To increment
actionCount.incrementAndGet();
// To retrieve
int count = actionCount.get();
You can synchronize by waiting for the count to increase by one, rather than having to juggle a countdown.
Thanks @krmahadevan will try the code sample. @titusfortner "It is going away once we get BiDi". Should this implementation to check file downloads be avoided then? What is meant by going away?
This implementation relies on:
import org.openqa.selenium.devtools.v117.browser.Browser;
We're creating something that won't rely on a specific version of Chrome, and custom libraries. The process for doing so is going to take us quite a while, so the code here isn't going away any time soon.... My point is just that we aren't going to wrap or polish the existing implementation, we're going to focus on getting the next implementation working.
@ksingha161 We are supporting CDP as long as we are able to completely support BiDi. Once we support BiDi (its browser agnostic), we might start deprecating CDP support.
@krmahadevan It might be good to call out what the low-level CDP API aims to do versus calling out the behaviour for a specific API that is applicable to this specific use case. Since CDP has a lot of events, it is not feasible to map each event to how our listener works. This is just my thought process. But feel free to add samples to our documentation.
@titusfortner I think we might need to create an issue in that repo for the same. Since they already have https://w3c.github.io/webdriver-bidi/#event-browsingContext-downoadWillBegin, adding downloadProgress
makes sense.
something similar here - https://github.com/w3c/webdriver-bidi/issues/427
Based on the use case described here: https://github.com/w3c/webdriver-bidi/issues/427 Will download completed event suffice? I think it should suffice and download progress might not be needed.
@krmahadevan The example that you provided ( Thank you ), when I'm altering it as per the test needs it does not seem to work -
public static void getFileDownloadConfirmation( String attempt ) throws Exception {
DevTools devTools;
try {
devTools = ( ( HasDevTools ) driver ).getDevTools();
devTools.createSession();
devTools.send( Browser.setDownloadBehavior( Browser.SetDownloadBehaviorBehavior.DEFAULT,
Optional.empty(), Optional.of( "" ), Optional.of( true ) ) );
CountDownLatch latch = setupListener( devTools, attempt );
log.info( "latch countdown starts ======= " );
boolean status = latch.await( 20, TimeUnit.SECONDS );
System.err.println( "Status for download attempt 1 " + status );
if ( status == Boolean.TRUE ) {
log.info( "clearing listener now ======= " );
devTools.clearListeners(); //UNCOMMENT THIS LINE AND THE TEST WILL PASS
}
else {
System.err.println("failure ======================= ");
}
} finally {
System.err.println( tracker );
}
}
private static CountDownLatch setupListener(DevTools devTools, String attempt) {
CountDownLatch latch = new CountDownLatch(1);
devTools.addListener(Browser.downloadProgress(), progress -> {
log.info("download status ==========: {}", progress.getState() );
if ( Objects.equals(progress.getState().toString(), "completed")) {
latch.countDown();
tracker.computeIfAbsent(attempt,
k -> new AtomicInteger(0)).incrementAndGet();
}
});
return latch;
}
public static ImmutablePair<String, String> getDownloadedFileByGrid( long sleepInSeconds, String attempt ) throws Exception {
log.info( " ============ INSIDE DOWNLOAD FILE GRID BLOCK ===============" );
getFileDownloadConfirmation( attempt );
TimeUnit.SECONDS.sleep( sleepInSeconds );
Pair<File, SessionId> downloadDirectory = createDownloadDirectory( ts );
String uri = "/session/%s/se/files".formatted( downloadDirectory.getValue() );
String fileToDownload = "";
try ( HttpClient client = HttpClient.Factory.createDefault()
.createClient( new URL( CommonDataMaps.masterConfigValues.get( "gridUrl" ) ) ) ) {
// Grid download code here
}
try ( HttpClient client = HttpClient.Factory.createDefault()
.createClient( new URL( CommonDataMaps.masterConfigValues.get( "gridUrl" ) ) ) ) {
// Grid download code here
}
log.info( " ============ DONE WITH DOWNLOAD FILE GRID BLOCK ===============" );
return ImmutablePair.of( downloadDirectory.getKey() + File.separator + fileToDownload, fileToDownload );
}
getDownloadedFileByGrid
method gets invoked in every test that contains downloads.
In the logs i don't even see log.info("download status ==========: {}", progress.getState() );
Only see the failure log here, although the download gets completed.
if ( status == Boolean.TRUE ) {
log.info( "clearing listener now ======= " );
devTools.clearListeners(); //UNCOMMENT THIS LINE AND THE TEST WILL PASS
}
else {
System.err.println("failure ======================= ");
}
Based on the use case described here: w3c/webdriver-bidi#427 Will download completed event suffice? I think it should suffice and download progress might not be needed.
+1 . I think most of us ( as users ) are only looking for the download complete event.
@ksingha161 - Couple of things.
RemoteWebDriver
.DevTools
in an elaborate fashion, I would first like to know devtools event handling works with remotewebdriver ? I would like to believe that it does, but still it would be good to first get that confirmed.spoke too soon on (2). They work. From the example here please check if you have augmented the remotewebdriver to be DevTools
aware @ksingha161
@krmahadevan Yes, I've augmented it
driver = new RemoteWebDriver(new URL(gridUrl), chromeOptions);
driver = new Augmenter().augment(driver);
driver is a threadlocal driver
Could it be that in my case getFileDownloadConfirmation
( which contains add listener ) method is getting executed after I click on file download and in the example you shared, the listener is setup before the click?
@ksingha161 -
Could it be that in my case getFileDownloadConfirmation ( which contains add listener ) method is getting executed after I click on file download and in the example you shared, the listener is setup before the click?
Yes. The order matters. First setup the event handler and then trigger the event by clicking. If you flip the order, then the event would have already been raised.
Here's a Grid aware test that runs fine for me.
Note: My Grid is backed by docker containers and running on my local machine.
import org.openqa.selenium.By;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.MutableCapabilities;
import org.openqa.selenium.chrome.ChromeOptions;
import org.openqa.selenium.devtools.DevTools;
import org.openqa.selenium.devtools.HasDevTools;
import org.openqa.selenium.devtools.v116.browser.Browser;
import org.openqa.selenium.remote.Augmenter;
import org.openqa.selenium.remote.RemoteWebDriver;
import org.testng.annotations.Test;
import org.testng.asserts.SoftAssert;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
public class WaitForDownloadsToFinishTest {
private final Map<String, AtomicInteger> tracker = new ConcurrentHashMap<>();
@Test
public void testMethod() throws InterruptedException, MalformedURLException {
RemoteWebDriver driver = new RemoteWebDriver(new URL("http://localhost:4444"), new ChromeOptions());
try {
driver.get("https://www.selenium.dev/selenium/web/downloads/download.html");
augmentCDPURLToBeDockerFriendly(driver, "localhost", 4444);
DevTools devTools = setupDevToolsToTrackFileDownloads(driver);
CountDownLatch latch = setupListener(devTools, "One");
driver.findElement(By.id("file-2")).click();
boolean status = latch.await(10, TimeUnit.SECONDS);
System.err.println("Status for download attempt 1 " + status);
devTools.clearListeners(); //UNCOMMENT THIS LINE AND THE TEST WILL PASS
latch = setupListener(devTools, "Two");
driver.findElement(By.id("file-2")).click();
status = latch.await(10, TimeUnit.SECONDS);
System.err.println("Status for download attempt 2 " + status);
} finally {
System.err.println(tracker);
driver.quit();
}
SoftAssert softly = new SoftAssert();
tracker.forEach((key, value) -> softly.assertEquals(value.intValue(), 1, key + "'s callback should have been invoked exactly once"));
softly.assertAll();
}
private CountDownLatch setupListener(DevTools devTools, String attempt) {
CountDownLatch latch = new CountDownLatch(1);
devTools.addListener(Browser.downloadProgress(), progress -> {
if (Objects.equals(progress.getState().toString(), "completed")) {
latch.countDown();
tracker.computeIfAbsent(attempt,
k -> new AtomicInteger(0)).incrementAndGet();
}
});
return latch;
}
private static DevTools setupDevToolsToTrackFileDownloads(RemoteWebDriver driver) {
DevTools devTools = ((HasDevTools) new Augmenter().augment(driver)).getDevTools();
devTools.createSession();
devTools.send(Browser.setDownloadBehavior(Browser.SetDownloadBehaviorBehavior.DEFAULT,
Optional.empty(), Optional.of(""), Optional.of(true)));
return devTools;
}
private static void augmentCDPURLToBeDockerFriendly(RemoteWebDriver driver, String host, int port) {
Capabilities caps = driver.getCapabilities();
if (caps instanceof MutableCapabilities) {
MutableCapabilities mutable = (MutableCapabilities) driver.getCapabilities();
String url = String.format("ws://%s:%d/session/%s/se/cdp", host, port, driver.getSessionId().toString());
mutable.setCapability("se:cdp", url);
} else {
throw new IllegalStateException("Cannot override CDP URL");
}
}
}
@krmahadevan Thank you, the code works. However, what I'm noticing after multiple test executions, the code or I should say this event, it fails when we click on download and in some scenarios a new tab is opened temporarily before the file downloads. That is when the even is failing to get the download progress.
You would need to share a sample that can be used to reproduce the event failing part. Hard to say without the sample.
@krmahadevan Can you give this a try, in this case after clicking on download icon, the page opens a new tab for some milli seconds and then the download starts.
public class WaitForDownloadsToFinishTest {
private final Map<String, AtomicInteger> tracker = new ConcurrentHashMap<>();
@Test
public void testMethod() throws InterruptedException {
ChromeOptions chromeOptions = new ChromeOptions();
chromeOptions.addArguments( "--plugins.always_open_pdf_externally" );
Map<String, Object> chromePrefs = new HashMap<>();
chromePrefs.put( "download.prompt_for_download", Boolean.FALSE );
chromePrefs.put( "plugins.always_open_pdf_externally", Boolean.TRUE );
chromeOptions.setExperimentalOption("prefs", chromePrefs);
RemoteWebDriver driver = new ChromeDriver(chromeOptions);
try {
driver.get( "https://kifstradecapital.com/dummy-downloads/" );
TimeUnit.SECONDS.sleep( 2 );
driver.findElement( By.xpath( "//img[@alt=\"Close\"]" ) ).click();
DevTools devTools = ( ( HasDevTools ) driver ).getDevTools();
devTools.createSession();
devTools.send( Browser.setDownloadBehavior( Browser.SetDownloadBehaviorBehavior.DEFAULT,
Optional.empty(), Optional.of( "" ), Optional.of( true ) ) );
CountDownLatch latch = setupListener( devTools, "One" );
// click on download
driver.findElement( By.xpath( "//div[contains(@class,\"d-download-content\")]//a[contains(@href,\"\")]" ) ).click();
System.out.println("latch countdown starts ================ ");
boolean status = latch.await( 10, TimeUnit.SECONDS );
System.err.println( "Status for download attempt 1 " + status );
devTools.clearListeners(); //UNCOMMENT THIS LINE AND THE TEST WILL PASS
} finally {
System.err.println( tracker );
driver.quit();
}
SoftAssert softly = new SoftAssert();
tracker.forEach( ( key, value ) -> softly.assertEquals( value.intValue(), 1, key + "'s callback should have been invoked exactly once" ) );
softly.assertAll();
}
private CountDownLatch setupListener( DevTools devTools, String attempt ) {
CountDownLatch latch = new CountDownLatch( 1 );
devTools.addListener( Browser.downloadProgress(), progress -> {
if ( Objects.equals( progress.getState().toString(), "completed" ) ) {
latch.countDown();
tracker.computeIfAbsent( attempt,
k -> new AtomicInteger( 0 ) ).incrementAndGet();
}
} );
return latch;
}
}
@ksingha161 - I spent sometime trying to figure out what is going on. Here's what I have inferred.
I haven't been able to get past this information. Maybe @titusfortner @pujagani have more useful suggestions here ?
Thank you @krmahadevan for looking into this in depth. Though it seems like the browser is not firing the event when the tab is open because we are subscribed to the events in the current window and that is the window where the CDP session is created. Now, if the test opens another window, we do not have a CDP session in that window and hence are not listening to events in that window. https://github.com/SeleniumHQ/selenium/blob/ca15a17230bf6de01b3dad44c0525036ac038d47/java/src/org/openqa/selenium/devtools/DevTools.java#L127C19-L127C19 This is how CDP functions. CDP was designed with debugging in mind and not test automation. This is addressed in the upcoming BiDi protocol which focuses on test automation, where one can subscribe to events from multiple browsing contexts (i.e. in this case multiple windows).
@pujagani maybe too much to ask here.. Is there any workaround that can be done?
I don't think there is a workaround.
@ksingha161 - Well, atleast for the website in your sample, I believe that we could do something like this
download
attribute to the anchor so that the browser treats this as a downloadable element.By how = By.xpath("//a[contains(@href,'Complaint-Redressal-Mechanism.pdf')]");
WebElement downloadLink = driver.findElement(how);
//First add the "download" attribute to the href so that the browser treats this as something
//that can be downloadable.
driver.executeScript("arguments[0].setAttribute('download','');", downloadLink);
// click on download
downloadLink.click();
Below is the full sample that shows this in action. Hope that helps!
Thank you so much @krmahadevan will try and integrate this with my test cases.
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Feature and motivation
I understand this is a chrome devtools api,
Browser.downloadProgress(), Browser.downloadWillBegin()
and similar. I just wanted to understand if there are any examples for how to use them.As the names of the api suggests I think it would be really helpful in download scenarios where we want to check if a download is complete, right now there is no way of checking if the download is complete.
Usage example
All examples over the internet are about puppeteer and other js tools which use the same api.