#9202: fix tests

pixiebrix · Sep 25, 2024 · 33390dd · 33390dd
1 parent 6b01fa5
commit 33390dd
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 21 deletions.
diff --git a/src/bricks/transformers/controlFlow/WithCache.test.ts b/src/bricks/transformers/controlFlow/WithCache.test.ts
@@ -33,16 +33,23 @@ import brickRegistry from "@/bricks/registry";
 import { TEST_resetStateController } from "@/contentScript/stateController/stateController";
 import { reducePipeline } from "@/runtime/reducePipeline";
 import { reduceOptionsFactory } from "@/testUtils/factories/runtimeFactories";
+import { tick } from "@/starterBricks/starterBrickTestUtils";
+import { CancelError } from "@/errors/businessErrors";
+import { ContextError } from "@/errors/genericErrors";
 
 const withCacheBrick = new WithCache();
 
+const STATE_KEY = "testVariable";
+
 function makeCachePipeline(
   brick: Brick,
   {
     message,
     stateKey,
+    forceFetch = false,
   }: {
     message: string;
+    forceFetch?: boolean;
     stateKey: string | Expression;
   },
 ) {
@@ -58,6 +65,7 @@ function makeCachePipeline(
         },
       ]),
       stateKey,
+      forceFetch,
     },
   };
 }
@@ -95,7 +103,7 @@ describe("WithAsyncModVariable", () => {
 
   it("returns value if pipeline succeeds", async () => {
     const pipeline = makeCachePipeline(echoBrick, {
-      stateKey: "foo",
+      stateKey: STATE_KEY,
       message: "bar",
     });
 
@@ -110,7 +118,7 @@ describe("WithAsyncModVariable", () => {
     expect(brickOutput).toStrictEqual(expectedData);
 
     await expectPageState({
-      foo: {
+      [STATE_KEY]: {
         isLoading: false,
         isFetching: false,
         isSuccess: true,
@@ -126,7 +134,7 @@ describe("WithAsyncModVariable", () => {
 
   it("throws exception if pipeline throws", async () => {
     const pipeline = makeCachePipeline(throwBrick, {
-      stateKey: "foo",
+      stateKey: STATE_KEY,
       message: "bar",
     });
 
@@ -138,4 +146,109 @@ describe("WithAsyncModVariable", () => {
 
     await expect(brickPromise).rejects.toThrow("bar");
   });
+
+  it("memoizes value", async () => {
+    const firstCallPipeline = makeCachePipeline(asyncEchoBrick, {
+      stateKey: STATE_KEY,
+      message: "first",
+    });
+
+    const firstCallPromise = reducePipeline(
+      firstCallPipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    // Wait for the initial fetching state to be set
+    await tick();
+
+    const secondCallPipeline = makeCachePipeline(asyncEchoBrick, {
+      stateKey: STATE_KEY,
+      message: "second",
+    });
+
+    const secondCallPromise = reducePipeline(
+      secondCallPipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    deferred.resolve();
+
+    const target = { message: "first" };
+
+    await expect(firstCallPromise).resolves.toStrictEqual(target);
+    await expect(secondCallPromise).resolves.toStrictEqual(target);
+  });
+
+  it("memoizes error", async () => {
+    const pipeline = makeCachePipeline(asyncEchoBrick, {
+      stateKey: STATE_KEY,
+      message: "bar",
+    });
+
+    const requesterPromise = reducePipeline(
+      pipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    // Let the initial isFetching state be set
+    await tick();
+
+    const memoizedPromise = reducePipeline(
+      pipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    deferred.reject(new Error("Test Error"));
+
+    await expect(requesterPromise).rejects.toThrow("Test Error");
+    await expect(memoizedPromise).rejects.toThrow("Test Error");
+  });
+
+  it("forces fetch", async () => {
+    const firstCallPipeline = makeCachePipeline(asyncEchoBrick, {
+      stateKey: STATE_KEY,
+      message: "first",
+    });
+
+    const firstCallPromise = reducePipeline(
+      firstCallPipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    // Wait for the initial fetching state to be set
+    await tick();
+
+    const secondCallPipeline = makeCachePipeline(asyncEchoBrick, {
+      stateKey: STATE_KEY,
+      message: "second",
+      forceFetch: true,
+    });
+
+    const secondCallPromise = reducePipeline(
+      secondCallPipeline,
+      simpleInput({}),
+      reduceOptionsFactory("v3", { modComponentRef }),
+    );
+
+    // Wait for 2nd call to override the request id
+    await tick();
+
+    deferred.resolve();
+
+    try {
+      await firstCallPromise;
+    } catch (error) {
+      expect(error).toBeInstanceOf(ContextError);
+      expect((error as Error).cause).toBeInstanceOf(CancelError);
+    }
+
+    await expect(secondCallPromise).resolves.toStrictEqual({
+      message: "second",
+    });
+  });
 });
diff --git a/src/bricks/transformers/controlFlow/WithCache.ts b/src/bricks/transformers/controlFlow/WithCache.ts
@@ -420,24 +420,24 @@ export class WithCache extends TransformerABC {
       );
     }
 
-    if (isCacheVariableState(currentVariable) && currentVariable.isFetching) {
-      return this.waitForSettledRequest({
-        requestId: currentVariable.requestId,
-        args,
-        options,
-      });
-    }
+    if (!forceFetch && isCacheVariableState(currentVariable)) {
+      if (currentVariable.isFetching) {
+        return this.waitForSettledRequest({
+          requestId: currentVariable.requestId,
+          args,
+          options,
+        });
+      }
 
-    if (
-      !forceFetch &&
-      isCacheVariableState(currentVariable) &&
-      // Don't throw settled exceptions/rejections
-      currentVariable.isSuccess &&
-      // Only refetch if the cached value is still valid w.r.t. the TTL
-      (currentVariable.expiresAt == null ||
-        Date.now() < currentVariable.expiresAt)
-    ) {
-      return currentVariable.data;
+      if (
+        // Don't throw settled exceptions/rejections
+        currentVariable.isSuccess &&
+        // Only refetch if the cached value is still valid w.r.t. the TTL
+        (currentVariable.expiresAt == null ||
+          Date.now() < currentVariable.expiresAt)
+      ) {
+        return currentVariable.data;
+      }
     }
 
     return this.generateValue({

diff --git a/src/runtime/pipelineTests/pipelineTestHelpers.ts b/src/runtime/pipelineTests/pipelineTestHelpers.ts
@@ -109,7 +109,9 @@ class FeatureFlagBrick extends BrickABC {
 
 export class DeferredEchoBrick extends BrickABC {
   static BRICK_ID = validateRegistryId("test/deferred");
+
   readonly promiseOrFactory: Promise<unknown> | (() => Promise<unknown>);
+
   constructor(promiseOrFactory: Promise<unknown> | (() => Promise<unknown>)) {
     super(DeferredEchoBrick.BRICK_ID, "Deferred Brick");
     this.promiseOrFactory = promiseOrFactory;
@@ -131,7 +133,6 @@ export class DeferredEchoBrick extends BrickABC {
       await this.promiseOrFactory();
     }
 
-    await this.promiseOrFactory;
     return { message };
   }
 }