Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent behavior between elementWithin and someElementWithin #105

Closed
AlexeyRaga opened this issue Sep 5, 2013 · 10 comments
Closed

Comments

@AlexeyRaga
Copy link

elementWithin returns the first element that satisfies selector, while someElementWithin fails if there is more than one element can be found by the specified selector.

Expected behavior for someElementWithin:

When one element found -> Some element
When more than one element found -> Some (first element)
When no elements found -> None

@sergey-tihon
Copy link
Collaborator

I do not agree. When I expect one element it should throw an exception when there are more then one.

If you need first element you should use elementsWithin with fst function/pattern matching or design new methods firstElementWithin/headElementWithin, someFirstElementWithin/someHeadElementWithin

It is unexpected behavior for elementWithin =).

@AlexeyRaga
Copy link
Author

Following this logic elementWithin should throw too. But it doesn't. It just returns the first element found. Same for just element.

I cannot agree with you because to me selectors are not assertions. For example List.find does not throw if a list contains more than one element satisfying the predicate. However it will throw if there are no elements found.
So how searching for DOM elements is different to that matter?

If a part of my test is to assert that there is one and only one element for the specified selector, I will assert it explicitly. And it will be explicitly stated in my test, and everyone who read it would understand what is the assertion.
When I do element or elementWithin I don't care how many there are. Just like with List.find. But if I do care, it means that there is a meaning around it. And then I write specific code to express what's going on, and it may be fst or nth or whatever make sense. Perhaps I would even write a comment for why exactly I need fst or nth element.

Especially when we are talking about selecting "some" element. Even semantically it does not imply any failure. Juts give me "some" if you have, or "none" if you don't. Don't fail because you have a lot, it is illogical, just give me some :)

@sergey-tihon
Copy link
Collaborator

Canopy is not only a framework for testing but also for web automation.
I use it to script some scenarios, like data export from legacy systems that do not have an API for that.

For example, I want to login to the system, I need to find editbox for login and enter user name there. I am pretty sure that it should be one input element, otherwise my selector is wrong or something changed in system. In this case script should be stopped with error.

In this case, explicit check will double or even triple the size of scripts. Such behavior will add overhead in automation scripts.

@AlexeyRaga
Copy link
Author

Exactly. It is a testing framework and it is up to you how you write your test selectors.
What you say is incorrect exactly from testing perspective. What you test is that the element in your selector behaves in the specified way. Until it does what it should you don't care (and you shouldn't care) how the page is changed etc.
For example, if you test the "login" functionality and your designer decided to put an extra "sign in" button on the page (so you have one on the top and one of the bottom), your test must not fail. You will still find your span.sign-in and it still works.
Now, if you want to assert that there are two buttons, you do it explicitly.

It is about what you test. If you test that there should be only one element for the specified selectors, if that is your business logic - then yes, test it. Explicitly.

It is good to see that you now agree with me about consistency of find methods, some* semantics and selectors/assertions separation. The only objection left here is the size of your tests, let's deal with it.

First, I don't see how it triples size of your tests. Even if you really want to assert uniqueness every time you call element, elementWithin or someElement, then you just take your own advice and implement 3 one-liners: uniaueElement, uniqueElementWithin and someUniqueElement, and use them everywhere you want without doubling/tripling the size :)

It just should not be a default behavior because not everyone needs it, and not everywhere. I believe you also don't need it everywhere if you look at my example above :)

Yes, I could implement my own firstElement, etc. in the same way, but the difference here is in consistency.
Currently element and elementWithin do return the first element (which I think is fine), but someElementWithin would fail (and sometime it returns null, so it is buggy anyway). It is inconsistent (and semantically incorrect).

To make thing consistent either element, elementWithin (and maybe other selectors) should be changed to ensure uniqueness, or someElementWithin should be fixed.

I prefer the last, as it less destructive, introduces less breaking changes and - in my opinion - more logical.

But let's see what the author thinks and how he makes it :) I can live with both, as soon as everything is consistent.

@lefthandedgoat
Copy link
Owner

Interesting read =). Sergey is the author of elementWithin and someElement #73 so I don't want to break his use cases. I do agree though that they need to be consistent, and 2 of 3 work one way, and 1 works the other way.

Sergey would it be acceptable if I made all 3 not throw exception if there is > 1 element, but added a configuration that would make all 3 throw an exception for > 1 element. That way all 3 can be consistent with either style?

@sergey-tihon
Copy link
Collaborator

I agree that they need to be consistent. Also I want to have an option when they throw exception when >1 element.
Any suggestions how to do it in better way? New more clean naming maybe?

@lefthandedgoat
Copy link
Owner

I think I will just make a configuration handle it for now. We can always review/change if it does not work out.

@sergey-tihon
Copy link
Collaborator

I don't mind this option)

@lefthandedgoat
Copy link
Owner

This is now available on nuget, package 0.8.5
Change log wiki shows how to configure as well as an example in the tests in above pull commit.
Thanks for the feedback!

@lefthandedgoat
Copy link
Owner

Closing, please let me know if you would like any more similar changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants