andywer / drag-mock

Trigger HTML5 drag & drop events for testing
MIT License
35 stars 11 forks source link

Added support for xpath search element strategy in webdriverBridge #10

Closed shkaper closed 7 years ago

shkaper commented 7 years ago

Hi there! Enjoying the library so far, webdriver.io integration in particular. However, would be cool to have the xPath selector strategy supported.

andywer commented 7 years ago

Hey @shkaper! Nice to hear that it's still of use and thanks for your PR :)

XPath support is a useful gimmick, but I have some suggestions about the PR.

I'd like to use XPath queries in a rather explicit way. What do you think about something like that?

const css = dragMock.selectors.css
const xpath = dragMock.selectors.xpath

webdriver
  .dragStart(xpath('/some/xpath'))
  .dragOver(css('.some-element'))
  .drop('.some-other-element')    // defaults to using `dragMock.selectors.css`

Maybe css/xpath returning an object like { type: 'xpath', query: '...' } or something like that. I would even favor making that an instance of some XPathQuery, that has a function apply (Document) => HTMLElement on the prototype, but that won't work with the serialization of the code, I guess...

shkaper commented 7 years ago

@andywer , thanks for the quick reply!

  • Not sure if the heuristic deciding if it's a CSS or XPath selector could cause trouble, esp. since it's an implicit behavior you might not expect as a user

I'm all for being explicit, in most cases. However, as a user of webdriver.io myself, I could argue that this implicitness is exactly what users would expect from a webdriver.io-compatible method. In contrast to e.g. webdriverJS (driver.findElement(By.id('myId'))), webdriver.io expects a plain string (driver.element('#myId')) and defines querying strategy itself. Here's an example of how making users do otherwise could be cumbersome. In our team, we write cucumber-style tests wrapping some business logic into parametrized gherkin step definitions, e.g.

this.When(/^I click "([^"]*)"$/, function (element, next) {
  const query = this.page[element]
  driver
    .waitFor(query)
    .click(query)
    .call(next)
})

So, in this case, we don't even know if the query is css or xpath. To preserve the flexibility of these kinds of steps, while also using drag-mock, we'll need to implement the same strategy-defining heuristics on our end.

  • The feature should be mentioned somewhere in the README

Sure, I'll add some info about it.

andywer commented 7 years ago

Good point! Didn't recognize that webdriver supports it like that in the first place...

Then let's stick with the approach, but I'd like to modify the code a bit. Doesn't seem too elegant yet (reading it the first time was quite hard). If you would like to change it, go on, otherwise I will change it if you don't mind :)

shkaper commented 7 years ago

I've cleaned up somewhat, that's about as elegant as I can make it :) Feel free to change any parts you don't like. Thanks!

andywer commented 7 years ago

Yeah, the code now looks way friendlier :)

Unfortunately it is quite hard to unit-test this and I never bothered putting much effort into it. Did you test it, does it work for you without problems? (God, this is bad style! 🙈😅)

shkaper commented 7 years ago

Yeah, sure, it works just fine for me. Would it make sense to add some tests for the webdriver integration? This would mean adding stuff like phantomjs/http server/webdriver.io to the dependencies of course

andywer commented 7 years ago

@shkaper Feel free if you want to do it, but since I don't actively use it (wrote it for a project I am not working on anymore) I guess I won't 🙃😉

andywer commented 7 years ago

Published as v1.4.0

shkaper commented 7 years ago

Ok then, I'll look into adding the tests as well. Thanks for accepting PR!