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

Add a reasonable way to get DOM elements from page-objects #475

Open
NullVoxPopuli opened this issue Jan 7, 2020 · 21 comments
Open

Add a reasonable way to get DOM elements from page-objects #475

NullVoxPopuli opened this issue Jan 7, 2020 · 21 comments

Comments

@NullVoxPopuli
Copy link
Contributor

Supersedes: #473
As mentioned here: #467 (comment)

there are other APIs in progress, but they seem very verbose.

Ideally, from a testing perspective we wouldn't need any additional imports:

myPage.object.with.nested.things.dom

that dom getter would be present on all page objects, regardless of single or multiple, so that we have a single, consistent API for use with qunit-dom

thoughts?

/cc @ro0gr, @Turbo87

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

@NullVoxPopuli I believe, exposing an under-the-hood DOM element is a violation of the basic page object idea to abstract an implementation from the test. Typically when you change implemetation, you just want to update related page object, but all the related tests untouched, and that's one of the goals of the page objects.

As far as I can see, most of the qunit-dom helpers are tightly coupled to the dom(obviously), which makes them a bad choice to be integrated with page objects(imo).

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 11, 2020

then ember-cli-page-object needs assertions.: -\

as is, doing assert.equal(page.thing, 'value'), is super unhelpful if something goes wrong.
we need better error messages.

Like, expected the attribute [thing] to have value of 'value'

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

Yes, it does, but "qunit-dom" still doesn't seem to be a solution. Let's assume, you have:

const page = create({
  hasSomeState: hasClass('someState')
})

"qunit-dom" would not allow us to use this hasSomeState page object prop, and we would need to use hasClass from the "qunit-dom" to get some contextual message, which uncovers the same issue of exposing implementation details.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 11, 2020

then, it may be easiest, philosophically, to implement a:

assert.pageObject(page.hasSomeState)

so that way, we could have an error message like:

Expected [selector] to have class 'someState', but instead had [list of classes]

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

Exactly!

A while ago I've made a quick poc for this https://github.com/ro0gr/ember-qunit-page-object, and in general, this direction looks promising to me. Unfortunately I still don't have enough time to renew my efforts there, but we made a step on the that direction some time ago. In ec-page-object, we now have getPageObjectDefinition private API, which can be used to get of the original definition and build assertions API on the fly for each specific page object.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 11, 2020

I just found this: https://github.com/ro0gr/ember-qunit-page-object/blob/master/tests/smoke-test.js

all of that is very similar to qunit-dom, but requires all the assertions to be wrapped in an arrow function provided to .as() :-\

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

requires all the assertions to be wrapped in an arrow function provided to .as()

I don't think as should be required

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

I think it was just a single test, but you can find another tests w/o as there https://github.com/ro0gr/ember-qunit-page-object/blob/d17d2e60a37612acd99f86589f1ac500a7a0f473/tests/smoke-test.js#L63-L67

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 11, 2020

what about integrating with qunit-dom?

assert.dom could be altered to take a page object, it could read the getPageObjectDefinition on the page object property, and then we already have all the assertion work done for us? :D

assert.dom(page.object.subObject).hasText('some text');

I think this is doable if getPageObjectDefinition allows access to a selector (non-jquery selector) or element

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 11, 2020

I'd like to delegate that assertions api somewhere, but

if getPageObjectDefinition allows access to a selector (non-jquery selector) or element

I think it isn't achievable via a single CSS selector string. For example, there is no reliable way to get a nested collection element via single CSS selector. The best I can think of is some kind of query DSL object, which would contain nested scope and at pairs.

Also, I'm not sure if the knowledge about the page objects should be a "qunit-dom" responsibility 🤔

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jan 11, 2020

For example, there is no reliable way to get a nested collection element via single CSS selector.

that's all qunit-dom needs. the page object can provide the element or collection.

Also, I'm not sure if the knowledge about the page objects should be a "qunit-dom" responsibility 🤔

I think it's better than re-inventing qunit-dom for page objects? less to maintain overall, imo

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 12, 2020

I'd say, it gets inspiration from qunit-dom(pushResult() and Qunit.extend()), but it doesn't re-invent something.

FWIW, I think, you can achieve .dom for each page object by decorating create from the page object, so you can extend the base DSL.

@yratanov
Copy link
Contributor

@ro0gr what if under the hood of assert.po we just do smth like this

QUnit.assert.po(pageObject) {
  return this.dom(pageObject.element);
}

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 21, 2020

@yratanov I think this is a perfect answer to a question "how to integrate", with that we don't even need to expose element from the page objects:

QUnit.assert.po(pageObject) {
  return this.dom(findOne(pageObject));
}

However, in scale, integration of these 2 libs has a number of pitfalls, like:

  • inability to support custom properties
  • Leaking DOM knowledge to a test, which can lead to mixed styles even in a single test

@yratanov
Copy link
Contributor

@ro0gr yep, I think you are right. Can I be any help with https://github.com/ro0gr/ember-qunit-page-object?

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 21, 2020

@yratanov thanks! Glad to see you are interested. As far as I remember, there were some complexity in parsing page object instance shape(props) due to a lack of getPageObjectDefinition those days. Also some experimentation has to be done to figure out an effective way to build assertions(including negations, like hasSomething, not.hasSomething).

I'll try to renew my mind about this addon the next few days and create some tickets, in order to have some plan.

@ro0gr
Copy link
Collaborator

ro0gr commented Jan 21, 2020

@NullVoxPopuli what do you think? is the snippet, suggested by @yratanov, something you are looking for?

@NullVoxPopuli
Copy link
Contributor Author

maybe! would need to see it in action to really know what the implications are

@yratanov
Copy link
Contributor

yratanov commented Feb 15, 2020

@NullVoxPopuli @ro0gr I've created a small addon for it https://github.com/yratanov/ember-page-object-asserts . It's very simple, my main goal was to make errors more descriptive. API has something in common with qunit-dom.

const page = create({
  link: {
    scope: 'a',
    href: attribute('href'),
    isHighlighted: hasClass('highlighted'),
  },
  list: collection('li'),
  input: { scope: 'input' }
});

assert.po(page.link).hasText("test");
assert.po(page.link).hasNoText("bla");
assert.po(page.link).hasText(/test/);
assert.po(page.link).has('href', 'google.com');
assert.po(page.link).has('isHighlighted');
assert.po(page.input).hasValue('test');
assert.po(page.input).isPresent();
assert.po(page.input).isHidden();
assert.po(page.list).hasItems(2);

Please tell me what you think :)

It's my first addon, so I might be messed something :)

@yratanov
Copy link
Contributor

made few improvements to the addon, now you can do this:

const page = create({
  link: {
    scope: 'a',
    href: attribute('href'),
    isHighlighted: hasClass('highlighted'),
  }
});

assert.po(page.link).href('google.com');
assert.po(page.link).isHighlighted();

Directly calling props as methods :)

@ro0gr
Copy link
Collaborator

ro0gr commented Feb 16, 2020

@yratanov I like it!

Since this issue seems to be a bit unrelated to a custom assertions API, left some initial feedback at your addon repo yratanov/ember-page-object-asserts#1

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