-
Notifications
You must be signed in to change notification settings - Fork 612
WIP: Feature/webdriver support #409
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
Conversation
Feature/webdriver support
garris
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start! I only had time to skim -- but I am very excited about this feature idea.
I think it would help to bear in mind that the current code line is still evolving -- so to make this integration smooth you'll need to rebase from @beta a lot.
From a product standpoint I would want to look at keeping as much feature parity as possible between versions.
| - **`scenarios[n].url`** – Required. Tells BackstopJS what endpoint/document you want to test. This can be an absolute URL or local to your current working directory. | ||
| - **`scenarios[n].selectors`** – An array of CSS selector strings enabling you specify what part of your DOM you want to test. The default value is `document`, which will attempt to capture your entire layout. | ||
|
|
||
| - **`engine`** - by default is "phantomjs" to use webdriver set it to "chrome" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't the option be 'webdriver' or 'webdriver:browserArg' here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, makes sence.
|
|
||
| scenario.selectors.forEach(function (selector, i) { | ||
| let cleanedSelectorName = selector.replace(/[^a-z0-9_\-]/gi, ''); // remove anything that's not a letter or a number | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions like these should probably be broken out into separate util files with tests to make this refactor cleaner.
|
@uglow I have tested this on ubuntu and OSX, this worked fine. No idea about windows. |
|
@fincha tried it on windows, but the selenium/webdriver didn´t start - i only saw and then the process ended |
|
I'd love to see this feature completed – is anyone working on it right now? |
|
I'd love to see this too. I don't think it's being worked on now. The codebase has changed a bit since this PR -- if someone can rebase against master and validate that it still works on osx we should cut a branch for it. |
|
I had to stop working on this for now, the company I was working with chosen different way... so sorry guys, maybe some one can pick this up. |
|
Just wanted chime in support for this. I’m investigating alternatives to Wraith and I’m very happy to see that not only does BackstopJS have first-class support for Docker, but that also some desire to move toward leveraging Selenium/Webdriver for even more flexibility. I’m not familiar with the internals of this project, but superficially, it looks like it interfaces directly with Again, only looked at this for an hour or so (burning midnight oil here) but I’m thinking BackstopJS/Selenium/Docker would be a really kickass combination! 😎 |
improved webdriver io support, still have some open tasks, to fully support the api.