Issue 6930 - Add support for Microsoft Edge to WebDriver-based test automation

Issue 6930 - Add support for Microsoft Edge to WebDriver-based test automation

[geo, 2018-12-05 13:35:28]

You'll see I've added several calls to `driver.sleep`. The reason behind those
calls is that otherwise I'm getting inconsistent behavior: running the tests 10
times in a row might work perfectly fine or they might either crash/fail.

https://codereview.adblockplus.org/29959561/diff/29959562/README.md
File README.md (right):

https://codereview.adblockplus.org/29959561/diff/29959562/README.md#newcode130
README.md:130: 
Maybe I should mention that for Edge, we always test only against the installed
version, but I don't know exactly how to formulate that, or where to place it.

[Sebastian Noack, 2018-12-06 03:47:32]

https://codereview.adblockplus.org/29959561/diff/29959562/README.md
File README.md (right):

https://codereview.adblockplus.org/29959561/diff/29959562/README.md#newcode108
README.md:108: Also you are going to have to install [Microsoft
Webdriver](https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/)
This will be outdated very soon, as on Insider builds and future releases,
Microsoft WebDriver will be a Windows Feature on Demand.

https://codereview.adblockplus.org/29959561/diff/29959562/test/all.js
File test/all.js (right):

https://codereview.adblockplus.org/29959561/diff/29959562/test/all.js#newcode29
test/all.js:29: if (browser == "edge")
Nit: This can go at the very top of the function.

https://codereview.adblockplus.org/29959561/diff/29959562/test/all.js#newcode30
test/all.js:30: return [{version: "installed", getPath: () =>
Promise.resolve(null)}];
Nit: The version property only exists to distinguish the oldest and latest
version of Chromium and Firefox. I think we should just omit it here (since we
only run the tests on one version of Microsoft Edge, rather than having it
listed as "Edge (installed)" in the test output.

https://codereview.adblockplus.org/29959561/diff/29959562/test/all.js#newcode64
test/all.js:64: describe.skip : describe;
Is this sufficient? I suppose the before() hook would still run and fail for
Microsoft Edge on platforms other than Windows.

https://codereview.adblockplus.org/29959561/diff/29959562/test/all.js#newcode103
test/all.js:103: return this.driver.sleep(3000)
Is this really necessary? Or is there any promise we could wait for? If not,
retrying would be at least better than a fixed delay.

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge.js
File test/browsers/edge.js (right):

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:31: "AppData", "Local", "Packages",
"Microsoft.MicrosoftEdge_8wekyb3d8bbwe",
Will this work universally? Microsoft.MicrosoftEdge_8wekyb3d8bbwe seems
environment specific?

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:58: `powershell Copy-Item ${buildDir} -Destination
${extDir} -Recurse`,
Calling into powershell to recursively copy a directory seems inappropriate. We
already require the "ncp" package which can do that.

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:75: fs.exists(extDir, exists =>
Could you use util.promisify() here?

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:92: .build();
Nit: It seems wrapping this line isn't necessary.

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:102: let driver = edge.Driver.createSession(capabilities,
service);
Nit: Just return the driver without creating temporary variable.

https://codereview.adblockplus.org/29959561/diff/29959562/test/browsers/edge....
test/browsers/edge.js:106: 
Nit: Redundant blank lines at end of file.

https://codereview.adblockplus.org/29959561/diff/29959562/test/wrappers/pages.js
File test/wrappers/pages.js (right):

https://codereview.adblockplus.org/29959561/diff/29959562/test/wrappers/pages...
test/wrappers/pages.js:102: this.driver.sleep(1000).then(() =>
Please retry rather than using delay:

  this.driver.wait(until.elementLocated(By.css(".site-pagelist a"), 1000)

https://codereview.adblockplus.org/29959561/diff/29959562/test/wrappers/pages...
test/wrappers/pages.js:242: this.driver.sleep(1500).then(() =>
No delays please, use this.driver.wait() as shown here:
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/9#note_1...