fix terminology tests on ci

Author: Seryiza

src/code-mapping/terminology-api-impl.ts src/code-mapping/loinc-terminology.tssrc/code-mapping/terminology-api-impl.ts renamed to src/code-mapping/loinc-terminology.ts

@@ -77,13 +77,19 @@ function extractDesignation(
 }
 
 export async function searchLoincCodes(
-  query: string
+  query: string,
+  // Injectable for tests — defaults to the real aidboxFetch. This is
+  // NOT for general dependency-injection use; it exists specifically
+  // because `mock.module`-based stubbing of this file's aidbox import
+  // is unreliable across Bun versions / test-ordering on CI (see
+  // test/unit/code-mapping/terminology-api.test.ts for details).
+  fetchFn: typeof aidboxFetch = aidboxFetch,
 ): Promise<LoincSearchResult[]> {
   const encodedQuery = encodeURIComponent(query);
   const path = `/fhir/ValueSet/$expand?url=http://loinc.org/vs&filter=${encodedQuery}&count=10`;
 
   const response = await withRetry(() =>
-    aidboxFetch<ValueSetExpansion>(path)
+    fetchFn<ValueSetExpansion>(path)
   );
 
   const contains = response.expansion?.contains || [];
@@ -104,13 +110,14 @@ export interface LoincValidationResult {
 }
 
 export async function validateLoincCode(
-  code: string
+  code: string,
+  fetchFn: typeof aidboxFetch = aidboxFetch,
 ): Promise<LoincValidationResult | null> {
   const path = `/fhir/CodeSystem/$lookup?system=http://loinc.org&code=${encodeURIComponent(code)}`;
 
   try {
     const response = await withRetry(() =>
-      aidboxFetch<CodeSystemLookupResult>(path)
+      fetchFn<CodeSystemLookupResult>(path)
     );
 
     const displayParam = response.parameter?.find((p) => p.name === "display");

src/code-mapping/terminology-api.ts

@@ -1,16 +1,18 @@
 /**
  * Public entry for Terminology API (LOINC search + validation).
  *
- * The real implementation lives in `./terminology-api-impl`. This file
- * is a thin re-export so tests that stub this module (via `mock.module`
- * — process-wide in Bun) don't also stub the implementation behind it:
- * `test/unit/code-mapping/terminology-api.test.ts` imports the
- * implementation directly (`./terminology-api-impl`) to sidestep that
- * stub. Everything else in the codebase imports from here.
+ * The real implementation lives in `./loinc-terminology`. Keeping the
+ * implementation under a dissimilar filename lets test code import the
+ * impl directly without colliding with any `mock.module` that stubs the
+ * public `terminology-api` path. The impl also exposes an optional
+ * `fetchFn` parameter on each function for tests that want to bypass
+ * module-level mocking entirely — see the docs in
+ * `src/code-mapping/loinc-terminology.ts` and
+ * `test/unit/code-mapping/terminology-api.test.ts`.
  */
 export {
   searchLoincCodes,
   validateLoincCode,
   type LoincSearchResult,
   type LoincValidationResult,
-} from "./terminology-api-impl";
+} from "./loinc-terminology";

test/unit/code-mapping/terminology-api.test.ts

@@ -1,25 +1,25 @@
 /**
  * Tests for Terminology API (LOINC search + validation).
  *
- * Imports from `terminology-api-impl` (not `terminology-api`) on purpose:
- * `test/unit/api/terminology-suggest.test.ts` installs a process-wide
- * `mock.module("../../../src/code-mapping/terminology-api", ...)` that
- * stubs `searchLoincCodes`. In Bun 1.3.12+, `mock.restore()` doesn't
- * reliably revert a file-level `mock.module` once the module is cached,
- * so any test file that runs after terminology-suggest.test.ts and
- * imports from the public `terminology-api` path gets the stub rather
- * than the real implementation. Importing the impl module directly
- * bypasses that stub.
+ * Why this file does NOT use `mock.module`:
+ * Other test files install process-wide `mock.module` stubs that can
+ * bleed into this file's imports. In Bun 1.3.12 the effect is
+ * test-order- and filesystem-dependent: CI (Ubuntu) consistently
+ * inherits `test/unit/api/terminology-suggest.test.ts`'s stub on
+ * `terminology-api` even when we import from a different path, and
+ * even with `afterAll(mock.restore)`. Re-registering `mock.module`
+ * per-test is also unreliable after the target module has been cached.
  *
- * Uses the mutable-factory mock pattern (same as
- * test/unit/ui/unmapped.test.ts): `mock.module` runs once at file load
- * with a factory whose `aidboxFetch` delegates to a mutable `fetchImpl`.
- * Tests swap `fetchImpl` per case instead of re-registering `mock.module`
- * per test — re-registering is unreliable under Bun 1.3.12+ once the
- * dependent module has been imported.
+ * Instead we test the impl module (`loinc-terminology.ts`) via its
+ * injectable `fetchFn` parameter — each test passes an in-place fake
+ * and asserts on its behavior. No module mocking, no cross-file
+ * pollution, no ordering sensitivity.
  */
 import { describe, test, expect, mock, beforeEach } from "bun:test";
-import * as realAidbox from "../../../src/aidbox";
+import {
+  searchLoincCodes,
+  validateLoincCode,
+} from "../../../src/code-mapping/loinc-terminology";
 
 const sampleValueSetExpansion = {
   expansion: {
@@ -60,33 +60,29 @@ const sampleCodeSystemLookup = {
   ],
 };
 
-// Mutable fetch implementation swapped per test.
-let fetchImpl: (path: string) => Promise<unknown> = () =>
-  Promise.reject(new Error("aidboxFetch not stubbed for this test"));
-
-mock.module("../../../src/aidbox", () => ({
-  ...realAidbox,
-  aidboxFetch: (path: string) => fetchImpl(path),
-}));
-
-const { searchLoincCodes, validateLoincCode } = await import(
-  "../../../src/code-mapping/terminology-api-impl"
-);
+/**
+ * Build a minimal fake aidboxFetch whose T-parameter is discarded —
+ * good enough for stubbing the two callsites inside loinc-terminology.
+ */
+function fakeFetch<T>(handler: (path: string) => Promise<unknown>) {
+  return (async (path: string) => handler(path)) as unknown as <U = T>(
+    path: string,
+  ) => Promise<U>;
+}
 
 describe("searchLoincCodes", () => {
+  let calledPath = "";
+
   beforeEach(() => {
-    fetchImpl = () => Promise.reject(new Error("fetchImpl not set in test"));
+    calledPath = "";
   });
 
   test("searches by text query and returns up to 10 results", async () => {
-    let calledPath = "";
     const spy = mock((path: string) => {
       calledPath = path;
       return Promise.resolve(sampleValueSetExpansion);
     });
-    fetchImpl = spy;
-
-    const results = await searchLoincCodes("potassium");
+    const results = await searchLoincCodes("potassium", fakeFetch(spy));
 
     expect(spy).toHaveBeenCalled();
     expect(calledPath).toContain("ValueSet/$expand");
@@ -96,21 +92,22 @@ describe("searchLoincCodes", () => {
   });
 
   test("searches by code (numeric-looking query)", async () => {
-    let calledPath = "";
-    fetchImpl = (path: string) => {
-      calledPath = path;
-      return Promise.resolve(sampleValueSetExpansion);
-    };
-
-    await searchLoincCodes("2823");
+    await searchLoincCodes(
+      "2823",
+      fakeFetch((path) => {
+        calledPath = path;
+        return Promise.resolve(sampleValueSetExpansion);
+      }),
+    );
 
     expect(calledPath).toContain("filter=2823");
   });
 
   test("returns results with code, display, and optional component/property/timing/scale", async () => {
-    fetchImpl = () => Promise.resolve(sampleValueSetExpansion);
-
-    const results = await searchLoincCodes("potassium");
+    const results = await searchLoincCodes(
+      "potassium",
+      fakeFetch(() => Promise.resolve(sampleValueSetExpansion)),
+    );
 
     expect(results[0]!.code).toBe("2823-3");
     expect(results[0]!.display).toBe("Potassium [Moles/volume] in Serum or Plasma");
@@ -121,9 +118,10 @@ describe("searchLoincCodes", () => {
   });
 
   test("handles results without designation (optional fields)", async () => {
-    fetchImpl = () => Promise.resolve(sampleValueSetExpansion);
-
-    const results = await searchLoincCodes("potassium");
+    const results = await searchLoincCodes(
+      "potassium",
+      fakeFetch(() => Promise.resolve(sampleValueSetExpansion)),
+    );
     const resultWithoutDesignation = results.find((r) => r.code === "39789-3");
 
     expect(resultWithoutDesignation).toBeDefined();
@@ -132,80 +130,86 @@ describe("searchLoincCodes", () => {
   });
 
   test("returns empty array when no results found", async () => {
-    fetchImpl = () => Promise.resolve({ expansion: { contains: [] } });
-
-    const results = await searchLoincCodes("nonexistent");
+    const results = await searchLoincCodes(
+      "nonexistent",
+      fakeFetch(() => Promise.resolve({ expansion: { contains: [] } })),
+    );
 
     expect(results).toEqual([]);
   });
 
   test("returns empty array when expansion.contains is undefined", async () => {
-    fetchImpl = () => Promise.resolve({ expansion: {} });
-
-    const results = await searchLoincCodes("test");
+    const results = await searchLoincCodes(
+      "test",
+      fakeFetch(() => Promise.resolve({ expansion: {} })),
+    );
 
     expect(results).toEqual([]);
   });
 
   test("retries on transient failure (2 retries)", async () => {
     let callCount = 0;
-    fetchImpl = () => {
-      callCount++;
-      if (callCount < 3) {
-        return Promise.reject(new Error("HTTP 503: Service Unavailable"));
-      }
-      return Promise.resolve(sampleValueSetExpansion);
-    };
-
-    const results = await searchLoincCodes("potassium");
+    const results = await searchLoincCodes(
+      "potassium",
+      fakeFetch(() => {
+        callCount++;
+        if (callCount < 3) {
+          return Promise.reject(new Error("HTTP 503: Service Unavailable"));
+        }
+        return Promise.resolve(sampleValueSetExpansion);
+      }),
+    );
 
     expect(callCount).toBe(3);
     expect(results.length).toBeGreaterThan(0);
   });
 
   test("throws after max retries exceeded", async () => {
-    fetchImpl = () => Promise.reject(new Error("HTTP 503: Service Unavailable"));
-
-    await expect(searchLoincCodes("potassium")).rejects.toThrow();
+    await expect(
+      searchLoincCodes(
+        "potassium",
+        fakeFetch(() => Promise.reject(new Error("HTTP 503: Service Unavailable"))),
+      ),
+    ).rejects.toThrow();
   });
 
   test("does not retry on 4xx errors", async () => {
     let callCount = 0;
-    fetchImpl = () => {
-      callCount++;
-      return Promise.reject(new Error("HTTP 400: Bad Request"));
-    };
-
-    await expect(searchLoincCodes("potassium")).rejects.toThrow("400");
+    await expect(
+      searchLoincCodes(
+        "potassium",
+        fakeFetch(() => {
+          callCount++;
+          return Promise.reject(new Error("HTTP 400: Bad Request"));
+        }),
+      ),
+    ).rejects.toThrow("400");
     expect(callCount).toBe(1);
   });
 
   test("encodes special characters in query", async () => {
-    let calledPath = "";
-    fetchImpl = (path: string) => {
-      calledPath = path;
-      return Promise.resolve({ expansion: {} });
-    };
-
-    await searchLoincCodes("test & query");
+    await searchLoincCodes(
+      "test & query",
+      fakeFetch((path) => {
+        calledPath = path;
+        return Promise.resolve({ expansion: {} });
+      }),
+    );
 
     expect(calledPath).toContain("filter=test%20%26%20query");
   });
 });
 
 describe("validateLoincCode", () => {
-  beforeEach(() => {
-    fetchImpl = () => Promise.reject(new Error("fetchImpl not set in test"));
-  });
-
   test("returns code details when valid", async () => {
     let calledPath = "";
-    fetchImpl = (path: string) => {
-      calledPath = path;
-      return Promise.resolve(sampleCodeSystemLookup);
-    };
-
-    const result = await validateLoincCode("2823-3");
+    const result = await validateLoincCode(
+      "2823-3",
+      fakeFetch((path) => {
+        calledPath = path;
+        return Promise.resolve(sampleCodeSystemLookup);
+      }),
+    );
 
     expect(calledPath).toContain("CodeSystem/$lookup");
     expect(calledPath).toContain("system=http://loinc.org");
@@ -216,32 +220,37 @@ describe("validateLoincCode", () => {
   });
 
   test("returns null for invalid code", async () => {
-    fetchImpl = () => Promise.reject(new Error("HTTP 404: Not Found"));
-
-    const result = await validateLoincCode("INVALID-CODE");
+    const result = await validateLoincCode(
+      "INVALID-CODE",
+      fakeFetch(() => Promise.reject(new Error("HTTP 404: Not Found"))),
+    );
 
     expect(result).toBeNull();
   });
 
   test("retries on transient failure", async () => {
     let callCount = 0;
-    fetchImpl = () => {
-      callCount++;
-      if (callCount < 2) {
-        return Promise.reject(new Error("HTTP 503: Service Unavailable"));
-      }
-      return Promise.resolve(sampleCodeSystemLookup);
-    };
-
-    const result = await validateLoincCode("2823-3");
+    const result = await validateLoincCode(
+      "2823-3",
+      fakeFetch(() => {
+        callCount++;
+        if (callCount < 2) {
+          return Promise.reject(new Error("HTTP 503: Service Unavailable"));
+        }
+        return Promise.resolve(sampleCodeSystemLookup);
+      }),
+    );
 
     expect(callCount).toBe(2);
     expect(result).toBeDefined();
   });
 
   test("throws after max retries on non-404 errors", async () => {
-    fetchImpl = () => Promise.reject(new Error("HTTP 500: Internal Server Error"));
-
-    await expect(validateLoincCode("2823-3")).rejects.toThrow("500");
+    await expect(
+      validateLoincCode(
+        "2823-3",
+        fakeFetch(() => Promise.reject(new Error("HTTP 500: Internal Server Error"))),
+      ),
+    ).rejects.toThrow("500");
   });
 });