robotframework / OldSeleniumLibrary

Deprecated Selenium library for Robot Framework
Apache License 2.0
13 stars 3 forks source link

New `Xpath Should Match X Times` keyword #136

Closed spooning closed 9 years ago

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 18 Sep 2010

I wrote this keyword today to test something:

There should be exactly one result Call Selenium API verifyXpathCount id('ires')//li[@⁠class="g"] 1

It would be nice to have a Selenium Library keyword like "Page Should Contain Number Of Elements" that would allow me to do:

Page Should Contain Number Of Elements id('ires')//li[@⁠class="g"] 1

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 18 Sep 2010

There's already Get Matching Xpath Count that seems to do exactly this: http://robotframework-seleniumlibrary.googlecode.com/hg/doc/SeleniumLibrary.html?r=2.4#Get%20Matching%20Xpath%20Count

Should it possibly be linked from some other keywords to make it easier to find?

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 18 Sep 2010

Get Matching Xpath Count is nice, I didn't see this in the documentation. However, it does only one of the two steps I would like to perform: it gets the xpath count for the provided xpath. It doesn't verify its value though. So, the new keyword might look like this, in text form:

Page Should Contain Number Of Elements [Arguments] ${xpath} ${number} ${_number} = Get Matching Xpath Count ${xpath} Should Be Equal As Integers ${_number} ${number}

I've just tested this and it works just fine. I don't know yet how to express this in Python, but I'll try to propose a patch ;)

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 18 Sep 2010

This is so easy to implement as a user keyword that I'm not entirely sure is a library keyword needed. I'm not really against adding it, though, so if you Robert think it's useful, please go ahead and implement it. Implementation should be pretty trivial but it should nevertheless get an acceptance tests. Can you discuss with Andreas how to create them?

Finally, I think the keyword should be named Xpath Should Match X Times to match Get Matching Xpath Count and BuiltIn.Should Contain X Times. The Element Something keywords also handle the locator differently.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 18 Sep 2010

I've quickly implemented a method "xpath_should_match_x_times" in assertion.py and added an acceptance test, please see the attached patch for details. Maybe the keyword should be called ".. match n times" to make it more clear that n means a natural number.

btw, can you tell me how to generate the documentation?

Cheers Robert

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 19 Sep 2010

The patch looks good. Few code style comments, documentation enhancement ideas, and other nitpicking below:

1) Use 'under_score' format instead of 'camelCase' also in variable names. We generally try to follow PEP 8 so you may want to study it too: http://www.python.org/dev/peps/pep-0008/

2) No empty lines inside methods. If a method is too long to be understood without them -- this one isn't -- it should be split.

3) Use 'a != b' instead of 'not a == b`. (And similarly 'a is not b' instead of 'not a is b' if you ever need identity comparison.)

4) The expected and actual counts should probably be converted to same format (either str or int) before comparison. get_xpath_count apparently returns the count as a string, so if the actual value is given as a string the comparison works. If the actual value is given as an integer, comparison most likely fails. You could add this line to the current test:

Xpath Should Match X Times //input[@⁠type="text"] ${1}

In this case I wouldn't care about possible conversion errors i.e. wouldn't do the conversion in try/except.

5) If you change 'given xpath' -> 'given xpath' in the doc, the argument will get special formatting in the generated doc.

6) This keyword and 'Get Matching Xpath Count' should be cross-linked. This is easily done by mentioning the other keyword in the doc so that its name is in backticks like 'See also The Other Keyword'. In this case something like below could be added to the new keyword:

Use Get Matching Xpath Count keyword if you just want to get the count.

See the documentation of the libdoc.py tool for more information about cross-linking and formatting in general.

7) Add a note to the doc that this is a new keyword in 2.4.1 version. We haven't actually discussed what is the next version, but forgetting to update this note if we go directly to 2.5 isn't that bad.

8) Re-generate the documentation. This way you can verify that the new doc looks good and cross-linking works. You can also commit the generated doc in separately. We typically use simple message 'regen' with that kind of commits.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 19 Sep 2010

Thanks for the feedback! 1-3 and 5-7 are already done.

About 4: I was thinking about converting the expected_xpath_count to a String, but I didn't find a good way to do it. Can I use utils.unic(...) from the Robot Framework? Apart from the xpath code which imports utils, the Selenium Library code looks pretty independent from RF so I didn't dare to use an import from there without asking beforehand. ;)

About 8: how do I generate the docs? For RF, I used the utility scripts back when I worked on the docs, but I have no clue about generating the SeLib documentation.

Have a nice Sunday,

Cheers Robert

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 19 Sep 2010

Okay, I've figured out how to use libdoc. Couldn't be much easier =)

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 20 Sep 2010

No need to use utils.unic here. It's main purposes are handling a situation where the given string has invalid encodings gracefully and workaround some Jython bugs. In normal usage you are fine with unicode or str, and in this case the latter is definitely enough.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 21 Sep 2010

Thanks - here's a new patch for review!

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 22 Sep 2010

Looks fine. You could have actually already committed this in after we agreed that adding the keyword is a good idea. Google Code has really good review possibilities and it's thus easier to comment committed code than patches.

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 22 Sep 2010

One more comment: You should add a link pointing to this new keyword to the old Get Matching Xpath Count keyword.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 23 Sep 2010

This issue was closed by revision d8e0ff2b8b.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 23 Sep 2010

I've just pushed the changesets for this issue and issue 137 .

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 23 Sep 2010

I didn't look at the final commits too closely but the original patches looked good. Changing status to Done, which we use instead of Fixed that is default on Google Code. Done works well with all issues and not just bugs.

spooning commented 9 years ago

Originally submitted to Google Code by chetan.dewangan on 3 Nov 2011

In my script I am tring to use "Get Matching Xpath Count" but it is fatching 0 count, though the same xpath identified with "is_element_present" from "Call selenium api".

I tried xPath for form,tr and div

Whereas the //input is working fine and giving expected value.

spooning commented 9 years ago

Originally submitted to Google Code by asanke.g... on 28 Mar 2014

Why is this method only take the expath starting from "//"?

Ex: Xpath Should Match X Times //input[@⁠type="text"] ${1}

Whereas other functions support xpths to be provided starting with "//" and "xpath=//".

Ex:Element Should Be Visible

Either: ${PATH} //input[@⁠type="text"] Element Should Be Visible ${PATH}

Or: ${PATH} xpath=//input[@⁠type="text"] Element Should Be Visible ${PATH}