Skip to content

Remove sleep delay from JavaScript tests#7247

Merged
aduth merged 2 commits intomainfrom
aduth-rm-js-test-sleep
Oct 28, 2022
Merged

Remove sleep delay from JavaScript tests#7247
aduth merged 2 commits intomainfrom
aduth-rm-js-test-sleep

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 28, 2022

🛠 Summary of changes

Removes an unnecessary sleep delay from JavaScript tests, to avoid prolonging runtime of the tests. Ideally we should never have a realtime delay in any spec.

Impact should be at least 12 seconds faster test runtime (2x 6 second delay). Assuming an average of 1m30s test runtime in GitLab, should hopefully equate to ~13% faster completion.

Per the inline code comment, I tested to confirm that the tests would fail as expected before and after the removal of the delay with a reversion of the fix in #6938.

I'd like to remove this test group itIsUnaffectedByNewRelicEventBug altogether at some point after this and #7246 are merged, since we ideally shouldn't have to build-in assumed hostile third-party script handling to our specs.

📜 Testing Plan

Revert patch:

diff --git a/app/javascript/packages/memorable-date/index.ts b/app/javascript/packages/memorable-date/index.ts
index ced471a93..02ce0e931 100644
--- a/app/javascript/packages/memorable-date/index.ts
+++ b/app/javascript/packages/memorable-date/index.ts
@@ -108,11 +108,6 @@ class MemorableDateElement extends HTMLElement {
     this.validate();
 
     const inputListener = (event: Event) => {
-      // Don't process the event if this function generated it
-      if (event instanceof CustomEvent && event.detail?.flag === CUSTOM_INPUT_EVENT_DETAIL_FLAG) {
-        return;
-      }
-
       this.validate();
 
       // Artificially trigger input events on all inputs
@@ -122,16 +117,15 @@ class MemorableDateElement extends HTMLElement {
       // message (instead of only the selected field).
       const otherInputs = allInputs.filter((input) => input !== event.target);
 
-      otherInputs.forEach((input) => {
-        input.dispatchEvent(
-          new CustomEvent('input', {
-            bubbles: true,
-            detail: {
-              flag: CUSTOM_INPUT_EVENT_DETAIL_FLAG,
-            },
-          }),
-        );
-      });
+      try {
+        this.removeEventListener('input', inputListener);
+        otherInputs.forEach((input) => {
+          // Prevent recursion by removing listener temporarily
+          input.dispatchEvent(new CustomEvent('input', { bubbles: true }));
+        });
+      } finally {
+        this.addEventListener('input', inputListener);
+      }
     };
 
     this.addEventListener('input', inputListener);

changelog: Internal, Automated Tests, Improve runtime of automated tests
@aduth aduth merged commit c306b97 into main Oct 28, 2022
@aduth aduth deleted the aduth-rm-js-test-sleep branch October 28, 2022 19:36
@aduth aduth mentioned this pull request Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants