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

Library FlaUILibrary timeout not working #201

Open
djalan opened this issue Sep 4, 2024 · 9 comments
Open

Library FlaUILibrary timeout not working #201

djalan opened this issue Sep 4, 2024 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@djalan
Copy link

djalan commented Sep 4, 2024

Hello

The timeout setting is not working. It is always 10 seconds if I either increase it or decrease it.

I made a repo @ https://github.com/djalan/robot_flaui_timeout

*** Settings ***
Library     FlaUILibrary    timeout=3000    screenshot_on_failure=${False}
Library     DateTime


*** Test Cases ***
Please work timeout
    ${now}    Get Current Date
    Log To Console    ${now}
    Wait Until Element Exist    /Window[@Name="it does not matter"]
@JimTempAcc
Copy link

JimTempAcc commented Sep 11, 2024

def _wait_until_element_exist(self, xpath: str, retries: int):
        """Wait until element exist or timeout occurs.

        Args:
            xpath (String): XPath from element which should be hidden
            retries (Number): Maximum number from retries from wait until

        Raises:
            FlaUiError: If element does not exist.
        """

        timer = 0
        old_timeout = self._timeout
        self._set_timeout(0)

        while timer < retries:

            try:
                self._get_element(xpath)
                self._set_timeout(old_timeout)
                return
            except FlaUiError:
                pass

            time.sleep(1)
            timer += 1

        self._set_timeout(old_timeout)
        raise FlaUiError(FlaUiError.ElementNotExists.format(xpath))

In current design, the arg 'retries' decides the wait time in any wait_until_element_xxx method. 'retries'defaults to 10, so it forever sleep(1) * 10 in this case.

@djalan u can increase or decrease arg 'retries' as u need in current usage.

@Nepitwin IMO, change 'retries' into 'interval' and give a 'timeout' for method would be more flexible.
Sample code like

def _wait_until_element_exist(self, xpath: str, interval: float=200, timeout: float=10000):
        old_timeout = self._timeout
        self._set_timeout(0)
        
        max_time = time.time() * 1000 + timeout
        while time.time() * 1000 < max_time:
            try:
                self._get_element(xpath)
                self._set_timeout(old_timeout)
                return
            except FlaUiError:
                pass
            time.sleep(interval / 1000)
        
        self._set_timeout(old_timeout)
        raise FlaUiError(FlaUiError.ElementNotExists.format(xpath))

@Nepitwin
Copy link
Member

Thanks for you information.

Yes this operation should be asweel used with a timeout handling like mentioned. We will fix this behavior.

@Nepitwin Nepitwin added the bug Something isn't working label Sep 16, 2024
@noubar
Copy link
Contributor

noubar commented Sep 23, 2024

According to me it is better to not mix the general timeout and wait until keywords,

It is better to have the "wait until" keyword series independent from general timeout and they should have their own retry times and timeouts.

@Nepitwin Nepitwin self-assigned this Sep 26, 2024
@Nepitwin
Copy link
Member

Next week i will implement a solution for this use case.

@djalan
Copy link
Author

djalan commented Sep 30, 2024

According to me it is better to not mix the general timeout and wait until keywords,

It is better to have the "wait until" keyword series independent from general timeout and they should have their own retry times and timeouts.

I do not understand why we need a second default timeout for keywords branded as "Wait". This will confuse the user for sure.

Furthermore, it is not pythonic to use milliseconds as you propose. Python only has one "sleep" function and it takes a float as a parameter.

time.sleep(1.1)    # 1100 ms
time.sleep(0.5)    # 500 ms

@noubar
Copy link
Contributor

noubar commented Sep 30, 2024

@djalan
Because if you have wait until keyword
then you need to have a loop
The loop should have max times of try and timeout in ms to wait in between tries.
As @JimTempAcc has written in his code example.

Then the question is where do you want to use the general timeout in the loop? as absolute maximal timeout?

then

example:

You want to limit all wait until keywords to the given max timeout. even if the user wants to wait 4 minutes for some loading in one place in all the test suite. Then the user needs to set the general timeout to 4 mins. Then every click failure will wait at least for 4 mins until the keyword fails. Which will result to very slow test runs. This is why the general timeout and wait until timeout should be independent from each other.

@noubar
Copy link
Contributor

noubar commented Sep 30, 2024

The general timeout is more about how fast is your system you are testing and how responsive the ui you are testing. To have stable tests.

Faster Cpu and UI means smaller timeouts, slower cpu or UI means bigger general timeout.

@djalan
Copy link
Author

djalan commented Sep 30, 2024

@noubar Hello!

TLDR: I would stop using the number of retries in the loop and let it be determined by the maximum timeout and the interval. I would use two different timeouts.

I was suggesting to put the default timeout as the default value of the parameter in the function.

def _wait_until_element_exist(self, xpath: str, interval_secs: float=0.2, timeout_secs: float=DEFAULT_TIMEOUT):

if the user wants to wait 4 minutes for some loading in one place in all the test suite. Then the user needs to set the general timeout to 4 mins.

Why would I change the general timeout to 4 minutes when I can use wait_until_element_exist and its parameter.

wait_until_element_exist("//xpath/will/appear/in/four/minutes", timeout=4*60)

The loop should have max times of try

I don't think that a max times of try is relevant; I have never seen this in the past in a few different automation libraries. It should be calculated from the interval between each try and the maximum waiting time.

wait_until_element_exist("//xpath", interval=5 timeout=10)    # check twice in 10 sec
wait_until_element_exist("//xpath", interval=1 timeout=10)    # check once every sec for 10 sec
wait_until_element_exist("//xpath", interval=0.25 timeout=10)    # check four times per second for 10 sec

But... note that I like your comment regarding the responsiveness of the UI and the PC. For instance, we might need a general timeout of only 1 sec for clicks and things like that. It would not really make sense to use wait_until_element_exist to wait for only an extra second (if using the general timeout there); the user probably need a lot more. Something like 5 sec would make more sense. But I prefer a product of the general timeout.

def _wait_until_element_exist(self, xpath: str, interval_secs: float=1.0, timeout_secs: float=GENERAL * 3):

general 1 sec --> wait_until_element_exist gives an extra 3 secs
general 10 sec --> wait_until_element_exist gives an extra 30 secs
A flat number like 5 secs is not realistic if the general timeout is already a big 10 sec. That's why I like a multiplication.

@noubar
Copy link
Contributor

noubar commented Oct 3, 2024

@djalan
Very True
We are defending the exact same thing

You can alternatively use the following in your tests until the wait until keywords here are fixed


Wait Until Keyword Succeeds    2 min   1s   Element Should Exist   <xpath>
or 
Wait Until Keyword Succeeds    10x   1s   Element Should Exist   <xpath>

https://robotframework.org/robotframework/2.6.1/libraries/BuiltIn.html

Look more about The keyword in builtin library of robot
So you can time it to exactly what you want by combining the two keywords

And Element should Exist alone works 100% accuracy with general timeout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants