Add support for page object-formatted object-based selectors

[senocular]

(Partial implementation of the original #1116)

Adds support for optional, object-based selector values in commands using the format used by page elements. ex:

// before:
browser.getText('#foo', function(result){});

// now (optionally):
browser.getText({selector:'#foo', locateStrategy:'css selector'}, function(result){});

• This new implementation reduces complexity around page command wrapping. We no longer need to track and restore the locate strategy because its encoded directly within the selector; the callbacks are no longer wrapped. This should presumably fix (could use verification from submitters): Wrong error (allElements.shift not a function) showing in test summary #1142, Client is not defined properly when an assertion fail if the selector is defined in page objects #1107, Test randomly fails with Error while running waitForElementVisible command: allElements.shift is not a function error. #1100, Passing two page object element selectors as arguments in a custom command (error 'allElements.shift is not a function') #895
• As a result of the above, page objects can also use any form of supported locate strategy (https://groups.google.com/forum/#!topic/nightwatchjs/ZDuM7TX6fEA)
• It opens the selector format up to being more dynamic, such as having additional properties that could potentially further mutate the resulting selector - not unlike with what other PRs/issues (e.g. Page objects. Elements cannot have dynamic selectors.  #821) are doing/suggesting, trying to encode additional data in the selector string. This commit is not doing anything other than including selector and locateStrategy, but it opens up the possibility for additional properties in the future.
• Global locate strategies set with useCss() or useXpath() can be maintained when writing custom commands, for example, if the custom command uses strategies within the selectors for its requests, preventing interference with the global strategy (users won't have to restore their set strats upon using a custom command that uses other strats).

Also noticed this seems to be an exact implementation of #720.

[GBeauny]

@senocular This fix could be on the next release? Thanks Guys.
There is also this issue related: #903.

[beatfactor]

something happened to the tests, it seems to have crashed half way.

[senocular]

This has a dependency on #1155 and won't work until that's in

[beatfactor]

Ah yes. Would it be possible to merge the other PR into this one?

[senocular]

Yeah I can work on that

[senocular]

Merged with #1155 and fixed some tests. @drptbl, if you're still around and could give this a whirl, making sure nothing existing breaks and it works as you might expect, that would be great.

@beatfactor as with before, I can squash to one commit if/when it goes in.

[beatfactor]

Cool, I'll try to take another look here. Thanks.

[senocular]

There was an issue with error messaging when using selector objects in command calls. For example if you had something like:

    browser.getText({selecto:'div', locateStrategy:'css selector'}, function (result) {
        console.log('My div text is ', result.value);
    });

The error message would be:

Error while running getText command: No selector property for element "(anonymous)". Instead found properties: locateStrategy

The first problem being that its picking up the confusing "(anonymous)" text. But I think more importantly, you're losing the problem key. selector was misspelled as selecto and that was getting stripped because the object passed in wasn't the same object that was making it to the constructor where the error is getting generated.

With the latest commit, if Element.fromSelector() gets an object value, it keeps that value, passing it into the constructor so the full definition can be seen. Additionally the error message is smarter checking for a name, using the old classtype + name messaging when in a page object/section, and using a generic "selector object" text when used in a selector context. So now the error message is:

Error while running getText command: No selector property for selector object. Instead found properties: selecto, locateStrategy

[beatfactor]

Hey, sorry for the slow progress here. I think this looks good, but could you also add/update an example testcase with how will this be used? Also could you move all the selector tests inside a folder in src/api/element-selectors? Thanks.

[senocular]

...could you also add/update an example testcase with how will this be used?

@beatfactor Can you be more specific about this? Thanks.

[beatfactor]

I was thinking of updating one of the sample tests in the examples folder with showing how this can be used, but I think we can do that when the original #1116 is fully implemented?

[senocular]

@beatfactor yeah, as is, I don't think anyone would need to use this right now. It doesn't add much new other than a new syntax for the same inputs. With index support, I'll be sure to add an example.

[beatfactor]

Ok, can you squash the commits please? Then I will merge it to master. Thanks, and once again sorry for taking so long to review.

[senocular]

Done

[senocular]

Hold on, I think the tests didn't make the commit...

[senocular]

tests confirmed added: https://github.com/nightwatchjs/nightwatch/pull/1156/files#diff-3523a9a2061901f4ab5d9fba67c6da9e

And passing, so that is also a good sign

[laurensclaessen]

Hi, thanks for this feature! It's very useful!

@beatfactor, any thoughts on when this will be released in a tag?

[humphreyn]

@beatfactor Any idea when this is going to be released?