[ftr] remove digdug, use chromedriver directly #11558

spalger · 2017-05-02T01:30:49Z

why?

digdug is meant to be used by intern, and expects a certain lifecycle that the FTR has tried and continuously fails to mimic
rather than continue trying to force digdug into the stack, just spawn chromedriver ourselves. We know how to handle it
cleans up verbose remote logging while we're in there, since selenium-standalone-server was previously doing the verbose logging, and was provided by digdug
deprecate servers.webdriver ftr config
added chromedriver.url ftr config
if url at chromedriver.url points to a server that responds to pings, use it as the chromedriver instance, enables running chrome in a VM or container
if pings to chromedriver.url fail a local chromedriver is started

why? - digdug is meant to be used by intern, and expects a certain lifecycle that the FTR has tried and continuously fails to mimic - rather than continue trying to force digdug into the stack, just spawn chromedriver ourselves. We know how to handle it - cleans up verbose remote logging while we're in there, since selenium-standalone-server went with digdug, which was previously doing the verbose logging - deprecate config.servers.webdriver - add config.chromedriver.url - if url at config.chromedriver.url points to a server that responds to http requests use it as the chromedriver instance, enables running chrome in a VM or container - if pings to config.chromedriver.url fail a local chromedriver is started

rhoboat · 2017-05-04T14:14:21Z

Confirmed that tests run fine, and that Chrome and chromedriver are killed after failed test, passing tests, and interrupt (Ctrl+C).

rhoboat · 2017-05-04T14:25:09Z

When there is no kibana/es server running, the tests start and fail, but chromedriver does not get killed.

rhoboat · 2017-05-04T14:26:34Z

jenkins, test it

result: tests all passed but publishing artifacts to s3 failed.

rhoboat

Small spelling error and, if it's possible, killing chromedriver when there's a client request error, like:

     │        Client request error: connect ECONNREFUSED 127.0.0.1:5620
     │         Error: connect ECONNREFUSED 127.0.0.1:5620
     │         at Object.exports._errnoException (util.js:1022:11)
     │         at exports._exceptionWithHostPort (util.js:1045:20)
     │         at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1087:14)

Besides, LGTM!

I also confirmed that a separate instance of chromedriver running will get picked up and used by the ftr.

rhoboat · 2017-05-04T14:29:51Z

test/functional/services/remote/chromedriver_api/chromedriver_local_api.js

+
+      proc.on('exit', (code) => {
+        if (!api.isStopped() || code > 0) {
+          api.emit('error', new Error(`Chromedriver exitted with code ${code}`));


rhoboat · 2017-05-04T14:30:44Z

tasks/test.js

-    'clean:screenshots',
-    'functionalTestRunner'
-  ]);
-


rhoboat · 2017-05-04T14:34:11Z

test/functional/services/remote/chromedriver_api/chromedriver_api.js

+    this._url = url;
+    this._state = undefined;
+    this._callCustomStart = () => start(this);
+    this._callCustomStop = () => stop(this);


these two lines, do you need to pass this as an argument? i don't think it's used.

nevermind, i see it now. this is passed in as api in the factory methods.

rhoboat · 2017-05-04T14:42:08Z

test/functional/services/remote/chromedriver_api/chromedriver_local_api.js

+    },
+
+    async stop() {
+      await fcb(cb => treeKill(proc.pid, undefined, cb));


++ this is awesome. i didn't know about fromCallback in bluebird. but, why are you passing undefined as the 2nd arg? does it do SIGKILL by default?

I think it does SIGINT by default... I'm just passing undefined so it will use it's default.

rhoboat · 2017-05-04T14:50:11Z

test/functional/services/remote/chromedriver_api/chromedriver_api.js

+   *  Create an instance of the ChromedriverApi for a url.
+   *
+   *  @param  {ToolingLog} log
+   *  @param  {Lifecycle} lifecycle


do we need this comment?

…rt-remote-failures

* [ftr] remove digdug, use chromedriver directly why? - digdug is meant to be used by intern, and expects a certain lifecycle that the FTR has tried and continuously fails to mimic - rather than continue trying to force digdug into the stack, just spawn chromedriver ourselves. We know how to handle it - cleans up verbose remote logging while we're in there, since selenium-standalone-server went with digdug, which was previously doing the verbose logging - deprecate config.servers.webdriver - add config.chromedriver.url - if url at config.chromedriver.url points to a server that responds to http requests use it as the chromedriver instance, enables running chrome in a VM or container - if pings to config.chromedriver.url fail a local chromedriver is started * address review requests (cherry picked from commit f76bef4)

spalger · 2017-05-04T22:22:51Z

5.5/5.x: 0ee55f7

spalger added review test v6.0.0 labels May 2, 2017

spalger requested a review from rhoboat May 2, 2017 01:30

spalger force-pushed the debug/ftr-start-remote-failures branch 3 times, most recently from 6da72e8 to 22f0c8d Compare May 3, 2017 02:00

spalger force-pushed the debug/ftr-start-remote-failures branch from 22f0c8d to 3143aa9 Compare May 3, 2017 02:06

spalger changed the title ~~[ftr] temporarily force log level verbose during startup~~ [ftr] remove digdug, use chromedriver directly May 3, 2017

rhoboat suggested changes May 4, 2017

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into debug/ftr-sta…

d2bf829

…rt-remote-failures

rhoboat approved these changes May 4, 2017

View reviewed changes

address review requests

8db3d65

spalger force-pushed the debug/ftr-start-remote-failures branch from 50bff62 to 8db3d65 Compare May 4, 2017 18:03

spalger merged commit f76bef4 into elastic:master May 4, 2017

spalger added the v5.5.0 label May 4, 2017

spalger mentioned this pull request May 4, 2017

increase chromedriver startup timout to 15 seconds #11609

Merged

spalger deleted the debug/ftr-start-remote-failures branch October 18, 2019 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ftr] remove digdug, use chromedriver directly #11558

[ftr] remove digdug, use chromedriver directly #11558

spalger commented May 2, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat left a comment •

edited

Loading

rhoboat May 4, 2017

rhoboat May 4, 2017

rhoboat May 4, 2017

rhoboat May 4, 2017

rhoboat May 4, 2017

spalger May 4, 2017

rhoboat May 4, 2017

spalger May 4, 2017

spalger commented May 4, 2017

[ftr] remove digdug, use chromedriver directly #11558

[ftr] remove digdug, use chromedriver directly #11558

Conversation

spalger commented May 2, 2017 • edited Loading

rhoboat commented May 4, 2017 • edited Loading

rhoboat commented May 4, 2017 • edited Loading

rhoboat commented May 4, 2017 • edited Loading

rhoboat left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger commented May 4, 2017

spalger commented May 2, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat commented May 4, 2017 •

edited

Loading

rhoboat left a comment •

edited

Loading