mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 05:45:04 +03:00
playground-api-tab.test.tsx's SSE test always took the disabled-button branch (the fetch mock returned an empty model list) and asserted a tautology instead of exercising the SSE path it claims to verify. The test now selects a real model to enable Send, asserts it is actually enabled, and asserts the streamed SSE content reached the response editor. check-test-masking.mjs's tautology subcheck only compares base-vs-HEAD counts within a PR's own diff and no-ops entirely outside PR context (no GITHUB_BASE_SHA/REF) -- so a tautology merged once, or checked with a bare local run, stayed invisible forever after. Added an always-on absolute-floor scan (scanBareTautologies/countBareTautologies) over every tracked test file, scoped to the bare expect(true).toBe(true)/assert.equal(1,1) patterns that have zero legitimate uses in this codebase -- deliberately excluding assert.ok(true), which has ~15 pre-existing verified-legitimate try/catch-fallback uses and stays on the lenient diff-only path.
This commit is contained in:
committed by
GitHub
parent
f570960958
commit
ebdfe727a6
@@ -10,6 +10,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
||||
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
- **fix(test):** replace the bare `expect(true).toBe(true)` tautology in `playground-api-tab.test.tsx`'s SSE test and close the `check:test-masking` gap that let it slip through for a full cycle ([#6404](https://github.com/diegosouzapw/OmniRoute/issues/6404)) — a prior pass (#6548) had already swapped the literal to `expect(sendBtn).toBeDefined()`, but that stayed just as vacuous: the test's fetch mock returned an empty `/v1/models` list, so `ApiTab`'s Send button is always `disabled` (`!selectedModel`) and the SSE branch never runs — the "SSE infra is verified" comment was never true. The test now mocks a real model, drives the model `<select>` to enable Send, asserts `sendBtn.disabled === false` before clicking, and asserts the streamed SSE delta (`"Hello!"`) actually reached the response editor. Root cause on the detector side: `check-test-masking.mjs`'s tautology subcheck only compares base-vs-HEAD counts within a PR's own diff (`headExtTaut > baseExtTaut`) and no-ops locally when `GITHUB_BASE_SHA`/`GITHUB_BASE_REF` are unset ("sem base ref — pulando") — so a tautology merged once, or checked with a bare local run, was invisible forever after. Added a new always-on, PR-independent absolute-floor scan (`scanBareTautologies` + `countBareTautologies`) over every git-tracked test file for the bare `expect(true).toBe(true)` / `assert.equal(1,1)` / `assert.strictEqual(1,1)` patterns specifically (deliberately excluding `assert.ok(true)`, which has ~15 pre-existing verified-legitimate try/catch-fallback uses repo-wide and stays governed by the lenient diff-only subcheck) — verified zero pre-existing hits repo-wide once this file was fixed, so the new floor is safe to enforce unconditionally. Regression guard: `tests/unit/check-test-masking.test.ts` (new `scanBareTautologies`/`countBareTautologies` cases) + `tests/unit/ui/playground-api-tab.test.tsx`. (thanks @chirag127)
|
||||
- **fix(oauth):** Codex/ChatGPT (and every other OAuth provider) connection stays stuck showing "Auth Failed" even after a genuinely successful token refresh ([#6352](https://github.com/diegosouzapw/OmniRoute/issues/6352)) — `updateProviderCredentials()` (the shared `onPersist` callback for the manual "Refresh token" route, the reactive per-request refresh in `chat.ts`, and the Codex/Claude auth-file importers) correctly reused the stored `refresh_token`, persisted the new `access_token`, and replaced a rotated `refresh_token`, but never cleared the stale `testStatus`/`lastError*`/`errorCode` fields left over from a prior expired/invalid refresh or upstream 401/403 — only the separate background health-check sweep did that clearing. A successful refresh now resets `testStatus` to `"active"` and clears `lastError`, `lastErrorAt`, `lastErrorType`, `lastErrorSource`, and `errorCode` (an explicit `testStatus` from the caller still wins). Regression guard: `tests/unit/codex-oauth-refresh-persist-6352.test.ts`.
|
||||
- **fix(resilience):** `headroom` combo routing did not always select the Codex account with the most free quota ([#6379](https://github.com/diegosouzapw/OmniRoute/issues/6379)) — `orderTargetsByHeadroom` (`open-sse/services/combo/quotaStrategies.ts`) already loaded the per-connection DB snapshot (with decrypted credentials) via `expandTargetsByQuotaAwareConnections`, but discarded it before calling `getSaturation`; for Codex, `fetchCodexSaturation` forwards straight to `fetchCodexQuota(connectionId, connection)`, which needs `connection` (or a prior `registerCodexConnection()` call, which never happens before headroom ranking runs) to read `accessToken` — so it returned `null` for every candidate, saturation failed open to `0` across the board, and ranking fell back to the original combo order regardless of actual free quota. `getSaturation()` and the headroom `SaturationFetcher` seam now accept and thread the loaded connection snapshot through to `fetchCodexQuota`. Kilo's dup flag vs #5903 was a false positive — that issue is about session-sticky reset-aware/least-used selection, not headroom's Codex saturation lookup. Regression guard: `tests/unit/headroom-codex-quota-snapshot-6379.test.ts`. (thanks @eidoog)
|
||||
- **fix(providers):** custom models a provider actually has are no longer dropped from the Free Provider Rankings when both **"Configured only"** and **"Available only"** filters are applied ([#6368](https://github.com/diegosouzapw/OmniRoute/issues/6368)), follow-up to #6150 — `freeProviderRankings.ts::getProviderModels()` only ever walked the static `open-sse/config/providerRegistry.ts` catalog, so a user-added custom model (e.g. a Puter `claude-fable-5` model saved as "Claude Fable 5") never entered the candidate model list the ranking scores against, and could never survive the #6150 configured/available filters even when actually configured and available. It now additively merges the provider's custom models (`db/models.ts::getCustomModels`) into that candidate list via a new pure, de-duping `mergeProviderModels()` helper, before scoring/filtering runs — catalog free/paid filtering elsewhere is untouched. Regression guard: `tests/unit/free-provider-rankings-custom-models-6368.test.ts`. (thanks @shabeer)
|
||||
|
||||
@@ -59,6 +59,29 @@ export function countExtendedTautologies(src) {
|
||||
return count;
|
||||
}
|
||||
|
||||
/**
|
||||
* (#6404) Narrower sibling of countExtendedTautologies(), deliberately EXCLUDING
|
||||
* `assert.ok(true)`: that pattern is intentionally left to the lenient, diff-only,
|
||||
* new-occurrences-only subcheck 3 above, because ~15 pre-existing, verified-legitimate
|
||||
* uses already exist repo-wide (documented fallbacks like "expected to throw" /
|
||||
* "DB not available, expected" in try/catch branches) — an absolute, always-on scan
|
||||
* against all of them would be a mass false-positive, not a real signal.
|
||||
*
|
||||
* `expect(true).toBe(true)` / `assert.equal(1, 1)` / `assert.strictEqual(1, 1)` have
|
||||
* no such legitimate use anywhere in this codebase (verified zero pre-existing hits
|
||||
* after fixing #6404's playground-api-tab.test.tsx) — a genuinely bare, no-argument
|
||||
* tautology is never a deliberate pattern here, so it is safe to fail on ANY hit,
|
||||
* with or without a PR diff to compare against. See scanBareTautologies() below.
|
||||
*/
|
||||
export function countBareTautologies(src) {
|
||||
let count = 0;
|
||||
// expect(true).toBe(true)
|
||||
count += (src.match(/\bexpect\s*\(\s*true\s*\)\s*\.\s*toBe\s*\(\s*true\s*\)/g) || []).length;
|
||||
// assert.equal(1, 1) / assert.strictEqual(1, 1) — literal numeric identity
|
||||
count += (src.match(/\bassert\s*\.\s*(?:strict)?[Ee]qual\s*\(\s*1\s*,\s*1\s*\)/g) || []).length;
|
||||
return count;
|
||||
}
|
||||
|
||||
// ─── (6348) Subcheck 4: inline-reimplemented prod conditions (REPORT-ONLY) ───
|
||||
// A test that copies a conditional expression out of production code (instead of
|
||||
// importing and exercising the symbol that owns it) is the wrong-shape-contract-test
|
||||
@@ -325,6 +348,50 @@ export function evaluateMasking(perFile, assertReductionAllowlist = new Set()) {
|
||||
return flags;
|
||||
}
|
||||
|
||||
/**
|
||||
* (#6404) Absolute floor scan for bare tautologies (`expect(true).toBe(true)`,
|
||||
* `assert.equal(1, 1)` / `assert.strictEqual(1, 1)`), independent of PR diffing.
|
||||
*
|
||||
* The subcheck-3 diff logic above (`evaluateMasking`'s `headExtTaut > baseExtTaut`)
|
||||
* only fires for a tautology INTRODUCED within the current PR's own diff, and
|
||||
* `resolveBase()` returns `null` outside CI (no `GITHUB_BASE_SHA`/`GITHUB_BASE_REF`),
|
||||
* so a local `npm run check:test-masking` run silently no-ops — "sem base ref —
|
||||
* pulando" — regardless of what the tests actually contain. That is exactly how
|
||||
* #6404's `expect(true).toBe(true)` in `playground-api-tab.test.tsx` slipped through
|
||||
* for a full release cycle after merging once (the diff-only gate has nothing to
|
||||
* compare a pre-existing, already-merged tautology against, and local runs never
|
||||
* scan repo content at all). This scans every tracked test file's current content,
|
||||
* in or out of PR context, so a stray tautology can never hide once merged.
|
||||
*
|
||||
* Uses `countBareTautologies()` (not `countExtendedTautologies()`) — deliberately
|
||||
* excludes `assert.ok(true)`, which has ~15 verified-legitimate pre-existing uses
|
||||
* repo-wide and stays governed by the lenient, new-occurrence-only diff subcheck.
|
||||
*
|
||||
* `check-test-masking.test.ts` is excluded — its fixtures legitimately embed the
|
||||
* literal pattern as string literals to exercise the count* helpers themselves.
|
||||
*/
|
||||
export function scanBareTautologies(testFiles, readFile) {
|
||||
const read = readFile || ((f) => fs.readFileSync(f, "utf8"));
|
||||
const flags = [];
|
||||
for (const file of testFiles || []) {
|
||||
if (file.endsWith("check-test-masking.test.ts")) continue;
|
||||
let src;
|
||||
try {
|
||||
src = read(file);
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
const count = countBareTautologies(src);
|
||||
if (count > 0) {
|
||||
flags.push(
|
||||
`${file}: ${count} tautologia(s) pura(s) (expect(true).toBe(true) / assert.equal(1,1)) — ` +
|
||||
"substitua por um assert real do comportamento observável"
|
||||
);
|
||||
}
|
||||
}
|
||||
return flags;
|
||||
}
|
||||
|
||||
function git(args) {
|
||||
try {
|
||||
return execFileSync("git", args, { encoding: "utf8" });
|
||||
@@ -333,6 +400,15 @@ function git(args) {
|
||||
}
|
||||
}
|
||||
|
||||
/** All git-tracked test files (`.test.ts(x)`/`.spec.ts(x)`), repo-wide — used by the
|
||||
* absolute floor scan so it also covers files untouched by the current diff/PR. */
|
||||
function listTrackedTestFiles() {
|
||||
return git(["ls-files"])
|
||||
.split("\n")
|
||||
.map((s) => s.trim())
|
||||
.filter((f) => TEST_RE.test(f));
|
||||
}
|
||||
|
||||
function resolveBase() {
|
||||
if (process.env.GITHUB_BASE_SHA) return process.env.GITHUB_BASE_SHA;
|
||||
if (process.env.GITHUB_BASE_REF) return `origin/${process.env.GITHUB_BASE_REF}`;
|
||||
@@ -340,9 +416,34 @@ function resolveBase() {
|
||||
}
|
||||
|
||||
function main() {
|
||||
// (#6404) Absolute floor scan — runs unconditionally, PR or not, so a tautology
|
||||
// that is already merged into the base (and thus invisible to the diff-only
|
||||
// subchecks below) or a local pre-push run (which has no PR base to diff
|
||||
// against) still gets caught. See scanBareTautologies() doc comment.
|
||||
let bareTautAllowlist = new Set();
|
||||
try {
|
||||
const raw = JSON.parse(fs.readFileSync("config/quality/test-masking-allowlist.json", "utf8"));
|
||||
bareTautAllowlist = new Set(raw._bareTautologyAllowlist || []);
|
||||
} catch {
|
||||
// no allowlist file — treat as empty
|
||||
}
|
||||
const trackedTestFiles = listTrackedTestFiles().filter((f) => !bareTautAllowlist.has(f));
|
||||
const absoluteTautFlags = scanBareTautologies(trackedTestFiles);
|
||||
if (absoluteTautFlags.length) {
|
||||
console.error(
|
||||
`[test-masking] ${absoluteTautFlags.length} tautologia(s) pura(s) encontradas ` +
|
||||
`(scan absoluto — roda com ou sem contexto de PR):\n` +
|
||||
absoluteTautFlags.map((f) => " ✗ " + f).join("\n") +
|
||||
`\n → substitua por um assert real do comportamento observável.`
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const base = resolveBase();
|
||||
if (!base) {
|
||||
console.log("[test-masking] sem base ref (não é PR) — pulando.");
|
||||
console.log(
|
||||
"[test-masking] sem base ref (não é PR) — pulando checks de diff (scan absoluto de tautologias OK)."
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,8 @@ import {
|
||||
countTautologies,
|
||||
countSkips,
|
||||
countExtendedTautologies,
|
||||
countBareTautologies,
|
||||
scanBareTautologies,
|
||||
evaluateMasking,
|
||||
evaluateDeletedFiles,
|
||||
partitionDeletedRenamed,
|
||||
@@ -267,6 +269,75 @@ test("countExtendedTautologies: handles whitespace variants", () => {
|
||||
assert.equal(countExtendedTautologies(src), 2);
|
||||
});
|
||||
|
||||
// ─── #6404: bare tautologies (absolute floor scan, no PR diff needed) ───────
|
||||
|
||||
test("countBareTautologies: detects expect(true).toBe(true)", () => {
|
||||
const src = `expect(true).toBe(true);`;
|
||||
assert.equal(countBareTautologies(src), 1);
|
||||
});
|
||||
|
||||
test("countBareTautologies: detects assert.equal(1, 1) and assert.strictEqual(1, 1)", () => {
|
||||
assert.equal(countBareTautologies(`assert.equal(1, 1);`), 1);
|
||||
assert.equal(countBareTautologies(`assert.strictEqual(1, 1);`), 1);
|
||||
});
|
||||
|
||||
test("countBareTautologies: does NOT count assert.ok(true) (governed separately)", () => {
|
||||
// Unlike countExtendedTautologies, the absolute scan deliberately excludes
|
||||
// assert.ok(true) — it has many pre-existing, verified-legitimate uses
|
||||
// (try/catch fallbacks) across the codebase and stays on the lenient,
|
||||
// new-occurrence-only diff subcheck instead of an always-on floor.
|
||||
const src = `assert.ok(true);`;
|
||||
assert.equal(countBareTautologies(src), 0);
|
||||
});
|
||||
|
||||
test("countBareTautologies: returns 0 for real assertions", () => {
|
||||
const src = `
|
||||
expect(result).toBe(42);
|
||||
assert.equal(a, b);
|
||||
assert.ok(someCondition);
|
||||
`;
|
||||
assert.equal(countBareTautologies(src), 0);
|
||||
});
|
||||
|
||||
test("scanBareTautologies: flags a file containing the #6404 pattern (RED)", () => {
|
||||
const files = ["tests/unit/ui/playground-api-tab.test.tsx"];
|
||||
const read = () => `
|
||||
it("does something", async () => {
|
||||
// If button is disabled (no model selected), test still passes
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
`;
|
||||
const flags = scanBareTautologies(files, read);
|
||||
assert.equal(flags.length, 1);
|
||||
assert.match(flags[0], /playground-api-tab\.test\.tsx/);
|
||||
});
|
||||
|
||||
test("scanBareTautologies: a real assertion in the same spot is clean (GREEN)", () => {
|
||||
const files = ["tests/unit/ui/playground-api-tab.test.tsx"];
|
||||
const read = () => `
|
||||
it("sends SSE stream request and accumulates response", async () => {
|
||||
const responseEditor = editors[1];
|
||||
expect(responseEditor.value).toContain("Hello!");
|
||||
});
|
||||
`;
|
||||
const flags = scanBareTautologies(files, read);
|
||||
assert.deepEqual(flags, []);
|
||||
});
|
||||
|
||||
test("scanBareTautologies: excludes check-test-masking.test.ts itself", () => {
|
||||
const files = ["tests/unit/check-test-masking.test.ts"];
|
||||
const read = () => `expect(true).toBe(true);`;
|
||||
assert.deepEqual(scanBareTautologies(files, read), []);
|
||||
});
|
||||
|
||||
test("scanBareTautologies: skips unreadable files instead of throwing", () => {
|
||||
const files = ["tests/unit/does-not-exist.test.ts"];
|
||||
const read = () => {
|
||||
throw new Error("ENOENT");
|
||||
};
|
||||
assert.deepEqual(scanBareTautologies(files, read), []);
|
||||
});
|
||||
|
||||
test("evaluateMasking: new extended tautology is flagged", () => {
|
||||
const r = evaluateMasking([
|
||||
{
|
||||
|
||||
@@ -232,7 +232,10 @@ describe("ApiTab", () => {
|
||||
|
||||
mockFetch.mockImplementation(async (url: string) => {
|
||||
if (typeof url === "string" && url.includes("/v1/models")) {
|
||||
return new Response(JSON.stringify({ data: [] }), {
|
||||
// A non-empty model list is required so the Send button becomes
|
||||
// enabled (ApiTab disables it while `!selectedModel`) — an empty
|
||||
// list here would silently short-circuit the whole SSE path.
|
||||
return new Response(JSON.stringify({ data: [{ id: "openai/gpt-4" }] }), {
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
@@ -249,25 +252,40 @@ describe("ApiTab", () => {
|
||||
});
|
||||
|
||||
const el = renderApiTab();
|
||||
await waitFor(() => el.querySelector("select") !== null);
|
||||
// Selects render in order: [0] endpoint, [1] provider, [2] model, [3] account key.
|
||||
await waitFor(() => el.querySelectorAll("select").length >= 3);
|
||||
const modelSelect = el.querySelectorAll("select")[2] as HTMLSelectElement;
|
||||
await waitFor(() => modelSelect.options.length > 0);
|
||||
|
||||
act(() => {
|
||||
modelSelect.value = "openai/gpt-4";
|
||||
modelSelect.dispatchEvent(new Event("change", { bubbles: true }));
|
||||
});
|
||||
|
||||
// Find Send button
|
||||
const sendBtn = Array.from(el.querySelectorAll("button")).find(
|
||||
(b) => b.textContent?.includes("send")
|
||||
) as HTMLButtonElement | undefined;
|
||||
|
||||
if (sendBtn && !sendBtn.disabled) {
|
||||
await act(async () => {
|
||||
sendBtn.click();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
const editors = el.querySelectorAll("[data-testid='monaco-editor']");
|
||||
return editors.length >= 2 && (editors[1] as HTMLTextAreaElement).value !== "";
|
||||
}, 2000);
|
||||
}
|
||||
// SSE infra is verified by locating the send button; disabled state is fine.
|
||||
// Selecting a model + the auto-populated request body must enable Send —
|
||||
// otherwise the SSE path below never runs and this test would be vacuous.
|
||||
expect(sendBtn).toBeDefined();
|
||||
expect(sendBtn?.disabled).toBe(false);
|
||||
|
||||
await act(async () => {
|
||||
sendBtn?.click();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
const editors = el.querySelectorAll("[data-testid='monaco-editor']");
|
||||
return editors.length >= 2 && (editors[1] as HTMLTextAreaElement).value !== "";
|
||||
}, 2000);
|
||||
|
||||
// Assert the actual observable outcome: the streamed SSE delta content
|
||||
// reached the response editor, proving the SSE infra genuinely ran.
|
||||
const editors = el.querySelectorAll("[data-testid='monaco-editor']");
|
||||
const responseEditor = editors[1] as HTMLTextAreaElement;
|
||||
expect(responseEditor.value).toContain("Hello!");
|
||||
});
|
||||
|
||||
it("shows info banner", async () => {
|
||||
|
||||
Reference in New Issue
Block a user