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.
2026-07-26 09:52:11 +03:00 · 2026-06-02 22:39:13 -03:00
parent b80e6c26ac
commit fc77100c3f
4 changed files with 165 additions and 52 deletions
--- 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");
  });
 });
--- 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);
  });
 });
--- 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");
--- 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`);
  }