Enhanced support for page object model #414

sknopf · 2015-04-12T22:05:12Z

This adds support for enhanced page object model. It doesn't yet include tests or updated documentation, as I wanted to first get feedback.

A couple notable features:

Supports sections (i.e., nested page objects)
Supports mixin functions used to encapsulate logic that doesn't fit the normal element/selector paradigm
Easy to share elements across page objects or sections
Simple API that mirrors the one already in Nightwatch

We've been using this for a few weeks internally at Twitter now and it's been working great. Though I'm sure there are some kinks to still get sorted out - one I am aware of supporting custom client commands and assertions whose first argument is not a selector.

@beatfactor let me know what you think.

beatfactor · 2015-04-13T07:54:02Z

Quite a large feature. If you don't mind could you explain the differences between this and the existing implementation?

sknopf · 2015-04-13T15:47:45Z

Definitely - the existing feature allows for a page object that is more like a custom command except with special namespacing under client.api.page. This feature adds to that by allowing you to abstract the page, making it more similar to other page object libraries. The goal is to make for more readable and maintainable tests.

For comparison, here are the existing page object demo tests rewritten with the new feature:
https://github.com/beatfactor/nightwatch/blob/master/examples/pages/google.js
https://github.com/beatfactor/nightwatch/blob/master/examples/tests/google.js

tests/google.js:

module.exports = {
  tags: ['google'],
  'Demo test Google' : function (client) {
    var google = client.page.google();
    google.goTo() // if a url is provided on page object, will navigate there by default
      .assert.title('Google')
      .assert.visible('searchBar') // selectors live in page object, not hardcoded in tests
      .setValue('searchBar', 'nightwatch')
      .submit() // logic of waiting, clicking, then pause is encapsulated in page object
      .assert.containsText('results', 'Night Watch');

    client.end();
  }
};

pages/google.js:

var googleMixins = {
  submit: function() {
    this.waitForElementVisible('submitButton', 1000)
      .click('submitButton');
    this.api.pause(1000);
    return this; // For chaining to page object
  }
};

module.exports = {
  url: 'http://google.com',
  elements: {
    searchBar: { selector: 'input[type=text]' },
    submitButton: { selector: 'button[name=btnG]' },
    results: { selector: '#main' }
  },
  mixins: [googleMixins]
};

Key differences are:

Selectors are not harcoded in tests - they are defined together under elements property of the page object
Any special logic (waiting for element to be present, then click, then wait for something else) can be abstracted out of the test code and into the page object. This makes the test much more readable and enables sharing across tests. This is done via the mixins property.
To allow for chaining, page objects return the page object, rather than client.api
Provides convenience methods like goTo

For other features not illustrated in this example, check out the examples in the PR. There I use a more complicated site IMDB to better demonstrate some cool features:

Sections:
This allows you to model sections on a page (or sections on multiple pages). Sections are namespaced under the page (page.imdb.section.news). Selectors of elements under the section become nested with its parent (e.g., '.newsSection .search'). They can also be nested under other sections, if necessary.

Ability to share common selectors and functions across page objects:
Often selectors and a series of commands are shared across multiple pages. The new page object framework makes sharing across page objects and sections trivial with mixins and sharing element components. This is demonstrated in the imdbSearchResults page (see PR).

For reference, here's another page object library (written in Ruby) that I think does a nice job at providing a simple interface but also supports more advanced page object needs like sections:
https://github.com/natritmeyer/site_prism

Note also this feature is behind a feature flag use_nightwatch_page_object specified in the config file for backwards compatibility.

beatfactor · 2015-04-13T18:45:28Z

Thanks, I'll look into it.

sknopf · 2015-04-17T02:00:18Z

Hey @beatfactor - have you had a chance to take a look? What do you think?

beatfactor · 2015-04-17T06:12:14Z

I had a brief look but haven't reviewed everything yet. One question would
be I guess, why do you think it should be behind a toggle? Isn't compatible
with the existing implementation?

On Friday, April 17, 2015, Stephanie Madison [email protected]
wrote:

Hey @beatfactor https://github.com/beatfactor - have you had a chance
to take a look? What do you think?

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-93877665.

maxgalbu · 2015-04-17T08:13:15Z

It's not compatible probably because of this feature:

To allow for chaining, page objects return the page object, rather than client.api

But, isn't this a little bit confusing? nightwatch already has a page object feature, a use_new_pageobject flag could be better :)

sknopf · 2015-04-17T14:37:38Z

I put this behind a toggle more as a safeguard to ensure no backward compatibility issues. Here [1] we add methods previously just on client.api (element commands, assertions, etc) to the page object instance only if the toggle is set. Otherwise if someone happens to have a method called click using the current page object feature, this feature will break their code.

[1] https://github.com/sknopf/nightwatch/blob/add_support_for_page_object_model/lib/core/api.js#L376

sknopf · 2015-04-17T16:13:25Z

Oh and we can definitely change the name to @maxgalbu's point. I'm not sure I like the term "new" but what about use_enhanced_page_object or something like that?

Also @beatfactor I'm aware of some style inconsistencies since I posted this - I was going to wait to fix those until you've decided if you want to move forward with this but LMK if you want me to address those sooner.

beatfactor · 2015-04-17T16:24:00Z

I'm thinking if there's any way to merge this feature into the existing
implementation and make it backwards compatible too. That would be my
preference. Maybe we can detect if the page is written for the new style or
the existing one somehow? I've done something similar for the new
assertions style which it was introduced in 0.4.

On Friday, April 17, 2015, Stephanie Madison [email protected]
wrote:

Oh and we can definitely change the name to @maxgalbu
https://github.com/maxgalbu's point. I'm not sure I like the term "new"
but what about use_enhanced_page_object or something like that?

Also @beatfactor https://github.com/beatfactor I'm aware of some style
inconsistencies since I posted this - I was going to wait to fix those
until you've decided if you want to move forward with this but LMK if you
want me to address those sooner.

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-94018486.

sknopf · 2015-04-17T16:35:19Z

OK, I can think of a few ways offhand but not sure they are ideal. I'll check out what you did for assertions, thanks for the tip.

sknopf · 2015-04-17T21:55:57Z

I ended up adding the condition:
typeof pageFnOrObject == 'object' && (pageFnOrObject.elements || pageFnOrObject.sections)

It may actually be enough to just check that it's an object as the nightwatch documentation says that the page object should be a (non-instantiated) class. But I threw in the check for elements or sections attributes to be safe.

See https://github.com/beatfactor/nightwatch/pull/414

sknopf · 2015-04-27T18:38:56Z

Hey @beatfactor since this is such a big change I thought it would be helpful to draft what the documentation would look like after this change, so you can see better how it gets used:

nightwatchjs/nightwatch-docs#4

I'll start working on tests for this as well. Would definitely love to hear any feedback you have like if you think this is something you would consider adding to Nightwatch.

sknopf · 2015-04-29T17:13:25Z

Added tests. I also rebased as my branch was stale.

sknopf · 2015-04-29T22:04:40Z

In the latest I added jsdoc markup and also a small refactor to loadClientCommands so that only actual element commands are loaded onto the page object (not client commands like end, etc). The refactor is based on feedback I got internally from my company who have been using the forked version this PR is based on (dogfooding! 👍).

With the additional documentation + tests, I'm definitely happy with the feature as it stands.. If there's anything else I can do to aid your review please let me know.

beatfactor · 2015-04-30T06:45:54Z

Sounds great, I'll integrate this soon.

On Thu, Apr 30, 2015 at 12:04 AM, Stephanie Madison <
[email protected]> wrote:

In the latest I added jsdoc markup and also a small refactor to
loadClientCommands so that only actual element commands are loaded onto
the page object (not client commands like end, etc). The refactor is
based on feedback I got internally from my company who have been using the
forked version this PR is based on (dogfooding! [image: 👍]).

With the additional documentation + tests, I'm definitely happy with the
feature as it stands.. If there's anything else I can do to aid your review
please let me know.

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-97601505.

sknopf · 2015-04-30T16:36:07Z

Great news! With this latest I made a change to accommodate calling assert module assertions on page objects. LMK if you want me to squash commits before merging in..

beatfactor · 2015-04-30T16:44:36Z

Yeah, that would be nice

On Thursday, April 30, 2015, Stephanie Madison [email protected]
wrote:

Great news! With this latest I made a change to accommodate calling assert
module assertions on page objects. LMK if you want me to squash commits
before merging in..

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-97871829.

sknopf · 2015-04-30T16:48:04Z

Cool all set

beatfactor · 2015-05-07T14:24:10Z

@sknopf would it be ok if I merge this in the next major version, i.e. 0.7? I'm currently working on an another major new feature (new assertion library) and I think it would be nice to also include this as a headliner. Besides, this is a large enough enhancement to increase the version number.

sknopf · 2015-05-07T14:44:04Z

Yeah definitely. I look forward to the new assertion library..

Also FYI I've been thinking about an improvement that would replace some logic here [1] where you can actually nest elements inside a section instead of combining css selectors of an element and its parent section. That would allow the user to combine xpath and css which they can't do currently. As I understand the elementIdElement endpoint [2] it seems like that should be doable but I still need to test and implement it.

If I can get that working do you want me to add the diff here or wait for a later release?

beatfactor · 2015-05-07T14:46:11Z

Yeah, that sounds like a nice improvement.
On Thu 7 May 2015 at 16:44 Stephanie Madison [email protected]
wrote:

Yeah definitely. I look forward to the new assertion library..

Also FYI I've been thinking about an improvement that would replace some
logic here [1] where you can actually nest elements inside a section
instead of combining css selectors of an element and its parent section.
That would allow the user to combine xpath and css which they can't do
currently. As I understand the elementIdElement endpoint [2] it seems like
that should be doable but I still need to test and implement it.

If I can get that working do you want me to add the diff here or wait for
a later release?

https://github.com/sknopf/nightwatch/blob/add_support_for_page_object_model/lib/page-object/page-utils.js#L12
2)
https://code.google.com/p/selenium/wiki/JsonWireProtocol#/session/:sessionId/element/:id/element

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-99893626.

sknopf · 2015-05-13T21:49:15Z

@beatfactor I got the nested elements working in this last commit so you can now combine xpath and css in sections.

One thing to note - I create some new protocol actions as well as a useRecurse function. As it is, these are exposed on the client - though maybe they're better off to keep internal to Nightwatch. Anyway let me know your thoughts.

beatfactor · 2015-05-15T22:04:10Z

@sknopf if you want to have a look at the new assertion library it's in the features/chai-expect branch. To see an example, refer to google.js example.

sknopf · 2015-05-16T18:33:50Z

The new library is so cool! It's also really nice that it eliminates the need to write a lot of custom assertions that are variants of more common assertions (with negation, waiting, etc).

FYI some small things I noticed running the google.js test:

If you change #lst-ib to input[name=q] it'll pass cross-browser (I saw failures in PhantomJS with the original)
My US google doesn't have 'Norge' as its text for the DOM element #hplogo though if I explicitly navigate to the .no TLD it works - we could change that URL it navigates to so it passes outside Norwary
Maybe add a friendly error message if an assertion fails because the element is not present (ie if this.elementResult is null. Otherwise you get a stacktrace like [1] below. To reproduce, call .to.have.css on an element that is not present
fyi I get double "element not found" in the failed assertion: Expected element <#lst-ib> to be enabled - element was not found - element was not found - Expected "enabled" but got: "not found". To reproduce, call that line on an element that is not present.

What's the best way for me to incorporate this into the page object? Should I merge it in and update the PR? It looks like it should be pretty simple to add this new library on page objects.

Also I was thinking since the common test example is Google I should change the IMDB example I have for page object to Google. I will work on that as well.

[1] An error occurred while running the tests:
TypeError: Cannot read property 'ELEMENT' of null
at CssAssertion.executeCommand (/Users/sknopf/workspace/nightwatch/lib/api/expect/css.js:19:56)
at CssAssertion.BaseAssertion.onPromiseResolved (/Users/sknopf/workspace/nightwatch/lib/api/expect/_baseAssertion.js:71:8)
at CssAssertion.retryCommand (/Users/sknopf/workspace/nightwatch/lib/api/expect/css.js:48:8)
at Timer.listOnTimeout (timers.js:110:15)

beatfactor · 2015-05-16T19:38:14Z

Glad you like it. Yeah, I guess that will work for updating the page object branch. I'll look at that problem with the css assertion and update the google example soon. I also created a pull request if you have more comments you can leave them there: https://github.com/beatfactor/nightwatch/pull/456.

…Elements Selenium protocols

…model

Remove old Google page object example test

Add expect.section to page objects and sections which is an alias for expect.element

sknopf · 2015-05-20T00:50:30Z

@beatfactor just pushed the latest which merges in the Chai assertion branch and gets it working with page objects. I also swapped out the IMDB demo test for a Google test.

beatfactor · 2015-05-20T19:37:32Z

Great, but jshint is failing.

sknopf · 2015-05-20T20:29:51Z

Hm, I think jshint doesn't like the chai assertion syntax: Expected an assignment or function call and instead saw an expression.

Fails on assertions like client.expect.element('#lst-ib').to.be.enabled;

beatfactor · 2015-05-20T21:52:54Z

From what I saw it was something about a semicolon, but I only had a brief
look.

On Wednesday, May 20, 2015, Stephanie Madison [email protected]
wrote:

Hm, I think jshint doesn't like the chai assertion syntax: Expected an
assignment or function call and instead saw an expression.

Fails on assertions like client.expect.element('#lst-ib').to.be.enabled;

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-104026985
.

sknopf · 2015-05-20T21:55:59Z

Ah yes I fixed that - you will see though in the latest travis build it's
complaining about some tests.

On Wed, May 20, 2015 at 5:52 PM, Andrei Rusu [email protected]
wrote:

From what I saw it was something about a semicolon, but I only had a brief
look.

On Wednesday, May 20, 2015, Stephanie Madison [email protected]
wrote:

Hm, I think jshint doesn't like the chai assertion syntax: Expected an
assignment or function call and instead saw an expression.

Fails on assertions like client.expect.element('#lst-ib').to.be.enabled;

—
Reply to this email directly or view it on GitHub
<
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-104026985>

.

—
Reply to this email directly or view it on GitHub
https://github.com/beatfactor/nightwatch/pull/414#issuecomment-104049618
.

sknopf referenced this pull request in sknopf/nightwatch-docs Apr 27, 2015

Update documentation for using page objects.

2b344e8

See https://github.com/beatfactor/nightwatch/pull/414

sknopf mentioned this pull request Apr 27, 2015

Update documentation for using page objects. nightwatchjs/nightwatch-docs#4

Merged

sknopf force-pushed the add_support_for_page_object_model branch from c63b654 to 09127f9 Compare April 29, 2015 17:11

Added page object support with examples and tests

0f0cb18

sknopf force-pushed the add_support_for_page_object_model branch from 6d6c5bf to 0f0cb18 Compare April 30, 2015 16:47

sknopf changed the title ~~Add support for page object model~~ Enhanced support for page object model May 1, 2015

Use nested elements for sections using elementIdElement and elementId…

7714456

…Elements Selenium protocols

sknopf force-pushed the add_support_for_page_object_model branch from 9b79629 to 7714456 Compare May 17, 2015 23:47

Stephanie Madison added 3 commits May 17, 2015 20:07

Merge branch 'features/chai-expect' into add_support_for_page_object_…

e2ab233

…model

Swap page object demo test from IMDB to Google

9c47d27

Remove old Google page object example test

Page objects working with Chai assertion library

9fec831

Add expect.section to page objects and sections which is an alias for expect.element

jshint

57e2648

beatfactor merged commit 57e2648 into nightwatchjs:master Jun 21, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced support for page object model #414

Enhanced support for page object model #414

sknopf commented Apr 12, 2015

beatfactor commented Apr 13, 2015

sknopf commented Apr 13, 2015

beatfactor commented Apr 13, 2015

sknopf commented Apr 17, 2015

beatfactor commented Apr 17, 2015

maxgalbu commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 17, 2015

beatfactor commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 27, 2015

sknopf commented Apr 29, 2015

sknopf commented Apr 29, 2015

beatfactor commented Apr 30, 2015

sknopf commented Apr 30, 2015

beatfactor commented Apr 30, 2015

sknopf commented Apr 30, 2015

beatfactor commented May 7, 2015

sknopf commented May 7, 2015

beatfactor commented May 7, 2015

sknopf commented May 13, 2015

beatfactor commented May 15, 2015

sknopf commented May 16, 2015

beatfactor commented May 16, 2015

sknopf commented May 20, 2015

beatfactor commented May 20, 2015

sknopf commented May 20, 2015

beatfactor commented May 20, 2015

sknopf commented May 20, 2015

Enhanced support for page object model #414

Enhanced support for page object model #414

Conversation

sknopf commented Apr 12, 2015

beatfactor commented Apr 13, 2015

sknopf commented Apr 13, 2015

beatfactor commented Apr 13, 2015

sknopf commented Apr 17, 2015

beatfactor commented Apr 17, 2015

maxgalbu commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 17, 2015

beatfactor commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 17, 2015

sknopf commented Apr 27, 2015

sknopf commented Apr 29, 2015

sknopf commented Apr 29, 2015

beatfactor commented Apr 30, 2015

sknopf commented Apr 30, 2015

beatfactor commented Apr 30, 2015

sknopf commented Apr 30, 2015

beatfactor commented May 7, 2015

sknopf commented May 7, 2015

beatfactor commented May 7, 2015

sknopf commented May 13, 2015

beatfactor commented May 15, 2015

sknopf commented May 16, 2015

beatfactor commented May 16, 2015

sknopf commented May 20, 2015

beatfactor commented May 20, 2015

sknopf commented May 20, 2015

beatfactor commented May 20, 2015

sknopf commented May 20, 2015