Skip to content

Commit

Permalink
Throw error and disable suite for duplicate testcases in describe ins…
Browse files Browse the repository at this point in the history
…tance. (#4302)

Co-authored-by: Priyansh Garg <priyanshgarg30@gmail.com>
  • Loading branch information
badra022 and garg3133 authored Dec 4, 2024
1 parent 8967814 commit 329d797
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 2 deletions.
15 changes: 14 additions & 1 deletion lib/testsuite/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,20 @@ class Context extends EventEmitter {
throw new Error(`The "${testName}" test script must be a function. "${typeof testFn}" given.`);
}

// TODO: warn if test name already exists
if (this.allScreenedTests.includes(testName)) {
const {Logger} = Utils;

const err = new Error(
'An error occurred while loading the testsuite:\n' +
`A testcase with name "${testName}" already exists. Testcases must have unique names inside the test suite, ` +
'otherwise testcases with duplicate names might not run at all.\n\n' +
'This testsuite has been disabled, please fix the error to run it again properly.'
);
Logger.error(err);

this.setAttribute('@disabled', true);
}

if (!skipTest) {
this.tests.push(testName);
} else {
Expand Down
4 changes: 3 additions & 1 deletion lib/testsuite/interfaces/describe.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,10 @@ class Describe extends Common {
context.xcontext =
context.describe.skip = (title, describeFn) => {
this.instance.once('module-loaded', () => {
// in case tests have been declared using other interfaces (e.g. exports)
// in case tests have been declared using other interfaces (e.g. exports),
// we do not want to disable the suite.
if (this.instance.tests.length === 0) {
// if no tests are added after all interfaces are loaded, disable the suite.
this.instance.setAttribute('@disabled', true);
}
});
Expand Down
11 changes: 11 additions & 0 deletions test/sampletests/withdescribe/skipped/duplicateTestcases.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
describe('test', function () {
test('test find', async (browser) => {
browser.globals.calls++;
browser.element.find('#weblogin');
});

test('test find', async (browser) => {
browser.globals.calls++;
browser.element.find('#weblogin');
});
});
19 changes: 19 additions & 0 deletions test/src/runner/testRunTestcase.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,25 @@ describe('testRunTestcase', function() {
}));
});

it('testRunner with duplicated testcases', function() {
const testsPath = path.join(__dirname, '../../sampletests/withdescribe/skipped/duplicateTestcases.js');
const globals = {
calls: 0,
reporter(results, cb) {
assert.strictEqual(globals.calls, 0);
assert.strictEqual(results.skipped, 2);
cb();
},
retryAssertionTimeout: 0
};

return runTests({
_source: [testsPath]
}, settings({
globals
}));
});

it('testRunner with skipped beforeEach afterEach hooks', function() {
const testsPath = path.join(__dirname, '../../sampletests/withdescribe/skipped/skipBeforeAfterEach.js');
const globals = {
Expand Down

0 comments on commit 329d797

Please sign in to comment.