download_timeout not used

[raphaelcamus]

Describe the bug
It seems that whatever value I set in download_timeout parameter of Promise To Wait For Download keyword, I always get an error :
TimeoutError: page.waitForEvent: Timeout 15000ms exceeded while waiting for event "download"
(in case download takes more than 15s of course)

To Reproduce
Make a download of a large file that will exceed default timeout

{download_promise} =  Promise To Wait For Download  download_timeout=90 sec
Click Element  id=id_button
${file_obj} =  Wait For  ${download_promise}

Expected behavior
"Wait For" times out only when download_timeout is reached, not a 15s default one.

Screenshots
N/A

Desktop (please complete the following information):

• OS: Windows
• Browser Chromium

(By the way, the documentation is unclear: https://marketsquare.github.io/robotframework-browser/Browser.html#Promise%20To%20Wait%20For%20Download, especially keyword output)

[aaltat]

@allcontributors please add @raphaelcamus for bugs

[allcontributors]

@aaltat

I've put up a pull request to add @raphaelcamus! 🎉

[raphaelcamus]

Hi. Could you please increase priority ? This is preventing us from migrating from SeleniumLibrary to Browser :(
Thanks !

[aaltat]

Well, priority is just indicative thing. In practice it is used for release notes. That being said, the fastest way is to implement this one by someone else. Meaning by you or by someone from your company. If that is not possible, then you need to wait that someone is willing to implement it.

This is not a promise to implement it, but your need is heard and helping someone to migrate from SL to Browser is always a plus for me.

[raphaelcamus]

Adding: Set Browser Timeout ${download_timeout} scope=Task before calling Wait For seems to work as a workaround

(I also tried to override wait_for to pass timeout to pass to .result() but that does not work. I guess it does not work this way at all.)

I'll just wait for someone then.

[Snooz82]

@raphaelcamus
This is a general thing.
All our keywords use the global default timeout. There are very few keywords that have their own timeouts.

In case of Promise To Wait For Download what ever fires first, causes a failure.
So yes. You have to set the "Browser Timeout" to a higher value and then do the Promise keyword.

I think this is more of a documentation task.
Same issue is with retry assertions. If they are longer than the global timeout, that one fires first.

I don't think the backwards incompatibility that comes with changing the current behaviour is worth the advantage.

Lets improve the docs here.

[raphaelcamus]

@Snooz82 Meantime we found out that it was kind of working right.
Download itself was not starting right away, so resulting in a timeout (managed by global timeout).
And I believe that download_timeout is timeout only for the download part, not the download preparation time.
Maybe it could be interesting to have 2 timeouts in here: start_download_timeout and download_timeout

Here is our solution to handle this:

Prepare Download
    [Documentation]    Prepare the download of a file.
     ...    - start_download_timeout: timeout to wait for the download to start (managed with global Browser timeout)
     ...    - download_timeout: timeout to wait for the download to finish
     ...    - Returns a promise to wait for the download to finish, and the previous browser timeout (to restore it after the download)
    [Arguments]  ${start_download_timeout}=${Browser_Timeout}  ${download_timeout}=${None}
    ${old_browser_timeout} =  Set Browser Timeout  ${start_download_timeout}  scope=Test
    ${download_promise} =  Promise To Wait For Download  download_timeout=${download_timeout}
    [Return]    ${download_promise}  ${old_browser_timeout}

Wait For Download
    [Documentation]    Wait for the download to finish.
    ...    - download_promise: promise to wait for the download to finish (returned by 'Prepare Download' keyword)
    ...    - old_browser_timeout: previous browser timeout (returned by 'Prepare Download' keyword, in order to restore it)
    ...    - Returns the file name and the basename of the downloaded file
    [Arguments]    ${download_promise}  ${old_browser_timeout}=${Browser_Timeout}
    ${file_obj} =   Wait For  ${download_promise}
    Set Browser Timeout    ${old_browser_timeout}  scope=Test
    ${fileName} =  Rename Downloaded File  ${file_obj}
    ${basename} =    Get Basename Without Extension   ${fileName}
    [Return]  ${fileName}  ${basename}

Get Basename Without Extension
    [Arguments]    ${file_path}
    ${basename} =    Evaluate    os.path.basename(r'${file_path}')    os
    ${basename_without_ext} =    Evaluate    os.path.splitext(r'${basename}')[0]    os
    [Return]    ${basename_without_ext}

Rename Downloaded File
    [Arguments]  ${file_obj}
    ${save_as} =  Set Variable  ${file_obj.saveAs}
    ${suggested_filename} =  Set Variable  ${file_obj.suggestedFilename}
    ${directory} =  Evaluate  os.path.dirname(r'${save_as}')  os
    ${new_path} =  Evaluate  os.path.join(r'${directory}', '${suggested_filename}')  os
    Move File  ${save_as}  ${new_path}
    Log  Renamed file from ${save_as} to ${new_path}
    [Return]  ${new_path}