Tushard8 / robotframework-seleniumlibrary

Automatically exported from code.google.com/p/robotframework-seleniumlibrary
Apache License 2.0
0 stars 0 forks source link

Make use of RobotFramework TimeoutError to enable Running Keywords on timeout #126

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
RobotFramework 2.5 offers a new keyword:
Run Keyword If Timeout Occurred
However Timeouts from Selenium are not considered as RobotFramework Timeouts, 
thus this keyword will never run.
The reason for that is that selenium throws plain Exceptions. Which work fine 
for RobotFramework, but never cause TimeoutError.

My patch attached fixes this by handling the places where timeouts can occur 
(opening page, waiting for page load), inspecting the Exception Message and 
raising a more specific error instead.

Original issue reported on code.google.com by Lange.Fa...@gmail.com on 7 Jul 2010 at 12:56

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the patch. It will be included in the next release :)

Original comment by Andreas.EbbertKarroum@gmail.com on 7 Jul 2010 at 1:42

GoogleCodeExporter commented 8 years ago
Interesting idea. The `Run Keyword If Timeout Occurred` keyword was originally 
designed to be used with test and keyword timeouts but allowing it to be used 
with this kind of timeouts is actually a very good idea. Should we also change 
RF's BuiltIn `Wait Until Keyword Succeeds` to raise TimeoutError if a timeout 
occurs?

The provided patch looks fine too. The preferred syntax to raise exceptions in 
Python (and RF code base) is `raise TimeoutError(message)` instead of `raise 
TimeoutError, message`.

Original comment by pekka.klarck on 7 Jul 2010 at 2:55

GoogleCodeExporter commented 8 years ago
I looked through the codebase to get an idea of what the intention of the new 
"Run Keyword If Timeout Occured" might have been.
I was surprised to find out it is not used by "Wait Until Keyword Succeeds", 
but did not fully understand if the semantics are the same. If they are they 
should use the same concept.

The actual use case I am using this is not that very nice, but Ill include it 
here for completeness:
We are testing a remote application which is guarded by an instable firewall. 
This firewall causes timeouts in selenium.
At the moment they appear as critical failures in our log. Which leads to 
investigations why the test failed only finding the firewall again killed it.
With the new keyword and my patch we can now tag the test case wit a 
noncritical tag.
So we can more easily see how many tests were affected by a timeout, and also 
stop wasting time investigating. Of course it does not give an indication on 
the quality of the timeouted test. So a real judgement on the overall quality 
can only be done when no tests timeouted. But that situation remains unchanged.

Original comment by Lange.Fa...@gmail.com on 7 Jul 2010 at 3:25

GoogleCodeExporter commented 8 years ago
I thought about this a little more and realized that there are some problems 
with the proposed idea. TimeoutErrors are used internally by RF when test or 
keyword timeouts occur, and these exceptions are also handled specially 
afterwards. The biggest difference compared to the normal exceptions is that 
these exceptions are re-raised by some keywords (e.g. `Run Keyword And Expect 
Error` and `Wait Until Keyword Succeeds`) and they also cancel the effect of 
the new continue on failure mode.

If keywords in libraries themselves use TimeoutErrors, the keywords thus cannot 
be used inside `Run Keyword And Expect Error` and other similar keywords. This 
kind of change would be backwards incompatible and losing this functionality 
is, in my opinion, worse than gaining the possibility to use `Run Keyword If 
Timeout Occurred`.

I understand the need to have callbacks in situations like described in comment 
3. Similar needs could also appear in other contexts than with timeouts, 
though, which is another reason to solve this somehow otherwise than with 
TimeoutErrors. I'd like to have a generic solution in RF itself but don't have 
any really good idea how to accomplish it.

This discussion should probably be moved to the mailing list so that others 
could also participate.

Original comment by pekka.klarck on 8 Jul 2010 at 8:39

GoogleCodeExporter commented 8 years ago
Ok, do you think this patch is then invalid?

Original comment by Lange.Fa...@gmail.com on 9 Jul 2010 at 9:32

GoogleCodeExporter commented 8 years ago
Due to the consequences I mentioned in the comment #4 I think this patch should 
not be applied. TimeoutErrros are better left for test and keyword timeouts and 
some other solution should be found for this use case. 

Could you perhaps use the ${TEST MESSAGE} variable in the test teardown like 
this:

Run Keyword If  "${TEST MESSAGE}" == "<the timeout msg>"  Set Tags  some  tags

Original comment by pekka.klarck on 12 Jul 2010 at 10:38

GoogleCodeExporter commented 8 years ago
Well this IS a timeout of a Keyword. The Selenium Keyword runs in a timeout. So 
why shouldn't it use the TimeoutError to indicate that a timeout inside the 
keyword has occoured?

I see a point in keeping backwards compatibility, but that argumentation 
doesn't fit for me.

So you are saying I should not be able to use "Run Keyword if Timeout Occured" 
when a Selenium Keyword had a timeout?

Original comment by Lange.Fa...@gmail.com on 12 Jul 2010 at 4:40

GoogleCodeExporter commented 8 years ago
Yes, I think TimeoutErrors should only be used by the framework when a test 
timeout or keyword timeout [1] occurs. Notice that these timeouts are different 
from connections timeouts or other timeouts occurring inside keywords.

[1] 
http://robotframework.googlecode.com/svn/tags/robotframework-2.5/doc/userguide/R
obotFrameworkUserGuide.html#timeouts

If, for example, `Open Browser` keyword would use TimeoutError when a 
connection timeout occurs, you couldn't use it inside `Wait Until Keyword 
Succeeds` or `Run Keyword And Expect/Ignore Error` anymore. This is because 
these keywords will let TimeoutErrors go through because this special exception 
denotes that the test/keyword should be stopped immediately.

All this means that `Run Keyword if Timeout Occurred` can only be used when a 
test or keyword timeout occurs and not when a some other timeout occurs inside 
a keyword. For the latter usage we can try to find out some mechanism to 
register callbacks -- as I commented earlier you could have similar needs 
without any kind of timeouts.

Original comment by pekka.klarck on 12 Jul 2010 at 11:22

GoogleCodeExporter commented 8 years ago
Understood, though a bit confusing. I think a timeout should be a timeout, and 
not handled differently when a keyword times out or code inside a keyword times 
out (which is a really not so easy to understand differentiation :-))

From what you say I "could" do the following:
Use the timeout wrapper example
http://robotframework.googlecode.com/svn/tags/robotframework-2.5/doc/userguide/R
obotFrameworkUserGuide.html#user-keyword-timeout
Set it to selenium timeout-1 second. (e.g. 14-1)

Selenium Keyword will run for 14 seconds, would still wait an additional second 
before reaching its timeout.
Then the Robot Timeout would kick in, stop selenium keyword, and throw the 
timeout error.

And the reason for doing so is the different handling of TimeoutErrors and 
"keyword internal timeouts".

But I wonder how I could use "Wait Until Keyword Succeeds" with a Selenium 
Keyword like "Open Page". Would I just wrap it. It would try for 2 Minutes, in 
15 second steps, until the default robot timeout occurs?

I see that there a conceptual problems with my patch. So it shouldn't be 
applied until this is smoothed out.

However it works like a charm and solved all our timeout issues our suboptimal 
test setup :-) So if anybody stumbles across this, understands Pekkas 
reasoning, you might want to try this out still.

Original comment by Lange.Fa...@gmail.com on 13 Jul 2010 at 7:14

GoogleCodeExporter commented 8 years ago
Apart from the timeout!=timeout discussion, I wanted to test the approach that 
Pekka proposed:

${TEST MESSAGE} is great, but how do I check if it contains "Timed out"? (The 
full message is "Timed out after 15000.0ms", and that depends on the selenium 
timeout, so I'd like to not check for the amount of milliseconds)

Robot allows to use the Extended Variable Syntax 
(http://robotframework.googlecode.com/svn/tags/robotframework-2.5/doc/userguide/
RobotFrameworkUserGuide.html#extended-variable-syntax) so I thought I could use 
a method from the Python library 
(http://docs.python.org/library/stdtypes.html#sequence-types-str-unicode-list-tu
ple-buffer-xrange):

Run Keyword If  ${TEST MESSAGE.index('Timed out')}  Set Tags  Timeouted

But, the extended variable syntax only works for variables without space. ("The 
body of the name consists of all the characters after ${ until the first 
occurrence of a non-alphanumeric character or a space")

Any idea how to proceed with this?

Original comment by Andreas.EbbertKarroum@gmail.com on 28 Jul 2010 at 7:27

GoogleCodeExporter commented 8 years ago
Hm, tried it now like this and works:
    ${errormessage}=  Set Variable  ${TEST MESSAGE}  
    Run Keyword If  ${errormessage.find('Timed out')} >= 0  Set Tags  Timeouted

Original comment by Andreas.EbbertKarroum@gmail.com on 28 Jul 2010 at 9:26

GoogleCodeExporter commented 8 years ago
There seems to be a bug in the User Guide because `${SPACE IN NAME.method()}` 
actually does work. I believe the problem in your first attempt was that 
`str.index` raises an exception if the searched substring is not found. The 
best method to use in this case is probably `str.count`, because it returns 
correct truth value automatically and there's no need for `>= 0`:

    Run Keyword If  ${TEST MESSAGE.count('Timed out')}  Set Tags  Timeouted

The Pythonic way to test does a string contain a substring is using `in` 
operator, but that doesn't work with the extended variable syntax. `Run Keyword 
If` handles also Python expressions so this could be used instead:

    Run Keyword If  'Timed out' in '${TEST MESSAGE}'  Set Tags  Timeouted

Original comment by pekka.klarck on 3 Aug 2010 at 10:19

GoogleCodeExporter commented 8 years ago
Hi Pekka,

thanks for another Python lesson for me :)

1) I have not even tried the version with the space in variable name, after I 
saw the docs.
2) count/in good to know, I will change our teardown accordingly.

BTW: I think we can close that issue now.

Original comment by Andreas.EbbertKarroum@gmail.com on 3 Aug 2010 at 12:05

GoogleCodeExporter commented 8 years ago

Original comment by janne.t....@gmail.com on 15 Nov 2010 at 3:14