daluu / plrobotremoteserver

Moved my project here due to Google Code's demise
http://code.google.com/p/plrobotremoteserver
1 stars 1 forks source link

Perl RobotRemoteServer is picking 2 different keywords with similar names #12

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Have 2 methods with similar names: Check_AllType_CustomeField_Exist and 
CheckAllTypeCustomeFieldExist.
2. Write the RobotRemoteServer to list both these as keywords
3. Run the testcases using "Check_AllType_CustomeField_Exist"

What is the expected output? What do you see instead?
It should match to the first keyword and run that test case. Instead, it 
toggles randomly, at times it picks Check_AllType_CustomeField_Exist and at 
times CheckAllTypeCustomeFieldExist

What version of the product are you using? On what operating system?
Robot Framework 2.8.1 (Python 2.7.5 on linux2)

Please provide any additional information below.
This is a piece from Robot logs

 Remote.Check AllType CustomeField Exist 
 Remote.Check All Type Custome Field Exist

Original issue reported on code.google.com by poornima...@gmail.com on 7 Jul 2014 at 11:38

GoogleCodeExporter commented 9 years ago

Original comment by manga...@gmail.com on 23 Jul 2014 at 5:08

GoogleCodeExporter commented 9 years ago
Fair enough, this can be considered a valid issue, for which the remote server 
should handle better somehow. This is likely due to how Robot Framework (RF) 
itself treats keywords implemented in code as it supposedly supports keywords 
in these formats:

* my_keyword_name (Pythonic style)
* MyKeywordName (Camel Case style for other languages)

with both formats being interpreted the same by RF as "My Keyword Name". As 
such, defining such same names as 2 methods this way will surely confuse RF, 
not knowing which keyword to use, so it may be RF is sending request to the 
remote server to execute the wrong keyword because it is confused.

As such, why would one even write such conflicting method names here? Yes, from 
a coding standpoint (if we ignore the fact that RF may treat them as same) they 
are 2 separate methods. But as a user/developer reading code (without looking 
at the Perl doc or looking into the method implementation) just looking over 
the method names will be confusing even though they follow a different naming 
format because they define the same thing if we just look at it as pure "text". 
That's just bad code/software design.

The proper fix for this is either the remote server or RF itself throws a 
warning complaining that you are potentially defining duplicate keywords and 
the actual behavior is unpredictable, or even throw exception instead refusing 
to run due to this potential conflict until you resolve it.

One should be able to use either keyword name format and mix usage of them both 
in same library (though the latter is bad practice, just stick with one 
format), but one should ideally not define conflicting method names as you 
present here. Unless there is some relation of them to each other.

Take for example, the common practice of 

_variableName vs variableName

where the version with underscore is internal to the class while the other one 
is the public one accessible outside of the class and the public one simply 
refers to the private one when setting/getting its value. Following that, if 
one of your methods calls the other one in same design, then that makes sense, 
otherwise, it seems like bad design.

Original comment by manga...@gmail.com on 23 Jul 2014 at 5:30

GoogleCodeExporter commented 9 years ago
The latter is the exact design we are following, the keywords with "_" act as 
wrappers around the keywords without them. For certain users, we enable only 
the wrapper keywords while for others we enable both of them. This is when we 
encountered the above issue.
The actual issue came in when RF started splitting by both "_" and Capital 
letter on the same keyword. 
Check_AllType_CustomeField_Exist became Check All Type Custome Field Exist - 
instead of Check AllType CustomeField Exist

Original comment by poornima...@gmail.com on 23 Jul 2014 at 5:42

GoogleCodeExporter commented 9 years ago
Ah, got it. I also overlooked your method naming earlier. I didn't notice that 
the underscore version wasn't fully Pythonic, still mixing in some camel case.

Do you have to follow this naming convention for the methods? Changing the 
wrapper naming format might fix the issue. Perhaps something like 

Check-All-Type-Custome-Field-Exist

which would make the actual keyword name appear just as that in the tests. Per 
the RF docs:

http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#key
word-names

you're likely going to be having issues mixing "_" with camel case. So you're 
going to have to introduce some other differentiator.

I started a discussion thread on RF developer Google Group to get ideas and how 
to proceed.

https://groups.google.com/forum/#!topic/robotframework-devel/ercic27vtT8

From the standpoint of the remote server, we just pass the method names of the 
library module as is to RF. It's up to RF on how to interpret them as keywords, 
and pass back the appropriate method name to execute to the remote server. So 
it may have to be a fix on RF side as a bug for RF. 

What we can do on the remote server side is add special logic to translate the 
method names (as needed) on the fly (when passing back to RF or translating the 
name received from RF back to native code method name) so that there is no 
confusion. Unfortunately, this type of thing I think is not usually generic 
enough to handle most/all cases, so I'm not sure there's anything I could add 
for special logic that would solve your problem that's also useful for other 
people. You are welcome to customize your version of the remote server as such 
to meet your needs.

If you do find a general case (and implementation) for dynamic method 
translation (e.g. renaming) that fixes your problem and would be useful for 
others, feel free to submit a patch for the server, here or on GitHub.

Original comment by manga...@gmail.com on 23 Jul 2014 at 6:26

GoogleCodeExporter commented 9 years ago
Missed out to add, this issue occurred when RF splits the keyword from the test 
cases. 
The library has 2 keywords "Check All Type Custome Field Exist" and "Check 
AllType CustomeField Exist". 
I have 2 servers having the same setup. When I run a test case with 
"Check_AllType_CustomeField_Exist".
On one server, it picks the keyword as "Remote.Check AllType CustomeField 
Exist" matching to the correct method, while on the other server, it picks it 
up as "Remote.Check All Type Custome Field Exist", matching to the wrong method.

Original comment by poornima...@gmail.com on 23 Jul 2014 at 6:53

GoogleCodeExporter commented 9 years ago
So on one server it always works but not the other one? Or it varies?

Original comment by manga...@gmail.com on 23 Jul 2014 at 6:56

GoogleCodeExporter commented 9 years ago
Yes, on one server it always works. No variations.

Original comment by poornima...@gmail.com on 24 Jul 2014 at 5:48

GoogleCodeExporter commented 9 years ago
That I find strange. Have you tested this issue with servers shut down and 
restarted and such? I just wonder if it has any relation to which server is 
started up first and/or which server is "imported" first in RF test that 
affects which one works and which one doesn't. If you have tested that, try 
swapping the server order in RF import in test case and/or try swapping order 
of which server is started up first.

I would assume order of server startup might not matter, but order of import 
might for this bug. Also do you use aliasing of the remote server libraries 
being imported? (e.g. the "with name" feature: 
http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#set
ting-custom-name-to-test-library)

Original comment by manga...@gmail.com on 24 Jul 2014 at 6:15

GoogleCodeExporter commented 9 years ago
I can assume/generalize how you start the servers but would you happen to have 
an example test case you can share that reproduces this issue? Be better I 
debug with the same "test" setup.

Original comment by manga...@gmail.com on 26 Jul 2014 at 1:37

GoogleCodeExporter commented 9 years ago
Well, I investigated the problem and I can't exactly reproduce your bug 
scenario without having test files that reproduce that exact scenario (e.g. 
Perl library and RF test case, etc.). So if you want to address that exact 
scenario, please provide generalized test files that you can release to the 
public that can still reproduce the issue.

What I did observe is that the behavior of what keyword gets selected by RF 
depends on the order the keywords are returned by get_keyword_names(). So 
modifying get_keyword_names may (but not necessarily will) give you a desired 
result. However, one may likely not be able to use both keywords in the 
library, only one (the first one returned by get_keyword_names). As such when 
running RF test in this scenario, from my testing, I believe regardless of what 
keyword you select in test, it will map to the first one returned by 
get_keyword_name().

You can confirm this by 2 actions:

* run libdoc (e.g. python -m robot.libdoc -f html --name MyLibrary 
Remote::http://10.0.0.42:8270 MyLibrary.htm) to generate HTML documentation for 
the Perl library and see what keywords you get (you'll get only 1 out of the 2 
keywords - the first one returned by get_keyword_names() )

* make an XML-RPC call to the Perl remote server running your library to call 
for get_keyword_names() to see the order the keywords are returned in. To do 
this call, simply make an HTTP POST to the remote server URL (host + port) with 
this POST data:

<?xml version='1.0' 
encoding='UTF-8'?><methodCall><methodName>get_keyword_names</methodName><params>
</params></methodCall>

the response will be in XML with the keyword list within that.

If you know other languages, you can compare behavior of other remote servers 
as well (Java, .NET, Python, etc.), which is what I did. And I documented the 
findings also on the RF developer group post in previous comment here.

Original comment by manga...@gmail.com on 1 Aug 2014 at 9:39

GoogleCodeExporter commented 9 years ago
Based on my findings, the RF user documentation about how keyword "names" are 
translated between library code and RF test cases, and the RF developer group 
discussion post

https://groups.google.com/forum/#!topic/robotframework-devel/ercic27vtT8

I present the following solutions to the problem:

* you modify the implementation of get_keyword_names() - make the code 
reflection work differently or don't use reflection but statically return fixed 
keyword names in your desired order, etc. This only solves half the problem if 
you still want to be able to use both keywords. This I won't do for you.

* change your Perl library wrapper method name. Don't mix underscores with 
CamelCase, use one or the other, or use them wisely together. Because RF treats 
both techniques as a whitespace, therefore, it may behave unexpected from what 
you intended. Perhaps use hyphen or some other character as I originally 
suggested. I recommend this approach, where possible for you if not constrained 
to fix naming convention.

* (I plan to) update the Perl remote server to check for (seemingly) duplicate 
keywords and complain when such exists. This is implemented in the Java remote 
server and JavaLibCore library package for those that use it to build Java 
libraries for RF. So it makes sense to copy that behavior. I did in fact 
implement a version of your described library in Java for the Java remote 
server and it did in fact complain about duplicate keywords. My sample RF test 
fails with the following test execution error that causes the rest of test to 
eventually fail as well:

Error in file '/Users/xxx/Documents/Temp/rfdbg/remote_tests_java.html': Calling 
dynamic method 'get_keyword_names' failed: Connecting remote server at 
http://localhost:8270/lib2 failed: <Fault 0: "Failed to invoke method 
get_keyword_names in class 
org.robotframework.remoteserver.servlet.ServerMethods: 
java.lang.RuntimeException: Two keywords with name 
'CheckAllTypeCustomeFieldExist' found!">

Using the older obsolete Java remote server (the one on Google Code, not 
GitHub) will not give an error but give you similar bug scenario as this Perl 
one.

* add option in Perl server to exclude certain types of methods as keywords. 
This will be more discretionary or arbitrary since Perl doesn't have concept of 
private/public methods 
(http://stackoverflow.com/questions/451521/how-do-i-make-private-functions-in-a-
perl-module). So we could arbitrarily say if methods begin with "_" or 
something else, don't put them in the list that is returned by 
get_keyword_names() so they are never called as keywords directly. I'll 
consider adding this feature to the server in a future release.

Original comment by manga...@gmail.com on 1 Aug 2014 at 9:54