Fix #3466 Wait for queue traversal to begin before calling done() (#3490)

* set tree to started immediately

* if queue is not empty, wait for initial traversal to begin

* remove unnecessary changes

* add tests

* fix tests

Author: swrdfish

lib/core/queue.js

@@ -107,6 +107,10 @@ class CommandQueue extends EventEmitter {
     return this;
   }
 
+  get inProgress() {
+    return this.tree.rootNode.childNodes.length > 0;
+  }
+  
   done(err) {
     if (this.tree.rootNode.childNodes.length > 0) {
       return this;

lib/testsuite/hooks/_basehook.js

@@ -88,8 +88,19 @@ class BaseHook {
             if (result instanceof Error) {
               result.skipTestCases = this.skipTestcasesOnError;
             }
-            runnableDone && runnableDone(result);
-            doneFn(result);
+
+            if (this.context.queue && this.context.queue.inProgress) {
+              process.nextTick(() => {
+              
+                this.context.queue.once('queue:finished', _ => {
+                  runnableDone && runnableDone(result);
+                  doneFn(result);
+                });
+              });
+            } else {
+              runnableDone && runnableDone(result);
+              doneFn(result);
+            }
           });
 
           if (argsCount <= 1 && (invocationResult instanceof Promise)) {

test/sampletests/beforewithcommand/commandInsideBefore.js

@@ -0,0 +1,34 @@
+var assert = require('assert');
+
+module.exports = {
+  before: function(client, cb) {
+    client.assert.ok(false);
+    client.globals.calls++;
+    cb();
+  },
+
+  beforeEach: function(client, cb) {
+    client.globals.calls++;
+    cb();
+  },
+
+  demoTestSyncOne: function (client) {
+    client.url('http://localhost');
+    var testName = client.currentTest.name;
+    assert.strictEqual(testName, 'demoTestSyncOne');
+  },
+
+  demoTestSyncTwo: function (client) {
+    var testName = client.currentTest.name;
+    assert.strictEqual(testName, 'demoTestSyncTwo');
+    client.end();
+  },
+
+  afterEach: function(client) {
+    client.globals.calls++;
+  },
+
+  after: function(client) {
+    client.globals.calls++;
+  }
+};

test/sampletests/beforewithcommand/commandInsideBeforeWithNoParams.js

@@ -0,0 +1,33 @@
+var assert = require('assert');
+
+module.exports = {
+  before: function() {
+    browser.assert.ok(false);
+    browser.globals.calls++;
+  },
+
+  beforeEach: function(client, cb) {
+    client.globals.calls++;
+    cb();
+  },
+
+  demoTestSyncOne: function (client) {
+    client.url('http://localhost');
+    var testName = client.currentTest.name;
+    assert.strictEqual(testName, 'demoTestSyncOne');
+  },
+
+  demoTestSyncTwo: function (client) {
+    var testName = client.currentTest.name;
+    assert.strictEqual(testName, 'demoTestSyncTwo');
+    client.end();
+  },
+
+  afterEach: function(client) {
+    client.globals.calls++;
+  },
+
+  after: function(client) {
+    client.globals.calls++;
+  }
+};

test/src/runner/testRunWithHooks.js

@@ -300,6 +300,60 @@ describe('testRunWithHooks', function() {
     }));
   });
 
+  it('testRunner with command inside before', function() {
+    let testsPath = path.join(__dirname, '../../sampletests/beforewithcommand/commandInsideBefore.js');
+    let globals = {
+      calls: 0,
+      reporter(results) {
+        assert.ok(results.lastError instanceof Error);
+
+        assert.strictEqual(globals.calls, 6);
+        let result = results.modules.commandInsideBefore.completed;
+        // assert.ok('demoTestSyncOne' in result);
+        assert.ok(!('beforeEach' in result));
+        assert.ok(!('before' in result));
+        assert.ok(!('afterEach' in result));
+        assert.ok(!('after' in result));
+      }
+    };
+
+
+    return runTests({
+      _source: [testsPath]
+    }, settings({
+      seleniumPort: 10195,
+      globals,
+      output_folder: false
+    }));
+  });
+
+  it('testRunner with command inside before with 0 parameters', function() {
+    let testsPath = path.join(__dirname, '../../sampletests/beforewithcommand/commandInsideBeforeWithNoParams.js');
+    let globals = {
+      calls: 0,
+      reporter(results) {
+        assert.ok(results.lastError instanceof Error);
+
+        assert.strictEqual(globals.calls, 6);
+        let result = results.modules.commandInsideBeforeWithNoParams.completed;
+        // assert.ok('demoTestSyncOne' in result);
+        assert.ok(!('beforeEach' in result));
+        assert.ok(!('before' in result));
+        assert.ok(!('afterEach' in result));
+        assert.ok(!('after' in result));
+      }
+    };
+
+
+    return runTests({
+      _source: [testsPath]
+    }, settings({
+      seleniumPort: 10195,
+      globals,
+      output_folder: false
+    }));
+  });
+
   it('testRunWithAsyncHooks', function() {
     let testsPath = path.join(__dirname, '../../sampletests/withasynchooks');
     let globals = {

test/src/runner/testRunnerWithAsyncAssertionFailure.js

@@ -8,7 +8,7 @@ const {runTests} = common.require('index.js');
 
 describe('test Runner with async and assertion failure', function() {
   const originalExit = process.exit;
-  this.timeout(10000);
+  this.timeout(15000);
 
   beforeEach(function(done) {
     this.server = MockServer.init();
@@ -25,7 +25,7 @@ describe('test Runner with async and assertion failure', function() {
     Globals.afterEach.call(this, done);
   });
 
-  it('test run mocha samples with async and failures', function() {
+  it('test run mocha samples with async and assertion failures', function() {
     const globals = {
       test_calls: 0,
       retryAssertionTimeout: 0,