fix and refactoring

DevExpress · Sep 4, 2018 · 39e7cf7 · 39e7cf7
1 parent cbb711f
commit 39e7cf7
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 42 deletions.
diff --git a/src/browser/connection/index.js b/src/browser/connection/index.js
@@ -19,7 +19,8 @@ export default class BrowserConnection extends EventEmitter {
     constructor (gateway, browserInfo, permanent) {
         super();
 
-        this.HEARTBEAT_TIMEOUT = 2 * 60 * 1000;
+        this.HEARTBEAT_TIMEOUT       = 2 * 60 * 1000;
+        this.BROWSER_RESTART_TIMEOUT = 60 * 1000;
 
         this.id                       = BrowserConnection._generateId();
         this.jobQueue                 = [];
@@ -110,9 +111,13 @@ export default class BrowserConnection extends EventEmitter {
         }
     }
 
+    _createBrowserDisconnectedError () {
+        return new GeneralError(MESSAGE.browserDisconnected, this.userAgent);
+    }
+
     _waitForHeartbeat () {
         this.heartbeatTimeout = setTimeout(() => {
-            const err = new GeneralError(MESSAGE.browserDisconnected, this.userAgent);
+            const err = this._createBrowserDisconnectedError();
 
             this.opened         = false;
             this.errorSupressed = false;
@@ -149,8 +154,32 @@ export default class BrowserConnection extends EventEmitter {
 
         this._forceIdle();
 
-        await this._closeBrowser();
-        await this._runBrowser();
+        let resolveTimeout = null;
+        let onTimeout      = false;
+        let timeout        = null;
+
+        const restartPromise = this._closeBrowser()
+            .then(() => this._runBrowser());
+
+        const timeoutPromise = new Promise(resolve => {
+            resolveTimeout = resolve;
+
+            timeout = setTimeout(() => {
+                onTimeout = true;
+
+                resolve();
+            }, this.BROWSER_RESTART_TIMEOUT);
+        });
+
+        Promise.race([ restartPromise, timeoutPromise ])
+            .then(() => {
+                clearTimeout(timeout);
+
+                if (onTimeout)
+                    this.emit('error', this._createBrowserDisconnectedError());
+                else
+                    resolveTimeout();
+            });
     }
 
     supressError () {

diff --git a/src/runner/test-run-controller.js b/src/runner/test-run-controller.js
@@ -153,7 +153,7 @@ export default class TestRunController extends EventEmitter {
 
         testRun.once('start', () => this.emit('test-run-start'));
         testRun.once('done', () => this._testRunDone());
-        testRun.once('disconnected', () => this._testRunDisconnected(connection, testRun));
+        testRun.once('disconnected', () => this._testRunDisconnected(connection));
 
         testRun.start();
 

diff --git a/src/test-run/index.js b/src/test-run/index.js
@@ -282,13 +282,13 @@ export default class TestRun extends EventEmitter {
             await this._runAfterHook();
         }
 
-        if (!this.connected)
+        if (this.disconnected)
             return;
 
         if (this.errs.length && this.debugOnFail)
             await this._enqueueSetBreakpointCommand(null, this.debugReporterPluginHost.formatError(this.errs[0]));
 
-        await this.executeTestDoneCommand();
+        await this.executeCommand(new TestDoneCommand());
 
         this._addPendingPageErrorIfAny();
 
@@ -367,10 +367,6 @@ export default class TestRun extends EventEmitter {
         this.browserManipulationQueue.removeAllNonServiceManipulations();
     }
 
-    get connected () {
-        return this.browserConnection.opened;
-    }
-
     // Current driver task
     get currentDriverTask () {
         return this.driverTaskQueue[0];
@@ -434,8 +430,8 @@ export default class TestRun extends EventEmitter {
         const pageError                  = this.pendingPageError || driverStatus.pageError;
         const currentTaskRejectedByError = pageError && this._handlePageErrorStatus(pageError);
 
-        if (!this.connected)
-            return null;
+        if (this.disconnected)
+            return new Promise((_, reject) => reject());
 
         this.consoleMessages.concat(driverStatus.consoleMessages);
 
@@ -682,16 +678,14 @@ export default class TestRun extends EventEmitter {
     }
 
     disconnect (err) {
+        this.disconnected = true;
+
         this._rejectCurrentDriverTask(err);
 
         this.emit('disconnected', err);
 
         delete testRunTracker.activeTestRuns[this.session.id];
     }
-
-    async executeTestDoneCommand () {
-        await this.executeCommand(new TestDoneCommand());
-    }
 }
 
 

diff --git a/test/functional/fixtures/browser-provider/browser-reconnect/test.js b/test/functional/fixtures/browser-provider/browser-reconnect/test.js
@@ -24,7 +24,7 @@ function customReporter () {
 function createConnection (browser) {
     return browserProviderPool
         .getBrowserInfo(browser)
-        .then(browserInfo => new BrowserConnection(testCafe.browserConnectionGateway, browserInfo, true));
+        .then(browserInfo => new BrowserConnection(testCafe.browserConnectionGateway, browserInfo, false));
 }
 
 async function run (pathToTest, filter) {
@@ -59,16 +59,6 @@ describe('Browser reconnect', function () {
                 });
         });
 
-        it('Should fail on 3 disconnects', function () {
-            return run('./testcafe-fixtures/index-test.js', 'Should fail on 3 disconnects')
-                .then(() => {
-                    throw new Error('Test should have failed but it succeeded');
-                })
-                .catch(err => {
-                    expect(err.message).contains('browser disconnected. This problem may appear when a browser hangs or is closed, or due to network issues');
-                });
-        });
-
         it('Should fail on 3 disconnects in one browser', function () {
             return run('./testcafe-fixtures/index-test.js', 'Should fail on 3 disconnects in one browser')
                 .then(() => {

diff --git a/test/functional/fixtures/browser-provider/browser-reconnect/testcafe-fixtures/index-test.js b/test/functional/fixtures/browser-provider/browser-reconnect/testcafe-fixtures/index-test.js
@@ -28,20 +28,6 @@ test('Should restart browser when it does not respond', async t => {
     await t.expect(counter[userAgent]).eql(3);
 });
 
-test('Should fail on 3 disconnects', async t => {
-    const userAgent = await getUserAgent();
-
-    counter[userAgent] = counter[userAgent] || 0;
-
-    counter[userAgent]++;
-
-    await t.expect(counter[userAgent]).lte(3);
-
-    await hang();
-
-    throw new Error('browser has not restarted');
-});
-
 test('Should fail on 3 disconnects in one browser', async t => {
     const userAgent = await getUserAgent();