When multiple elements are found by the locator and keyword interacts with single element, generate a warning in the logs

Now if there are multiple elements found by the locator, then SL silently takes the first elements. This is not good implementation and we should want users about multiple elements found by the locators. We should perhaps display warning in the console and in the log.html that multiple elements. are found by the locator. Making this change is too late for 3.0 release.

Display warning in release 3.2.0. Should fail in release 3.3.0 (remember to create separate issue about failure when warning is done)

If there are no objections, I am going to see if I can tackle this one :)

I was trying to think of why having multiple items would be bad, and i guess only in the case of multiple id=[value] because id's are suppose to be unique within the document.

It's mostly that it feels a bit random to just select the first element if there are multiple matches. And test automation should not be random. You should be able to construct a locator that matches on the desired element.

The above said, this change is backwards incompatible and may cause lot of tests to break. If that happens, we need to think is being pedantic that important. Practicality vs. purity once again.

I have fix proposal in #1150 but I would need feedback on few items

Page Should Contain keyword
Now I did made that Page Should (Not) Contain does not log warning because it finds often multiple elements (Because of the xpath it internal creates) and with this keyword user is not interested about elements. User is interested about text in the page.
Page Should Contain ... keywords
Now, should other Page Should Contain ... keywords log the warning or not. I am thinking that they should, but I can think in other direction too. The complicated part is the Page Should Contain Radio Button keyword, because html could look like this:

<input type="radio" name="sex" value="female" checked /> Female<br/>
<input type="radio" name="sex" value="male" /> Male

And it could be used like this: | Page Should Contain Radio Button | sex |, which would lead to warning. But should it, because radio button could be seen as a single element (although it is not) and user want s to verify that radio button group is there.

IMHO Contain keywords should generally work without warnings if there are multiple matches. For example, BuildIn Should Contain works that way. Implementation could be something like

elements = self.find_elements(...)
if not elements:
    raise AssertionError
self.info("%d element%s matched." % (...))

It is good idea, but I honestly do not know yet (mostly because JavaScript argument thing has taken all the available time). I can think of exception to that rule: Page Should Contain Element, because in that we are interested about element(s). And what about Page Should Contain Button. Perhaps before implementing anything more, I need list all the keywords and decide what will and what will not log the warning. Then it is more meaningful to have a discussion, when full picture is available.

In any case, regardless to which direction we go, I think documenting this thing will be painful. The solution for the documentation, is to simple list all the keywords which will and which one will not log an warning when multiple elements are found.

Also it might be good idea to require testing this feature on new keywords. Also that should documented too.

I don't see why Page Should Contain Element, Page Should Contain Button, or any other Contain keyword should fail or even emit a warning if there is more than one element, button, whatever. I prefer that approach in general, but now that we need to think about backwards compatibility failures or warnings would be especially problematic. Also, all/most of these keywords have limit=None argument that can be used to validate that there is a certain exact number of matches.

The actual problem this issue tries to address is that currently keywords doing something automatically select the first match. Just ignoring possible other matches is not good. I don't think keywords validating something (i.e. Should keywords) should be affected in general.

Originally I started to take on this ticket but then realized it wouldn't be good. I've been following all along with the discussion but what would we gain by emiting warnings? even if it is invalid HTML, that is still not the purpose of RF. I can't really see a good reason to do this :(

…

On Wed, Jul 4, 2018 at 3:34 PM Pekka Klärck ***@***.***> wrote: I don't see why Page Should Contain Element, Page Should Contain Button, or any other Contain keyword should fail or even emit a warning if there is more than one element, button, whatever. I prefer that approach in general, but now that we need to think about backwards compatibility failures or warnings would be especially problematic. Also, all/most of these keywords have limit=None argument that can be used to validate that there is a certain exact number of matches. The actual problem this issue tries to address is that currently keywords doing something automatically select the first match. Just ignoring possible other matches is not good. I don't think keywords validating something (i.e. Should keywords) should be affected in general. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#978 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAoXU9zCaMC4fcNerRVf6ySozrQiH-Sks5uDSbugaJpZM4QMqmX> .

-- Ruby: http://blog.rubygeek.com - http://www.twitter.com/rubygeek http://www.linkedin.com/in/nolastowe - my linkedin profile http://github.com/rubygeek - my code

@pekkaklarck

@pekkaklarck
So the keywords that do not interact with the element, should not log a warning, like the Page Should Contain... and Wait Until Page... keywords. But keywords that interact with the element, like Click Button and Get Element Attribute should log warning if multiple elements are found with the locator. Is that the idea in nutshell? If it's then I can see the reasoning in your idea. Just need to give it a some tough.

@rubygeek
The HTML is not invalid in the page. But the if the locator provided by the user matches to multiple elements, then the SeleniumLibrary will silently select the first element and performs the possible interact with the first element. And we are trying to solve part that instead being silent, the library would inform users about the decision it has made. The best idea so far has been to display a warning to users in the log.html file. But I am open to other ideas too and happy to change the implementation too.

I am sorry, that I did not have more time to spend with you when you started to implement this feature and you did implement the wrong thing. And it's not your fault at all, it's just different expectations what the feature should actually do. As you can see, I and Pekka have different expectations and we did actually discuss this face to face few times :-)

As I've commented earlier, the problem is that there are cases when a locator matches multiple elements but SL just throws other than the first one away. The place where it happens is in ElementFinder.find(). This feels pretty random for me and test automation shouldn't be random. It ought to always be possible to construct a locator that matches exactly the desired element. For example, instead of using //h1 you ought to be able to use //h1[1].

This mostly affects keywords interacting with elements. For example, if I say Click Link class:xxx and there are multiple links with class xxx, blindly clicking the first link sounds wrong. Notice that jQuery would click all links in this case, but I don't think that's a good idea with SL. Requiring users to give an exact locator sounds much better.

I was wrong above when I validation keywords wouldn't be affected in general. It's true that Contains keyword shouldn't be affected, but keywords like Element Text Should Be probably are. My guess is that at the moment they also only check the first element and ignore others. I can see many ways this could create false positive results.

The main problem with the change is that it is backwards incompatible and likely requires many users to updated their locators. Although updating should generally be pretty easy, there may be lot of locators to update making the change problematic. We definitely cannot make the situation error right a way but emitting a warning ought to be fine. If there a lot of complaints, the warning can then be removed.

Navigation Menu

Repository navigation

When multiple elements are found by the locator and keyword interacts with single element, generate a warning in the logs #978

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

When multiple elements are found by the locator and keyword interacts with single element, generate a warning in the logs #978

Description

Activity

rubygeek commented on Apr 12, 2018

rubygeek commented on Apr 13, 2018

pekkaklarck commented on Apr 13, 2018

7 remaining items

aaltat commented on Jun 30, 2018

pekkaklarck commented on Jul 3, 2018

aaltat commented on Jul 4, 2018

pekkaklarck commented on Jul 4, 2018

rubygeek commented on Jul 4, 2018

aaltat commented on Jul 5, 2018

pekkaklarck commented on Jul 5, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions