Skip to content

Apply 15s timeout to subscription limit tests#521

Merged
jahooma merged 1 commit intomainfrom
jahooma/fix-block-grant-test
Apr 20, 2026
Merged

Apply 15s timeout to subscription limit tests#521
jahooma merged 1 commit intomainfrom
jahooma/fix-block-grant-test

Conversation

@jahooma
Copy link
Copy Markdown
Contributor

@jahooma jahooma commented Apr 20, 2026

Summary

  • The continues when block grant is created successfully test in completions.test.ts flaked and failed at 5001ms — Bun's default 5s timeout.
  • The file already defines SUBSCRIPTION_TEST_TIMEOUT_MS = 15000 with a comment noting these non-streaming fetch-path tests flake at the boundary, but only the .skipped tests had it applied.
  • Apply the timeout to the five active tests in the Subscription limit enforcement block.

Test plan

  • bun test src/app/api/v1/chat/completions/__tests__/completions.test.ts passes (24 pass, 4 skip, 0 fail)

🤖 Generated with Claude Code

Why: active tests in the Subscription limit enforcement block lacked the
bumped timeout that the file's own comment said was needed, so the block
grant test flaked at Bun's 5s boundary.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jahooma jahooma merged commit 0db97f3 into main Apr 20, 2026
11 checks passed
@jahooma jahooma deleted the jahooma/fix-block-grant-test branch April 20, 2026 01:57
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 20, 2026

Greptile Summary

This PR fixes test flakiness by applying the existing SUBSCRIPTION_TEST_TIMEOUT_MS = 15000 constant to the active (non-skipped) tests inside the Subscription limit enforcement describe block in completions.test.ts. The 5 s default Bun timeout was causing intermittent failures on loaded machines for tests that exercise the non-streaming fetch path.

  • The fix is correct for the 5 tests it targets.
  • Two additional active tests in the same block were missed: returns 402 for non-subscriber with 0 credits when ensureSubscriberBlockGrant returns null (line 1119) and does not call ensureSubscriberBlockGrant before validation passes (line 1155). Both use the same non-streaming code path and should also receive the extended timeout.
  • No production code is changed; this is a test-only PR.

Confidence Score: 4/5

Safe to merge, but two active tests in the same block still lack the 15 s timeout and will continue to flake on loaded machines.

The change is a no-risk test-only fix that resolves the reported flake for 5 tests. However, 2 of the 7 active tests in the block are missing the timeout, leaving them open to the exact same category of failure.

web/src/app/api/v1/chat/completions/tests/completions.test.ts — two active tests at lines 1119 and 1155 are missing SUBSCRIPTION_TEST_TIMEOUT_MS

Important Files Changed

Filename Overview
web/src/app/api/v1/chat/completions/tests/completions.test.ts Applied SUBSCRIPTION_TEST_TIMEOUT_MS (15 s) to 5 of the 7 active tests in the Subscription limit enforcement block; the two tests at lines 1119 and 1155 are still missing the timeout and remain susceptible to the same 5 s flake.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Subscription limit enforcement\ndescribe block\n(7 active tests)"] --> B["Tests with 15s timeout applied ✅\n(lines 804, 838, 885, 919, 950)"]
    A --> C["Tests still using default 5s timeout ⚠️\n(lines 1119, 1155)"]
    B --> D["SUBSCRIPTION_TEST_TIMEOUT_MS = 15000"]
    C --> E["Bun default = 5000ms\n→ potential flake on loaded machines"]
Loading

Comments Outside Diff (2)

  1. web/src/app/api/v1/chat/completions/__tests__/completions.test.ts, line 1153 (link)

    P1 Two active subscription tests missing timeout

    The PR description says it applies SUBSCRIPTION_TEST_TIMEOUT_MS to "the five active tests" in the Subscription limit enforcement block, but there are actually seven active (non-skipped) tests in that block. The two tests below were added to the block but are missing the timeout:

    • 'returns 402 for non-subscriber with 0 credits when ensureSubscriberBlockGrant returns null' (line 1153)
    • 'does not call ensureSubscriberBlockGrant before validation passes' (line 1191)

    Both use the non-streaming (stream: false) fetch path that the comment on SUBSCRIPTION_TEST_TIMEOUT_MS specifically calls out as having flaked at the 5 s boundary on loaded machines. Without the timeout, these two tests are still subject to the same flake risk.

    For the 'returns 402 for non-subscriber…' test closing brace:

  2. web/src/app/api/v1/chat/completions/__tests__/completions.test.ts, line 1191 (link)

    P1 Missing timeout on last active subscription test

    Same as above — this test closes without SUBSCRIPTION_TEST_TIMEOUT_MS, leaving it vulnerable to the same 5 s flake the PR is trying to fix.

Reviews (1): Last reviewed commit: "Apply 15s timeout to subscription limit ..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant