GDATASoftwareAG / robotframework-flaui

Windows user interface automation library for Robot-Framework. FlaUILibrary is a wrapper for the FlaUI automation library.
MIT License
60 stars 12 forks source link

Library FlaUILibrary timeout not working #201

Open djalan opened 1 month ago

djalan commented 1 month ago

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 commented 1 month ago
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 commented 1 month ago

Thanks for you information.

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

noubar commented 1 month ago

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 commented 1 month ago

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

djalan commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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