From 0e2eb7904d8dd8bce04f556cbd31376cfadd4f7d Mon Sep 17 00:00:00 2001
From: "ahmed.badra" <42746335+badra022@users.noreply.github.com>
Date: Fri, 15 Nov 2024 15:09:38 +0200
Subject: [PATCH 1/3] added thrown error for duplicate testcases in describe
 instance

testcases have a 1 to 1 relation with functions so each test name should map to one function only, can't map to multiple functions in the same describeInstance
---
 lib/testsuite/context.js | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/testsuite/context.js b/lib/testsuite/context.js
index b32f9f9cc5..a79aaf7bdc 100644
--- a/lib/testsuite/context.js
+++ b/lib/testsuite/context.js
@@ -316,9 +316,13 @@ 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 (!skipTest) {
-      this.tests.push(testName);
+      if(this.tests.includes(testName)) {
+        throw new Error(`${testName} step already exists, testcases must have unique names in the test suite.`);
+      }
+      else {
+        this.tests.push(testName);
+      }
     } else {
       this.skippedTests.push(testName);
     }

From 286b73a775cb460f160646997640aeb351357716 Mon Sep 17 00:00:00 2001
From: Priyansh Garg <priyanshgarg30@gmail.com>
Date: Wed, 27 Nov 2024 06:21:14 +0530
Subject: [PATCH 2/3] Fix placement of error

---
 lib/testsuite/context.js | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/testsuite/context.js b/lib/testsuite/context.js
index a79aaf7bdc..fbefb6b793 100644
--- a/lib/testsuite/context.js
+++ b/lib/testsuite/context.js
@@ -316,13 +316,12 @@ class Context extends EventEmitter {
       throw new Error(`The "${testName}" test script must be a function. "${typeof testFn}" given.`);
     }
 
+    if (this.allScreenedTests.includes(testName)) {
+      throw new Error(`A testcase with name "${testName}" already exists. Testcases must have unique names inside a test suite.`);
+    }
+
     if (!skipTest) {
-      if(this.tests.includes(testName)) {
-        throw new Error(`${testName} step already exists, testcases must have unique names in the test suite.`);
-      }
-      else {
-        this.tests.push(testName);
-      }
+      this.tests.push(testName);
     } else {
       this.skippedTests.push(testName);
     }

From 11138e7dd196958807f1dd9febddce68be39097f Mon Sep 17 00:00:00 2001
From: Priyansh Garg <priyanshgarg30@gmail.com>
Date: Wed, 27 Nov 2024 09:27:57 +0530
Subject: [PATCH 3/3] Log the error directly instead of throwing it.

---
 lib/testsuite/context.js             | 12 +++++++++++-
 lib/testsuite/interfaces/describe.js |  4 +++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/testsuite/context.js b/lib/testsuite/context.js
index fbefb6b793..81785510c7 100644
--- a/lib/testsuite/context.js
+++ b/lib/testsuite/context.js
@@ -317,7 +317,17 @@ class Context extends EventEmitter {
     }
 
     if (this.allScreenedTests.includes(testName)) {
-      throw new Error(`A testcase with name "${testName}" already exists. Testcases must have unique names inside a test suite.`);
+      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 properly.'
+      );
+      Logger.error(err);
+
+      this.setAttribute('@disabled', true);
     }
 
     if (!skipTest) {
diff --git a/lib/testsuite/interfaces/describe.js b/lib/testsuite/interfaces/describe.js
index 35d4d1e6bb..d0bd0a456a 100644
--- a/lib/testsuite/interfaces/describe.js
+++ b/lib/testsuite/interfaces/describe.js
@@ -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);
             }
           });