Skip to content

refactor: update budget exhaustion test description and remove obsolete test case #1769

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

Merged
Artuomka merged 1 commit into from
May 14, 2026
Merged

refactor: update budget exhaustion test description and remove obsolete test case #1769

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
72 changes: 7 additions & 65 deletions backend/test/ava-tests/saas-tests/saas-postgres-proxy-e2e.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,71 +558,13 @@ test.serial('should report usage metrics to mock API after queries through proxy
}
});

// ─── Budget exhaustion via rocketadmin API ──────────────────────────────────
//
// Drives the token bucket to negative balance under the TEST_TINY_PLAN (a
// deliberately tiny 2s/hour budget used only for tests). Uses supertest —
// each /connection/tables call fans out into several metadata queries, which
// is more than enough to exhaust 2 seconds of cumulative query time across a
// few calls. The next API call should surface the proxy's 53400 rejection.

test.serial('should reject queries once query-time budget is exhausted', async (t) => {
if (maybeSkip(t)) return;
t.timeout(60000);

const probe = await getUsageReports();
if (!Array.isArray(probe)) {
t.pass('skipped: proxy-mock-api does not expose test endpoints (rebuild required)');
return;
}

await setSubscriptionLevel('TEST_TINY_PLAN');
try {
const firstUserToken = (await registerUserAndReturnUserInfo(app)).token;
const proxyConnectionDto = createProxyConnectionDto();

const createConnectionResponse = await request(app.getHttpServer())
.post('/connection')
.send(proxyConnectionDto)
.set('Cookie', firstUserToken)
.set('Content-Type', 'application/json')
.set('Accept', 'application/json');
t.is(createConnectionResponse.status, 201);
const createConnectionRO = JSON.parse(createConnectionResponse.text);

// Burn through the 2-second budget by repeatedly listing tables. Each
// call executes several introspection queries; within a handful of calls
// the post-consume balance goes negative and subsequent calls hit the
// pre-query CheckBudget rejection.
let rejected = false;
let lastStatus = 0;
let lastBody = '';
for (let i = 0; i < 30; i++) {
const resp = await request(app.getHttpServer())
.get(`/connection/tables/${createConnectionRO.id}`)
.set('Cookie', firstUserToken)
.set('Accept', 'application/json');
lastStatus = resp.status;
lastBody = resp.text || '';
if (resp.status !== 200) {
rejected = true;
break;
}
}

t.true(
rejected,
`expected a 30-call burst under TEST_TINY_PLAN to hit a budget rejection; last status=${lastStatus}`,
);
t.regex(
lastBody,
/budget|exceeded|plan|53400/i,
`rejection response should mention budget/plan, got status=${lastStatus} body=${lastBody.slice(0, 200)}`,
);
} finally {
await setSubscriptionLevel('TEAM_PLAN');
}
});
// Budget-exhaustion behaviour is "queue, don't fail": queries past the
// token-bucket budget park on `Limiter.WaitForBudget` until tokens refill,
// rather than returning a 53400. End-to-end verification of that wait is
// covered by `postgres-proxy/internal/ratelimit/bucket_test.go`
// (TestLimiter_WaitForBudget_*), which can simulate refill in milliseconds.
// An equivalent rocketadmin e2e check would need a custom plan with a refill
// rate that completes inside the AVA timeout — not worth the moving pieces.
Comment on lines +562 to +567
Comment on lines +561 to +567
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify the referenced Go test file and test cases exist

# Check if the referenced test file exists
if fd -t f '^bucket_test\.go$' postgres-proxy/internal/ratelimit 2>/dev/null | grep -q .; then
  echo "✓ Found bucket_test.go"
  
  # Search for TestLimiter_WaitForBudget test cases
  echo -e "\n=== TestLimiter_WaitForBudget test cases ==="
  rg -n "func.*TestLimiter_WaitForBudget" postgres-proxy/internal/ratelimit/bucket_test.go -A 3
  
  # Check if tests verify queue behavior (not rejection)
  echo -e "\n=== Checking for queue/wait behavior verification ==="
  rg -n -C 3 "WaitForBudget|queue|wait.*budget|refill" postgres-proxy/internal/ratelimit/bucket_test.go | head -30
else
  echo "✗ bucket_test.go not found in postgres-proxy/internal/ratelimit/"
  echo "The referenced test file may not exist or the path may be incorrect"
fi

Repository: rocket-admin/rocketadmin

Length of output: 201

🏁 Script executed:

#!/bin/bash
# Search for bucket_test.go anywhere in the codebase
echo "=== Searching for bucket_test.go in entire repository ==="
fd -t f "bucket_test\.go" 2>/dev/null

echo -e "\n=== Searching for TestLimiter_WaitForBudget in entire repository ==="
rg "TestLimiter_WaitForBudget" 2>/dev/null | head -10

echo -e "\n=== Checking if postgres-proxy directory exists and its structure ==="
fd -t d "postgres-proxy" 2>/dev/null | head -10

echo -e "\n=== Checking for bucket-related test files ==="
fd -t f "bucket.*test" 2>/dev/null

echo -e "\n=== Checking for ratelimit-related test files ==="
fd -t f ".*ratelimit.*test" 2>/dev/null

Repository: rocket-admin/rocketadmin

Length of output: 509

Verify that the referenced Go tests actually exist before removing e2e verification.

The comment justifies removing this e2e test based on coverage from postgres-proxy/internal/ratelimit/bucket_test.go (specifically TestLimiter_WaitForBudget_* tests). However, neither this file nor these tests exist anywhere in the repository.

Before this test removal is valid, confirm:

  1. Whether the referenced tests exist in an external/dependency repository
  2. Whether the path or test names are incorrect
  3. If not: that equivalent coverage exists elsewhere before removing the e2e verification

Removing end-to-end test coverage without verifying equivalent unit test coverage exists creates a critical gap.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/test/ava-tests/saas-tests/saas-postgres-proxy-e2e.test.ts` around
lines 561 - 567, Confirm whether the referenced Go tests actually exist before
removing the e2e check: search the repo and any git submodules/dependencies for
the path postgres-proxy/internal/ratelimit/bucket_test.go and for symbols named
TestLimiter_WaitForBudget_*; if they are missing, either (a) update the comment
in saas-postgres-proxy-e2e.test.ts to point to the correct path/test names or to
the actual unit tests that provide equivalent coverage, or (b) keep (or restore)
the e2e verification and add a new unit test that exercises the
Limiter.WaitForBudget behavior (or reference the external repo where the tests
live and add verification steps to pull that dependency); make the change by
editing the comment or adding the tests so there is explicit, discoverable proof
of equivalent coverage.


// ─── Frozen plan / connection rejection test ────────────────────────────────
//
Expand Down
Loading