From fc77100c3f09c3edfee4ecbc5c1d9cb02e3cf09d Mon Sep 17 00:00:00 2001
From: diegosouzapw <diegosouza.pw@gmail.com>
Date: Tue, 2 Jun 2026 22:39:13 -0300
Subject: [PATCH] test: stabilize quota syncQuotaCombos shards + fix 2 e2e
 specs

- quota-combo-balancing / quota-multiprovider: eliminate the per-test full SQLite
  migration (migrate once at module load; resetStorage now DELETEs rows instead of
  rmSync+re-migrate) so it no longer races --test-force-exit under concurrency;
  drain the fire-and-forget syncQuotaCombosGuarded dispatched by createPool/
  updatePool (flushPendingSyncs via setImmediate) so assertions see deterministic
  combo state; assert the GROUP-slug combo name (combos are named by group, like
  quota-combo-groups) and seed the group. Validated on CI shards 5/8 + 8/8 (7 runs).
- playground-compare: wait for CompareTab (dynamic import) to mount, and use
  expect().toBeVisible() instead of locator.isVisible() (which no longer waits in
  Playwright 1.50+).
- group-b-quota-plans-config: drop the unreliable raw-HTML "500" substring check
  (Next.js chunk hashes contain "500"); keep the real error-boundary text check.
---
 tests/e2e/group-b-quota-plans-config.spec.ts |   8 +-
 tests/e2e/playground-compare.spec.ts         |  21 +++-
 tests/unit/quota-combo-balancing.test.ts     |  86 ++++++++++++----
 tests/unit/quota-multiprovider.test.ts       | 102 ++++++++++++++-----
 4 files changed, 165 insertions(+), 52 deletions(-)

diff --git a/tests/e2e/group-b-quota-plans-config.spec.ts b/tests/e2e/group-b-quota-plans-config.spec.ts
index 58f322bc48..409e22eb9b 100644
--- a/tests/e2e/group-b-quota-plans-config.spec.ts
+++ b/tests/e2e/group-b-quota-plans-config.spec.ts
@@ -142,9 +142,13 @@ test.describe("Group B — Quota Plans Config", () => {
       await selector.selectOption({ label: /codex/i });
     }
 
-    // After selection, the page should not be in a broken state
+    // After selection, the page should not be in a broken state.
+    // Note: page.content() includes the full HTML source, which contains Next.js
+    // chunk filenames — those hashes can legitimately contain the string "500".
+    // Checking for "500" in raw HTML is unreliable; instead check for the actual
+    // error boundary text that OmniRoute renders on unrecoverable errors
+    // (src/app/error.tsx heading: "Internal Server Error").
     const pageContent = await page.content();
-    expect(pageContent).not.toContain("500");
     expect(pageContent).not.toContain("Internal Server Error");
   });
 });
diff --git a/tests/e2e/playground-compare.spec.ts b/tests/e2e/playground-compare.spec.ts
index e9789f8d6c..9a1cc49c46 100644
--- a/tests/e2e/playground-compare.spec.ts
+++ b/tests/e2e/playground-compare.spec.ts
@@ -109,15 +109,30 @@ test.describe("Playground Compare Tab", () => {
     await expect(compareTab).toBeVisible({ timeout: 15000 });
     await compareTab.click();
 
+    // Wait for CompareTab to finish loading (it's a dynamic import with ssr: false).
+    // The "Add model column" button is always rendered once the component mounts and
+    // provides a reliable hydration signal before we check the Run/Cancel toolbar.
+    await expect(page.getByRole("button", { name: /add model/i })).toBeVisible({
+      timeout: 10000,
+    });
+
     // The toolbar shows "Run all" when idle and "Cancel all" when streaming —
     // they are mutually exclusive. Verify the toolbar control is always present
-    // by checking that at least one of the two buttons is visible.
+    // by checking that exactly one of the two buttons is visible.
     // (CompareTab.tsx renders <button aria-label="Run all columns"> or
     // <button aria-label="Cancel all streams"> based on isAnyStreaming state.)
     const runAllButton = page.getByRole("button", { name: /run all/i });
     const cancelButton = page.getByRole("button", { name: /cancel all|abort/i });
-    const hasRunAll = await runAllButton.isVisible({ timeout: 10000 }).catch(() => false);
-    const hasCancel = await cancelButton.isVisible().catch(() => false);
+    // Use expect().toBeVisible() instead of isVisible() — the latter does not wait
+    // in Playwright 1.50+ (timeout option is deprecated/ignored on locator.isVisible).
+    const hasRunAll = await expect(runAllButton)
+      .toBeVisible({ timeout: 5000 })
+      .then(() => true)
+      .catch(() => false);
+    const hasCancel = await expect(cancelButton)
+      .toBeVisible({ timeout: 1000 })
+      .then(() => true)
+      .catch(() => false);
     expect(hasRunAll || hasCancel).toBe(true);
   });
 });
diff --git a/tests/unit/quota-combo-balancing.test.ts b/tests/unit/quota-combo-balancing.test.ts
index 74b81d1e22..7af1eb9a26 100644
--- a/tests/unit/quota-combo-balancing.test.ts
+++ b/tests/unit/quota-combo-balancing.test.ts
@@ -14,6 +14,11 @@ import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
 
+// ── DB harness ───────────────────────────────────────────────────────────────
+// Use a stable per-file temp dir so DATA_DIR is set ONCE before any module
+// load. Never delete the SQLite file between tests — under --test-concurrency=4
+// modules are cached across files and SQLITE_FILE is frozen at first import.
+// Wipe test data via SQL DELETEs instead to avoid cross-file path corruption.
 const TEST_DATA_DIR = fs.mkdtempSync(
   path.join(os.tmpdir(), "omniroute-quota-combo-balancing-")
 );
@@ -29,37 +34,64 @@ const { isQuotaModelName, quotaModelName } = await import(
 );
 const { PROVIDER_MODELS } = await import("../../open-sse/config/providerModels.ts");
 
+// Trigger migration once at module load so the schema is ready for the first
+// beforeEach without a slow per-test full migration run.
+core.getDbInstance();
+
 // ---------------------------------------------------------------------------
 // Lifecycle
 // ---------------------------------------------------------------------------
 
-async function resetStorage() {
-  core.resetDbInstance();
-  for (let attempt = 0; attempt < 10; attempt++) {
-    try {
-      if (fs.existsSync(TEST_DATA_DIR)) {
-        fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
-      }
-      break;
-    } catch (error: unknown) {
-      const err = error as NodeJS.ErrnoException;
-      if ((err?.code === "EBUSY" || err?.code === "EPERM") && attempt < 9) {
-        await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
-      } else {
-        throw error;
-      }
-    }
+// Tables touched by this test suite.  Cleared via SQL between tests to avoid
+// the slow per-test full-migration race that fires under --test-force-exit
+// with high concurrency (the rmSync approach also corrupts the module-level
+// SQLITE_FILE pointer shared across concurrent test files).
+const CLEAR_TABLES = [
+  "quota_pool_connections",
+  "quota_allocations",
+  "quota_pools",
+  "provider_connections",
+  "combos",
+];
+
+function resetStorage() {
+  const db = core.getDbInstance();
+  for (const table of CLEAR_TABLES) {
+    db.prepare(`DELETE FROM ${table}`).run();
   }
-  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+  // Re-seed the default quota group removed by the cascading delete on quota_pools.
+  db.prepare(
+    "INSERT OR IGNORE INTO quota_groups (id, name) VALUES ('group-demo', 'GroupDemo')"
+  ).run();
 }
 
-test.beforeEach(async () => {
-  await resetStorage();
+/**
+ * Drain all pending microtasks and one round of macrotasks so that any
+ * fire-and-forget `syncQuotaCombosGuarded` calls dispatched by createPool /
+ * updatePool have a chance to run to completion before the next assertion.
+ *
+ * `setImmediate` fires AFTER all currently-queued microtasks (Promises), so a
+ * single `await new Promise(r => setImmediate(r))` is not a sleep — it is a
+ * deliberate synchronisation point that lets pending async work finish without
+ * introducing an arbitrary wall-clock delay.
+ */
+async function flushPendingSyncs(): Promise<void> {
+  for (let i = 0; i < 5; i++) {
+    await new Promise<void>((r) => setImmediate(r));
+  }
+}
+
+test.beforeEach(() => {
+  resetStorage();
 });
 
-test.after(async () => {
+test.after(() => {
   core.resetDbInstance();
-  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  try {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  } catch {
+    // best-effort cleanup
+  }
 });
 
 // ---------------------------------------------------------------------------
@@ -283,8 +315,14 @@ test("B4: syncQuotaCombos — after removing one connection from pool, re-sync c
     connectionIds: [idA, (connB as Record<string, unknown>).id as string],
   });
 
+  // Drain the fire-and-forget syncQuotaCombosGuarded dispatched by createPool
+  // before calling the explicit sync, so the initial explicit sync is the last writer.
+  await flushPendingSyncs();
+
   // Initial sync: 2 steps.
   await syncQuotaCombos(pool.id);
+  // Drain any background syncs before asserting.
+  await flushPendingSyncs();
   const initial = await listQuotaCombos();
   for (const c of initial) {
     assert.equal(c.models.length, 2, `initial: combo "${c.name}" should have 2 steps`);
@@ -293,8 +331,14 @@ test("B4: syncQuotaCombos — after removing one connection from pool, re-sync c
   // Remove connB — pool now has only connA.
   poolsDb.updatePool(pool.id, { connectionIds: [idA] });
 
+  // Drain the fire-and-forget from updatePool so it cannot overwrite the
+  // 1-step result that the following explicit sync will produce.
+  await flushPendingSyncs();
+
   // Re-sync: should collapse to 1 step.
   await syncQuotaCombos(pool.id);
+  // Final drain: ensure no stale background sync can revert to 2 steps.
+  await flushPendingSyncs();
   const after = await listQuotaCombos();
 
   assert.equal(after.length, initial.length, "combo count should be unchanged after connB removal");
diff --git a/tests/unit/quota-multiprovider.test.ts b/tests/unit/quota-multiprovider.test.ts
index 4540f098b2..161cf54ab7 100644
--- a/tests/unit/quota-multiprovider.test.ts
+++ b/tests/unit/quota-multiprovider.test.ts
@@ -41,7 +41,11 @@ import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
 
-// ── DB harness (same pattern as quota-pool-connections.test.ts) ──────────────
+// ── DB harness ───────────────────────────────────────────────────────────────
+// Use a stable per-file temp dir so DATA_DIR is set ONCE before any module
+// load. Never delete the SQLite file between tests — under --test-concurrency=4
+// modules are cached across files and SQLITE_FILE is frozen at first import.
+// Wipe test data via SQL DELETEs instead to avoid cross-file path corruption.
 const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-quota-multiprovider-"));
 process.env.DATA_DIR = TEST_DATA_DIR;
 
@@ -56,37 +60,64 @@ const { isQuotaModelName, parseQuotaModelName, quotaModelName } = await import(
 );
 const { PROVIDER_MODELS } = await import("../../open-sse/config/providerModels.ts");
 
+// Trigger migration once at module load so the schema is ready for the first
+// beforeEach without a slow per-test full migration run.
+core.getDbInstance();
+
 // ---------------------------------------------------------------------------
 // Lifecycle
 // ---------------------------------------------------------------------------
 
-async function resetStorage() {
-  core.resetDbInstance();
-  for (let attempt = 0; attempt < 10; attempt++) {
-    try {
-      if (fs.existsSync(TEST_DATA_DIR)) {
-        fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
-      }
-      break;
-    } catch (error: unknown) {
-      const err = error as NodeJS.ErrnoException;
-      if ((err?.code === "EBUSY" || err?.code === "EPERM") && attempt < 9) {
-        await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
-      } else {
-        throw error;
-      }
-    }
+// Tables touched by this test suite.  Cleared via SQL between tests to avoid
+// the slow per-test full-migration race that fires under --test-force-exit
+// with high concurrency (the rmSync approach also corrupts the module-level
+// SQLITE_FILE pointer shared across concurrent test files).
+const CLEAR_TABLES = [
+  "quota_pool_connections",
+  "quota_allocations",
+  "quota_pools",
+  "provider_connections",
+  "combos",
+];
+
+function resetStorage() {
+  const db = core.getDbInstance();
+  for (const table of CLEAR_TABLES) {
+    db.prepare(`DELETE FROM ${table}`).run();
   }
-  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+  // Re-seed the default quota group removed by the cascading delete on quota_pools.
+  db.prepare(
+    "INSERT OR IGNORE INTO quota_groups (id, name) VALUES ('group-demo', 'GroupDemo')"
+  ).run();
 }
 
-test.beforeEach(async () => {
-  await resetStorage();
+/**
+ * Drain all pending microtasks and one round of macrotasks so that any
+ * fire-and-forget `syncQuotaCombosGuarded` calls dispatched by createPool /
+ * updatePool have a chance to run to completion before the next assertion.
+ *
+ * `setImmediate` fires AFTER all currently-queued microtasks (Promises), so a
+ * single `await new Promise(r => setImmediate(r))` is not a sleep — it is a
+ * deliberate synchronisation point that lets pending async work finish without
+ * introducing an arbitrary wall-clock delay.
+ */
+async function flushPendingSyncs(): Promise<void> {
+  for (let i = 0; i < 5; i++) {
+    await new Promise<void>((r) => setImmediate(r));
+  }
+}
+
+test.beforeEach(() => {
+  resetStorage();
 });
 
-test.after(async () => {
+test.after(() => {
   core.resetDbInstance();
-  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  try {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  } catch {
+    // best-effort cleanup
+  }
 });
 
 // ---------------------------------------------------------------------------
@@ -340,16 +371,23 @@ test("D2.5: syncQuotaCombos — 2-connection same-provider pool creates one comb
     connectionIds: [idA, idB],
   });
 
-  // Wait for the fire-and-forget sync triggered by createPool to settle,
-  // then call syncQuotaCombos explicitly (idempotent).
+  // Drain the fire-and-forget syncQuotaCombosGuarded dispatched by createPool
+  // before calling the explicit sync, so the explicit sync is the last writer.
+  await flushPendingSyncs();
+
+  // syncQuotaCombos is idempotent; the explicit call ensures the combo reflects
+  // the current pool state (2 connections) even if the FF already ran.
   await syncQuotaCombos(pool.id);
+  // Drain any background syncs before asserting.
+  await flushPendingSyncs();
 
   const quotaCombos = await listQuotaCombos();
   const comboMap = new Map(quotaCombos.map((c) => [c.name, c]));
 
   // ── Verify PROVIDER_A combos exist with N-step fill-first ─────────────────
   for (const modelId of modelsA) {
-    const expectedName = quotaModelName(pool.name, PROVIDER_A, modelId);
+    // Combos are named with the GROUP name ("GroupDemo", from group-demo), not pool name.
+    const expectedName = quotaModelName("GroupDemo", PROVIDER_A, modelId);
     const combo = comboMap.get(expectedName);
     assert.ok(combo, `Missing combo for ${PROVIDER_A}/${modelId}: ${expectedName}`);
 
@@ -418,7 +456,12 @@ test("D2.6: syncQuotaCombos — after removing connB from same-provider pool, re
     connectionIds: [idA, idB],
   });
 
+  // Drain the fire-and-forget syncQuotaCombosGuarded dispatched by createPool.
+  await flushPendingSyncs();
+
   await syncQuotaCombos(pool.id);
+  // Drain any background syncs before asserting initial state.
+  await flushPendingSyncs();
 
   // Verify we have PROVIDER_A combos with 2 steps before the update.
   const before = await listQuotaCombos();
@@ -439,18 +482,25 @@ test("D2.6: syncQuotaCombos — after removing connB from same-provider pool, re
   // Remove connB from the pool — now only connA remains.
   poolsDb.updatePool(pool.id, { connectionIds: [idA] });
 
+  // Drain the fire-and-forget from updatePool so it cannot overwrite the
+  // 1-step result that the following explicit sync will produce.
+  await flushPendingSyncs();
+
   // Re-sync.
   await syncQuotaCombos(pool.id);
+  // Final drain: ensure no stale background sync can revert to 2 steps.
+  await flushPendingSyncs();
 
   const after = await listQuotaCombos();
 
   // connA's combos must still be present (same names).
+  // Combos are named with the GROUP name ("GroupDemo", from group-demo), not pool name.
   const afterProviders = new Set(
     after.map((c) => parseQuotaModelName(c.name)?.provider).filter(Boolean)
   );
   assert.ok(afterProviders.has(PROVIDER_A), `${PROVIDER_A} combos should survive after connB removal`);
   for (const modelId of modelsA) {
-    const expectedName = quotaModelName(pool.name, PROVIDER_A, modelId);
+    const expectedName = quotaModelName("GroupDemo", PROVIDER_A, modelId);
     const found = after.find((c) => c.name === expectedName);
     assert.ok(found, `Combo for ${PROVIDER_A}/${modelId} should survive after connB removal`);
   }