Skip to content

fix: harden production deploy smoke coverage #1523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sheygoodbai wants to merge 1 commit into
base: main
Choose a base branch
from
+78 −17
Open

fix: harden production deploy smoke coverage #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,16 @@ jobs:
if: needs.validate-deploy-request.outputs.run_smoke == 'true'
run: bunx playwright install --with-deps chromium

- name: Smoke test production HTTP
if: needs.validate-deploy-request.outputs.run_smoke == 'true'
run: bun run test:e2e:prod-http

- name: Write authenticated storage state
if: needs.validate-deploy-request.outputs.run_smoke == 'true' && env.PLAYWRIGHT_AUTH_STORAGE_STATE_JSON != ''
run: |
echo "$PLAYWRIGHT_AUTH_STORAGE_STATE_JSON" > "$RUNNER_TEMP/playwright-auth.json"
echo "PLAYWRIGHT_AUTH_STORAGE_STATE=$RUNNER_TEMP/playwright-auth.json" >> "$GITHUB_ENV"

- name: Smoke test production
- name: Smoke test production UI
if: needs.validate-deploy-request.outputs.run_smoke == 'true'
run: bunx playwright test e2e/menu-smoke.pw.test.ts e2e/upload-auth-smoke.pw.test.ts
run: bunx playwright test e2e/menu-smoke.pw.test.ts e2e/publish-entry-workflows.pw.test.ts e2e/upload-auth-smoke.pw.test.ts
84 changes: 70 additions & 14 deletions e2e/prod-http-smoke.e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Agent, setGlobalDispatcher } from "undici";
import { describe, expect, it } from "vitest";

const REQUEST_TIMEOUT_MS = 15_000;
const MAX_RATE_LIMIT_RETRIES = 3;
const MAX_RATE_LIMIT_WAIT_MS = 15_000;

try {
setGlobalDispatcher(
Expand Down Expand Up @@ -39,28 +41,82 @@ async function fetchWithTimeout(input: RequestInfo | URL, init?: RequestInit) {
}
}

function parsePositiveNumber(value: string | null) {
const parsed = Number(value);
return Number.isFinite(parsed) && parsed > 0 ? parsed : null;
}

function getRetryDelayMs(response: Response) {
const retryAfterSeconds = parsePositiveNumber(response.headers.get("Retry-After"));
if (retryAfterSeconds !== null) {
return Math.min(retryAfterSeconds * 1000, MAX_RATE_LIMIT_WAIT_MS);
}

const relativeResetSeconds = parsePositiveNumber(response.headers.get("RateLimit-Reset"));
if (relativeResetSeconds !== null) {
return Math.min(relativeResetSeconds * 1000, MAX_RATE_LIMIT_WAIT_MS);
}

const absoluteResetSeconds = parsePositiveNumber(response.headers.get("X-RateLimit-Reset"));
if (absoluteResetSeconds !== null) {
return Math.min(Math.max(absoluteResetSeconds * 1000 - Date.now(), 0), MAX_RATE_LIMIT_WAIT_MS);
}

return 1000;
}

async function fetchWithRetry(input: RequestInfo | URL, init?: RequestInit) {
let lastResponse: Response | null = null;

for (let attempt = 0; attempt < MAX_RATE_LIMIT_RETRIES; attempt += 1) {
const response = await fetchWithTimeout(input, init);
if (response.status !== 429) return response;

lastResponse = response;
if (attempt === MAX_RATE_LIMIT_RETRIES - 1) return response;

await new Promise((resolve) => setTimeout(resolve, getRetryDelayMs(response)));
}

if (!lastResponse) {
throw new Error("Expected a response while retrying rate-limited request.");
}

return lastResponse;
Comment on lines +81 to +85
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unreachable dead code after loop

The if (!lastResponse) guard and return lastResponse are unreachable. On the final iteration (attempt === MAX_RATE_LIMIT_RETRIES - 1), line 76 always returns the 429 response before the for loop can exit naturally, so execution never reaches line 81. The post-loop block can be safely removed.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: e2e/prod-http-smoke.e2e.test.ts
Line: 81-85

Comment:
**Unreachable dead code after loop**

The `if (!lastResponse)` guard and `return lastResponse` are unreachable. On the final iteration (`attempt === MAX_RATE_LIMIT_RETRIES - 1`), line 76 always `return`s the 429 response before the `for` loop can exit naturally, so execution never reaches line 81. The post-loop block can be safely removed.

How can I resolve this? If you propose a fix, please make it concise.

}

async function fetchHtml(pathname: string) {
const response = await fetchWithTimeout(new URL(pathname, getSiteBase()), {
const response = await fetchWithRetry(new URL(pathname, getSiteBase()), {
headers: { Accept: "text/html" },
});
expect(response.ok).toBe(true);
expect(response.headers.get("content-type")).toContain("text/html");
return response.text();
}

type SkillDetailResponse = {
skill: { slug: string; displayName: string; summary: string | null };
latestVersion: { version: string | null } | null;
owner: { handle: string | null };
};

let skillDetailPromise: Promise<SkillDetailResponse> | null = null;

async function fetchSkillDetail() {
const response = await fetchWithTimeout(
new URL(`/api/v1/skills/${getSkillSlug()}`, getSiteBase()),
{
headers: { Accept: "application/json" },
},
);
expect(response.ok).toBe(true);
return (await response.json()) as {
skill: { slug: string; displayName: string; summary: string | null };
latestVersion: { version: string | null } | null;
owner: { handle: string | null };
};
if (!skillDetailPromise) {
skillDetailPromise = (async () => {
const response = await fetchWithRetry(
new URL(`/api/v1/skills/${getSkillSlug()}`, getSiteBase()),
{
headers: { Accept: "application/json" },
},
);
expect(response.ok).toBe(true);
return (await response.json()) as SkillDetailResponse;
})();
}

return skillDetailPromise;
}

describe("prod http smoke", () => {
Expand Down Expand Up @@ -99,7 +155,7 @@ describe("prod http smoke", () => {
params.set("version", detail.latestVersion.version);
}

const response = await fetchWithTimeout(
const response = await fetchWithRetry(
new URL(`/og/skill.png?${params.toString()}`, getSiteBase()),
);

Expand Down
3 changes: 2 additions & 1 deletion e2e/publish-entry-workflows.pw.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ test("upload shows signed-out publish gate", async ({ page }) => {
const errors = trackRuntimeErrors(page);

await page.goto("/upload", { waitUntil: "domcontentloaded" });
await expect(page.getByText(/Sign in to upload a skill\./i)).toBeVisible();
await expect(page).toHaveURL(/\/publish-skill$/);
await expect(page.getByText("Sign in to publish a skill.")).toBeVisible();
await expectHealthyPage(page, errors);
});

Expand Down