Assert selector matches single element? about ember-cli-page-object HOT 8 CLOSED

Hi @machty, thanks for your feedback.

I'm not sure if we should assert that exactly one element is matched or if we need to use only one of the matched elements. For some actions (i.e. clickOnText) we use the last matched element of the matched set.

So, I'm not sure if this is would be the desired behavior for all the attributes, maybe we should make this the default behavior for predicates and query attributes but not for actions.

So, .hasClass, .notHasClass, .isVisible, .isHidden, .attribute, .count, .text and .value attributes could raise an error when more than one element is matched.

What do you think?

from ember-cli-page-object.

Is it desirable for clickOnText to use the last matched element? That seems a bit like a footgun. If you're performing an action, isn't the expectation that you'd only be performing it on one element (and wouldn't you want it to error loudly if you unknowingly asked it to fire on multiple elements, forcing it to arbitrarily choose one of them)?

from ember-cli-page-object.

@san650 I would leave .count out of that list, we are using it to count how many elements of the selector passed as parameter exists so I guess it doesn't make a lot of sense to assert in that case.

@machty Regarding clickOnText, using the last matched element is an arbitrary decision, the thing is that there might me multiple elements with the same text to be clicked, several "Login" button for instance. I believe clickOnText should be used when there is no difference between clicking any element with the "Login" text. If I want to click a particular element, I would use .click and provide a unique selector, in that case it might be a good idea to assert it only matches one element.

What do you think?

from ember-cli-page-object.

I think clickOnText is a special case. We use the last matched element because we are looking for elements that contains a specific text. Let me try to explain it with an example.

Suppose that you have a modal with two buttons

<div class="modal">
  <div class="modal-body">
    <p>Lorem ipsum</p>

    <button>Accept</button> <button>Cancel</button>
  </div>
</div>

You could have a page object like

var modal = PageObject.build({
  click: clickOnText('.modal');
});

modal.click('Accept');

The selector we're generating for the modal.click call is .modal :contains(Accept):last. If we don't use :last we would be clicking the
div.modal-body instead of the button.

Does this make sense? It has been working great so far :)

from ember-cli-page-object.

@san650 I think I understand now: what you really want is to select the most specific element that contains the "Accept" and prune any other selected ancestor elements of that specific element, and :last just happens to be the quickest way to do that. The problem though is: what if you didn't realize you had two different "Accept" buttons on the page, perhaps one in the background and in one in a modal that you're current trying to test? Depending on how you insert modals into the page, your page object might be matching/clicking the first or the last "Accept". I think the correct thing to do here is raise an error that your Page Object isn't specific enough so that you don't wind up in a situation where you're not testing what you think you're testing.

Other examples that might show this behavior are if you have animations (e.g. Liquid Fire) in your app, and at the time that you assert the value of a Page Object selector, a previous view is still on the page and matches that Page Object, and you end up testing the wrong thing. Even though I usually end up disabling animations in test environments, if there's one I missed, I'd get very confusing feedback from multiply-matching Page Objects.

Unless you are asserting the state of multiple elements, it seems like any assertion / action that only makes sense when applied to a single object should yell loudly.

from ember-cli-page-object.

@machty Makes sense. I think we could tackle this issue in two steps. For some attributes the change is pretty straight forward (.text, .count, etc.) but for the case of .clickOnText we would need a more clever implementation.

I believe there's a ember addon which adds test helpers that have something similar to this clickOnText, I don't remember the name of the addon right now but I'll take a look at it later.

Right now we're working on refactoring the code to make it simpler, I hope to have this refactoring done this week, after that I can take a look at this.

If you want, you can contribute some of this changes by opening a PR.

Thanks for the ideas, I like the approach of "fail fast".

from ember-cli-page-object.

Let's target this work for v1.0 which won't be backward compatible.

from ember-cli-page-object.

Fixed in master

from ember-cli-page-object.