Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upWhen multiple elements are found by the locator and keyword interacts with single element, generate a warning in the logs #978
Comments
|
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. |
|
if i want multiple matches i use: `css=.feature-article` . if i want only
first one i can use `css=.feature-article:first-of-type` ... if i want the
second .feature-article i can use `css=.feature-article:nth-child(2)`
…On Fri, Apr 13, 2018 at 5:17 AM, Pekka Klärck ***@***.***> wrote:
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.
—
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/AAAoXVytaNDU__z_eRs4D5w7z6LohNQMks5toHtOgaJpZM4QMqmX>
.
--
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
|
|
Yes that is possible and there are many other ways too. Also users that are skilful enough, do know how to select the correct single element. But when user is new to UI and SeleniumLibrary automation this can be a problem for the users. Also UI may change and once unique selector can now point to multiple elements. In those cases it would be good to receive feedback. |
|
Exactly, there should always be a way to pinpoint a single element. The library automatically discarding other than the first element can lead to hard to diagnose problems. Another alternatively would be somehow acting on all found elements similarly as, for example, jQuery. I don't think that makes any sense in SL case, though. |
|
Why would/should the library fail when multiple elements are found as compared to giving a clear and informative warning? |
|
That was the initial idea and it sounded logical to me. When I think of it, I can see the failure would beneficial for the users that are not so familiar with browser UI automation. But I also see that more experienced user could be offended. After the warning is implemented and released, we should ask from the community what they would like to happen. |
|
I ask mostly because I am just trying to work out in my mind what I would expect the behavior to be or how the system should react in such a situation. As I recall the internal workings of the keywords where a locator is passed in if only one element is expected like |
|
one of the parameters is first_only ...so wonder if we can make another
function where that is passed in as False
https://github.com/rubygeek/SeleniumLibrary/blob/master/src/SeleniumLibrary/locators/elementfinder.py#L61
…On Wed, Apr 18, 2018 at 6:48 PM, Ed Manlove ***@***.***> wrote:
I ask mostly because I am just trying to work out in my mind what I would
expect the behavior to be or how the system should react in such a
situation. As I recall the internal workings of the keywords where a
locator is passed in if only one element is expected like Click Element
then I could an error when locator turns up multiple elements. My concern
would be areas where it is ambiguous as to whether or not multiple elements
is fine or places where we would, for whatever reason, need to understand
the locator is perfectly fine to have multiple elements.
I was also trying to think long term to see what are the possibilities for
how we would handle locators or elements and would the behavior box us into
limited solutions, or open us up to a better framework for using locators,
or may have no affect..
—
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/AAAoXWc3P4iM-n5Hd9kZCZEi2HBxWWPUks5tp9DKgaJpZM4QMqmX>
.
--
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
|
|
I have fix proposal in #1150 but I would need feedback on few items
And it could be used like this: |
|
IMHO 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: 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 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. |
|
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 @rubygeek 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 This mostly affects keywords interacting with elements. For example, if I say I was wrong above when I validation keywords wouldn't be affected in general. It's true that 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. |
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)