From d5a7a6d5756dd55a651d43bb9d47bade80bcca86 Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Wed, 13 Oct 2021 08:55:12 -0400
Subject: chore: fix flaky steps_invalid_usage tests (#12422)

---
 cli/tests/testdata/test/steps/invalid_usage.out |  7 ++++---
 cli/tests/testdata/test/steps/invalid_usage.ts  |  8 +++-----
 runtime/js/40_testing.js                        | 16 ++++++++++++----
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/cli/tests/testdata/test/steps/invalid_usage.out b/cli/tests/testdata/test/steps/invalid_usage.out
index b03ca57b6..d6f3c8d33 100644
--- a/cli/tests/testdata/test/steps/invalid_usage.out
+++ b/cli/tests/testdata/test/steps/invalid_usage.out
@@ -82,9 +82,10 @@ Error: 1 test step failed.
     at [WILDCARD]
 
 parallel steps with sanitizers
-Error: 1 test step failed.
-    at runTest ([WILDCARD])
-    at [WILDCARD]
+Error: There were still test steps running after the current scope finished execution. Ensure all steps are awaited (ex. `await t.step(...)`).
+    at postValidation [WILDCARD]
+    at testStepSanitizer ([WILDCARD])
+    [WILDCARD]
 
 parallel steps when first has sanitizer
 Error: 1 test step failed.
diff --git a/cli/tests/testdata/test/steps/invalid_usage.ts b/cli/tests/testdata/test/steps/invalid_usage.ts
index f670c842e..bc761b5dd 100644
--- a/cli/tests/testdata/test/steps/invalid_usage.ts
+++ b/cli/tests/testdata/test/steps/invalid_usage.ts
@@ -33,15 +33,13 @@ Deno.test({
 Deno.test("parallel steps with sanitizers", async (t) => {
   // not allowed because steps with sanitizers cannot be run in parallel
   const step1Entered = deferred();
-  const step2Finished = deferred();
-  const step1 = t.step("step 1", async () => {
+  const testFinished = deferred();
+  t.step("step 1", async () => {
     step1Entered.resolve();
-    await step2Finished;
+    await testFinished;
   });
   await step1Entered;
   await t.step("step 2", () => {});
-  step2Finished.resolve();
-  await step1;
 });
 
 Deno.test("parallel steps when first has sanitizer", async (t) => {
diff --git a/runtime/js/40_testing.js b/runtime/js/40_testing.js
index b55e5cc2c..5b404766e 100644
--- a/runtime/js/40_testing.js
+++ b/runtime/js/40_testing.js
@@ -47,8 +47,8 @@
         await new Promise((resolve) => setTimeout(resolve, 0));
       }
 
-      if (step.hasRunningChildren) {
-        return; // test step validation error thrown, don't check ops
+      if (step.shouldSkipSanitizers) {
+        return;
       }
 
       const post = metrics();
@@ -111,8 +111,8 @@ finishing test case.`;
       const pre = core.resources();
       await fn(step);
 
-      if (step.hasRunningChildren) {
-        return; // test step validation error thrown, don't check resources
+      if (step.shouldSkipSanitizers) {
+        return;
       }
 
       const post = core.resources();
@@ -360,6 +360,7 @@ finishing test case.`;
         "failed": formatError(error),
       };
     } finally {
+      step.finalized = true;
       // ensure the children report their result
       for (const child of step.children) {
         child.reportResult();
@@ -529,6 +530,13 @@ finishing test case.`;
         this.#params.sanitizeExit;
     }
 
+    /** If a test validation error already occurred then don't bother checking
+     * the sanitizers as that will create extra noise.
+     */
+    get shouldSkipSanitizers() {
+      return this.hasRunningChildren || this.parent?.finalized;
+    }
+
     get hasRunningChildren() {
       return ArrayPrototypeSome(
         this.children,
-- 
cgit v1.2.3