new methods to add

[grawk]

Barry Dutton, a developer at PayPal, has created some custom methods to use in his nemo test suite. I think they would translate well as new helper methods in nemo-view:

waitForNotPresent (add *WaitNotPresent)
waitForNotDisplayed (add *WaitNotVisible)
waitForTextExists (add *WaitTextExists)
waitForTextEqual (add *WaitTextEqual)
waitForTextNotEqual (add *WaitTextNotEqual)
waitForAttributeEqual (add *WaitAttributeEqual)
dragAndDrop (add *DragDrop(targetElement)) (removed "And" in an edit on 31July)

[aks-]

Hey, I'd love to try working on these, can I ask for clarifications?

[grawk]

That'd be great :)

There isn't a development guide for adding methods (there should be). But in the meantime, the high level would be:

1. clone nemo-view locally and make sure you are able to run the unit tests (npm test)
2. clone selenium-drivex and make sure you can run unit tests
3. add method(s) to selenium-drivex
   • note this PR as an example
4. consume those method(s) in nemo-view
   • note this PR which matches up with the selenium-drivex PR from above
5. construct appropriate unit tests for new methods
   • if necessary, request changes to the unit test app: http://warm-river-3624.herokuapp.com
6. from your own forks, submit PRs to selenium-drivex and nemo-view develop branches

Beyond that, let us know what help you need along the way.

[aks-]

aha, great. I will get back to it soon. Thanks for details and sure I will let you know in case I need more help :)

[aks-]

Hi again! Do you think waitForNotVisible is better name for waitForNotDisplayed?

[grawk]

Thanks a lot for asking. To be consistent with the other methods, we should stick with the names in parentheses above (alone below for clarity):

• *WaitNotPresent
• *WaitNotVisible
• *WaitTextExists
• *WaitTextEqual
• *WaitTextNotEqual
• *WaitAttributeEqual
• *DragDrop(targetElement)) (after some thought removed "And" from method name)

We want to remove any unnecessary words from method names since they are added onto the users locator names. So we want them to be as succinct as possible while still conveying meaning.

[aks-]

I also want to ask if I need to make a WIP PR, selenium-drivex doesn't have a develop branch(any other branch than master). Can you create one or I should make a PR against master?

[grawk]

Thanks for helping us find the deficiencies in our contribution model!!! Let me make a develop branch now..

[grawk]

Done!

[aks-]

Thank you for quick action. I would also love to know about few more bits. If there is any style guide? As I see somewhere there are line breaks when writing chaining methods and somewhere they go on same line. Besides that there are few space which are inconsistent. Also will we consider removing defer as it's old API and unsupported?

[grawk]

Regarding "style". I note that there is not a JSHint task in selenium-drivex. There should be. But for now, just do your best to match the existing style. I'll file an issue on the repo to resolve the lack of JSHint. Also there is no extensive style guide for nemo, but that's something we can address over time. In the meantime, just do what you consider to be appropriate. If/when there is a comprehensive style guide we can go back through the modules and clean up anything that doesn't match up.

Regarding "defer", I personally wasn't aware of the deprecation. If that's the case then we should file issues on all the modules to replace its usage. Do you have any further reference information on that which I can use as a basis?

[aks-]

Aha, okay. About 'defer' our code has explicit construction anti pattern http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it
So we should remove the anti pattern as it's library agnostic.

[grawk]

Based on that, there are still instances where using "defer" are acceptable (i.e. converting another API into a promise). But, where possible and applicable, yes we should make sure we are utilizing existing promises and not creating another. If you find such instances and have the time, please do file appropriate PRs. But let's try and keep PRs succinct in order to facilitate timely code reviews, merges and releases.

[grawk]

I'll take a look at your PR to selenium-drivex as soon as I can. It will need to be a little later on today though.

[aks-]

Sure I will try to make those changes regarding 'defer'. And please take your time, this PR is actually work in progress. So it take me time to add tests etc anyway :)

[aks-]

I believe adding dragDropToOffset(sourceElement, targetElement, x, y) will also be useful?

[grawk]

How about making x/y optional parameters to the dragDrop method?

[aks-]

Sure, will make it so

[grawk]

Actually, does it make sense to have targetElement AND x, y both?

[aks-]

Apparently, it doesn't make sense. You are right, It could be dragDrop(sourceElement, x, y). So do we need it anymore? Or we should just go with dragDrop(sourceElement, targetElement)?

[grawk]

I guess, unfortunately, we are discussing the selenium-drivex API here on this issue. We probably should discuss those details on the PR in question:
paypal/selenium-drivex#6

In terms of the nemo-view API, myLocatorDragDrop could be defined as:
myLocatorDragDrop(targetElement) and overridden as
myLocatorDragDrop(x, y)

[aks-]

Hmm :S
Will discuss selenium related details there. Thanks for informing :)

[gkushang]

👍 if would be helpful to have these methods. When can have these merged?