From aace0f47702c0ae98cc2d58e9e4a55334ef2e738 Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Mon, 9 Dec 2024 03:04:22 +0530 Subject: [PATCH 1/6] Abstain tree from resolving in between test case. --- lib/core/asynctree.js | 4 ++++ lib/core/queue.js | 3 ++- lib/testsuite/context.js | 14 ++++++++++++-- lib/testsuite/runnable.js | 13 ++++++++++--- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/core/asynctree.js b/lib/core/asynctree.js index 926a45a17f..4c8e31bf82 100644 --- a/lib/core/asynctree.js +++ b/lib/core/asynctree.js @@ -212,6 +212,10 @@ class AsyncTree extends EventEmitter{ } async done(err = null) { + if (this.currentResult instanceof Promise && !this.currentResult.settled) { + return err; + } + this.emit('asynctree:finished', this); this.empty(); this.createRootNode(); diff --git a/lib/core/queue.js b/lib/core/queue.js index 3e8b3a71bb..970489b566 100644 --- a/lib/core/queue.js +++ b/lib/core/queue.js @@ -145,7 +145,8 @@ class CommandQueue extends EventEmitter { return Promise.resolve(); } - run() { + run(result) { + this.tree.currentResult = result; if (this.tree.started) { return this; } diff --git a/lib/testsuite/context.js b/lib/testsuite/context.js index 9a9b4f3f7c..f12e971503 100644 --- a/lib/testsuite/context.js +++ b/lib/testsuite/context.js @@ -525,7 +525,12 @@ class Context extends EventEmitter { args[0] = client.api; } - return this.testsuite[fnName].apply(context, args); + const result = this.testsuite[fnName].apply(context, args); + if (this.currentRunnable) { + this.currentRunnable.currentTestCaseResult = result; + } + + return result; } /** @@ -541,7 +546,12 @@ class Context extends EventEmitter { const fnAsync = Utils.makeFnAsync(expectedArgs, this.testsuite[fnName], context); const args = this.getHookFnArgs(done, api, expectedArgs); - return fnAsync.apply(context, args); + const result = fnAsync.apply(context, args); + if (this.currentRunnable) { + this.currentRunnable.currentTestCaseResult = result; + } + + return result; } //////////////////////////////////////////////////////////////// diff --git a/lib/testsuite/runnable.js b/lib/testsuite/runnable.js index b60f43d3a8..6671d9a4f9 100644 --- a/lib/testsuite/runnable.js +++ b/lib/testsuite/runnable.js @@ -103,8 +103,8 @@ class Runnable { } setDoneCallback(cb) { - let originalResolve = this.deffered.resolveFn; - let originalReject = this.deffered.rejectFn; + const originalResolve = this.deffered.resolveFn; + const originalReject = this.deffered.rejectFn; this.deffered.resolveFn = function() {}; this.deffered.rejectFn = function() {}; @@ -159,7 +159,14 @@ class Runnable { } } - this.queue.run().then(err => { + if (this.currentTestCaseResult instanceof Promise) { + this.currentTestCaseResult.finally(() => { + this.currentTestCaseResult.settled = true; + this.queue.scheduleTraverse(); + }); + } + + this.queue.run(this.currentTestCaseResult).then(err => { if (err) { return this.deffered.rejectFn(err); } From 54914bf5ca14d48775d4a07580a1e369d00d7234 Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Mon, 9 Dec 2024 16:26:50 +0530 Subject: [PATCH 2/6] Fix failing tests due to unhandledRejection. --- lib/testsuite/runnable.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/testsuite/runnable.js b/lib/testsuite/runnable.js index 6671d9a4f9..6285741177 100644 --- a/lib/testsuite/runnable.js +++ b/lib/testsuite/runnable.js @@ -160,10 +160,14 @@ class Runnable { } if (this.currentTestCaseResult instanceof Promise) { - this.currentTestCaseResult.finally(() => { - this.currentTestCaseResult.settled = true; - this.queue.scheduleTraverse(); - }); + this.currentTestCaseResult + .catch(() => { + // to avoid unhandledRejections + }) + .finally(() => { + this.currentTestCaseResult.settled = true; + this.queue.scheduleTraverse(); + }); } this.queue.run(this.currentTestCaseResult).then(err => { From 88878d05d8023a35c1708812d4c0e6be8c682917 Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Mon, 16 Dec 2024 22:27:11 +0530 Subject: [PATCH 3/6] Add code comment. --- lib/testsuite/runnable.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/testsuite/runnable.js b/lib/testsuite/runnable.js index 6285741177..5e1d2d44f9 100644 --- a/lib/testsuite/runnable.js +++ b/lib/testsuite/runnable.js @@ -163,6 +163,8 @@ class Runnable { this.currentTestCaseResult .catch(() => { // to avoid unhandledRejections + // without `catch`, in case of rejection of `this.currentTestCaseResult` promise, + // `.finally` will return a new promise that will be rejected without any error handling. }) .finally(() => { this.currentTestCaseResult.settled = true; From bf47c8ce3e03f71bbe2d5c4dc1c91cd3b9ca127b Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Sun, 29 Dec 2024 02:32:24 +0530 Subject: [PATCH 4/6] Add code comments. --- lib/core/asynctree.js | 5 +++++ lib/testsuite/runnable.js | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/core/asynctree.js b/lib/core/asynctree.js index 4c8e31bf82..11fbe1a104 100644 --- a/lib/core/asynctree.js +++ b/lib/core/asynctree.js @@ -212,6 +212,11 @@ class AsyncTree extends EventEmitter{ } async done(err = null) { + // `this.currentResult` represents the return value of the currently + // executing `it` test case and hook. + // We do not want to clear the tree if the test case is still running. + // In case of an error, the tree will be cleared in the `runChildNode` + // method itself. if (this.currentResult instanceof Promise && !this.currentResult.settled) { return err; } diff --git a/lib/testsuite/runnable.js b/lib/testsuite/runnable.js index 5e1d2d44f9..e57847674b 100644 --- a/lib/testsuite/runnable.js +++ b/lib/testsuite/runnable.js @@ -159,15 +159,25 @@ class Runnable { } } + // `this.currentTestCaseResult` represents the return value of the + // `it` function and other hooks. if (this.currentTestCaseResult instanceof Promise) { this.currentTestCaseResult .catch(() => { // to avoid unhandledRejections - // without `catch`, in case of rejection of `this.currentTestCaseResult` promise, - // `.finally` will return a new promise that will be rejected without any error handling. + // although the test case promises are already handled for rejections + // above (`result.catch()`), if we don't use `.catch()` here again, + // `.finally` will return a new promise that will be rejected without + // any error handling. }) .finally(() => { + // mark the promise as settled as a cue to the asynctree so that it + // can get cleared out and subsequently call the queue `done` method. this.currentTestCaseResult.settled = true; + + // sometimes this promise is settled after the last call of + // asynctree `done` method, so we need schedule a tree traversal + // again to clear out the tree and call the queue `done` method. this.queue.scheduleTraverse(); }); } From 9884c309890147280ea9569a5cd60459d0c3d642 Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Mon, 30 Dec 2024 20:40:44 +0530 Subject: [PATCH 5/6] Add tests. --- .../sampletests/withdescribe/basic2/sample.js | 80 +++++++++++++++++++ test/src/core/testAsyncTree.js | 47 +++++++++++ 2 files changed, 127 insertions(+) create mode 100644 test/sampletests/withdescribe/basic2/sample.js create mode 100644 test/src/core/testAsyncTree.js diff --git a/test/sampletests/withdescribe/basic2/sample.js b/test/sampletests/withdescribe/basic2/sample.js new file mode 100644 index 0000000000..6a03ec49c9 --- /dev/null +++ b/test/sampletests/withdescribe/basic2/sample.js @@ -0,0 +1,80 @@ +const assert = require('assert'); + +describe('basic describe test', async function () { + const nwClient = this['[client]']; + + const asynctreeEventListener = () => { + this.globals.asynctreeFinishedEventCount++; + }; + + const queueEventListener = () => { + this.globals.queueFinishedEventCount++; + }; + + this.desiredCapabilities = { + name: 'basicDescribe' + }; + + test('with awaited JS command bw Nightwatch commands', async function (client) { + nwClient.queue.tree.on('asynctree:finished', asynctreeEventListener); + + // queue event listeners are automatically removed after + // every (test case + afterEach + after (if applicable)) + // so, no need to remove this listener manually before next test case run. + nwClient.queue.on('queue:finished', queueEventListener); + + await client.assert.strictEqual(client.options.desiredCapabilities.name, 'basicDescribe'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 1); + + await client.url('http://localhost'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 2); + + await new Promise((resolve) => { + setTimeout(function() { + resolve(null); + }, 50); + }); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 2); + + await client.assert.elementPresent('#weblogin'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 3); + + // queue:finished event shouldn't be emitted in between the test. + assert.equal(this.globals.queueFinishedEventCount, 0); + }); + + test('with await in last command', async function (client) { + nwClient.queue.tree.on('asynctree:finished', asynctreeEventListener); + + await client.assert.strictEqual(client.options.desiredCapabilities.name, 'basicDescribe'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 1); + + await client.url('http://localhost'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 2); + + await client.assert.elementPresent('#weblogin'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 3); + }); + + test('without await in last command', async function (client) { + nwClient.queue.tree.on('asynctree:finished', asynctreeEventListener); + + await client.assert.strictEqual(client.options.desiredCapabilities.name, 'basicDescribe'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 1); + + await client.url('http://localhost'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 2); + + client.assert.elementPresent('#weblogin'); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 3); + }); + + afterEach(function () { + nwClient.queue.tree.removeListener('asynctree:finished', asynctreeEventListener); + assert.equal(nwClient.queue.tree.rootNode.childNodes.length, 0); + }); + + after(function (client) { + client.end(); + }); +}); diff --git a/test/src/core/testAsyncTree.js b/test/src/core/testAsyncTree.js new file mode 100644 index 0000000000..676698ca8b --- /dev/null +++ b/test/src/core/testAsyncTree.js @@ -0,0 +1,47 @@ +const assert = require('assert'); +const path = require('path'); +const CommandGlobals = require('../../lib/globals/commands-w3c.js'); +const common = require('../../common.js'); +const {runTests} = common.require('index.js'); +const {settings} = common; + +describe('element().isPresent() command', function() { + before(function (done) { + CommandGlobals.beforeEach.call(this, done); + }); + + after(function (done) { + CommandGlobals.afterEach.call(this, done); + }); + + it('test runner with multiple test interfaces - exports/describe', function () { + const srcFolders = [ + path.join(__dirname, '../../sampletests/withdescribe/basic2') + ]; + + const globals = { + asynctreeFinishedEventCount: 0, + queueFinishedEventCount: 0, + reporter(results, cb) { + if (results.lastError) { + throw results.lastError; + } + + // should equal the number of test cases. + assert.equal(globals.asynctreeFinishedEventCount, 3); + + // only counts for the first test case + afterEach + assert.equal(globals.queueFinishedEventCount, 2); + + cb(); + } + }; + + return runTests(settings({ + globals, + src_folders: srcFolders, + output: true, + silent: false + })); + }); +}); From a699263296209514e6b9b6d2f2ef853dd8ef8942 Mon Sep 17 00:00:00 2001 From: Priyansh Garg Date: Mon, 30 Dec 2024 22:28:40 +0530 Subject: [PATCH 6/6] Few small refactors. --- lib/core/asynctree.js | 6 +++--- lib/core/queue.js | 4 ++-- lib/testsuite/runnable.js | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/core/asynctree.js b/lib/core/asynctree.js index 11fbe1a104..4c782cba2a 100644 --- a/lib/core/asynctree.js +++ b/lib/core/asynctree.js @@ -212,12 +212,12 @@ class AsyncTree extends EventEmitter{ } async done(err = null) { - // `this.currentResult` represents the return value of the currently - // executing `it` test case and hook. + // `this.currentTestCaseResult` represents the return value of the + // currently executing `it` test case or hook. // We do not want to clear the tree if the test case is still running. // In case of an error, the tree will be cleared in the `runChildNode` // method itself. - if (this.currentResult instanceof Promise && !this.currentResult.settled) { + if (this.currentTestCaseResult instanceof Promise && !this.currentTestCaseResult.settled) { return err; } diff --git a/lib/core/queue.js b/lib/core/queue.js index 970489b566..eac2f33966 100644 --- a/lib/core/queue.js +++ b/lib/core/queue.js @@ -145,8 +145,8 @@ class CommandQueue extends EventEmitter { return Promise.resolve(); } - run(result) { - this.tree.currentResult = result; + run(currentTestCaseResult) { + this.tree.currentTestCaseResult = currentTestCaseResult; if (this.tree.started) { return this; } diff --git a/lib/testsuite/runnable.js b/lib/testsuite/runnable.js index e57847674b..bb42d03ef1 100644 --- a/lib/testsuite/runnable.js +++ b/lib/testsuite/runnable.js @@ -159,8 +159,10 @@ class Runnable { } } - // `this.currentTestCaseResult` represents the return value of the - // `it` function and other hooks. + // `this.currentTestCaseResult` represents the return value of the currently + // running test case or hook. + // in case the runnable is executing something other than a test case/hook, + // `this.currentTestCaseResult` will be `undefined`. if (this.currentTestCaseResult instanceof Promise) { this.currentTestCaseResult .catch(() => {