Skip to content

Add token-based ratelimit IT scenarios for empty token-limit arrays#1235

Merged
renuka-fernando merged 3 commits intowso2:mainfrom
Tharsanan1:codex/token-ratelimit-empty-array-it
Feb 18, 2026
Merged

Add token-based ratelimit IT scenarios for empty token-limit arrays#1235
renuka-fernando merged 3 commits intowso2:mainfrom
Tharsanan1:codex/token-ratelimit-empty-array-it

Conversation

@Tharsanan1
Copy link
Contributor

@Tharsanan1 Tharsanan1 commented Feb 18, 2026

Summary

  • add integration test scenario for total-token-only limits when promptTokenLimits and completionTokenLimits are explicitly empty arrays
  • add integration test scenario for prompt-token-only limits when completion and total limits are empty arrays
  • add integration test scenario for completion-token-only limits when prompt and total limits are empty arrays
  • assert runtime enforcement with 429 after quota exhaustion in each case

Validation

  • IT_FEATURE_PATHS=features/token-based-ratelimit.feature make test (13/13 scenarios passed)

Summary by CodeRabbit

  • Tests
    • Expanded rate-limiting test coverage: new scenarios for token-based limits (including empty-limit variants), basic rate limiting, and response-cost overage to validate quota clamping, headers, and 200/429 behaviors.
  • Chores
    • Updated default gateway policy versions across multiple policies (internal defaults refreshed).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 18, 2026

Walkthrough

Adds extensive token-based rate-limit test scenarios and a new basic rate-limit feature, a ratelimit overage scenario, and updates many gateway default policy versions; no functional policy logic changes are made.

Changes

Cohort / File(s) Summary
Token-Based Rate Limiting tests
gateway/it/features/token-based-ratelimit.feature
Adds ~328 lines of Gherkin scenarios covering empty-limit permutations (prompt, completion, total), per-provider isolation, multi-quotas, rate-limit header validation, and corresponding provider templates/providers for each scenario.
Basic rate-limit tests
gateway/it/features/basic-ratelimit.feature
Adds a new feature file with scenarios validating basic quota enforcement, header emission, per-route vs API-scoped quotas, and 429/Retry-After behavior.
Rate limit overage scenario
gateway/it/features/ratelimit.feature
Adds a scenario asserting response-cost overage clamps quota to zero and subsequent 429 blocking.
Gateway default policies (version bumps / minor description edits)
gateway/gateway-controller/default-policies/...
advanced-ratelimit.yaml, api-key-auth.yaml, azure-content-safety-content-moderation.yaml, basic-auth.yaml, basic-ratelimit.yaml, cors.yaml, jwt-auth.yaml, model-round-robin.yaml, pii-masking-regex.yaml, prompt-decorator.yaml, prompt-template.yaml, remove-headers.yaml, semantic-cache.yaml, semantic-prompt-guard.yaml, set-headers.yaml, token-based-ratelimit.yaml
Updated policy version fields across many default policies; one file (pii-masking-regex.yaml) expands description. No behavioral or schema changes reported.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through tests with nimble feet,

Empty tokens, quotas to meet,
Providers, headers, 429 in sight,
I stitched the scenarios neat and tight,
A tiny rabbit cheering every test's light. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It only provides a summary and validation but is missing most required template sections: Purpose, Goals, Approach, User stories, Documentation, Automation tests, Security checks, Samples, Related PRs, and Test environment. Complete the PR description by filling in all required template sections, including Purpose/Goals/Approach, documentation impact, detailed automation test coverage, security checks, and test environment information.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: adding integration test scenarios for token-based rate limits with empty token-limit arrays.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
gateway/it/features/token-based-ratelimit.feature (1)

1389-1389: Consider adding a response body assertion after 429 for consistency.

Scenarios at lines 152–153, 692–693, and 997 verify And the response body should contain "Rate limit exceeded" after each 429. All three new scenarios skip this step. Adding it would close the assertion gap and make the failure mode fully verifiable beyond just the status code.

✏️ Suggested addition (same pattern for all three scenarios)
    Then the response status code should be 429
+   And the response body should contain "Rate limit exceeded"

Also applies to: 1498-1498, 1607-1607

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gateway/it/features/token-based-ratelimit.feature` at line 1389, Add a
response body assertion after the existing "Then the response status code should
be 429" step in the feature scenarios that currently stop at the status check
(token-based-ratelimit.feature occurrences around the shown diff); append the
step 'And the response body should contain "Rate limit exceeded"' immediately
after the 429 assertion in each of the affected scenarios so the failure mode is
verified by both status and body content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@gateway/it/features/token-based-ratelimit.feature`:
- Line 1389: Add a response body assertion after the existing "Then the response
status code should be 429" step in the feature scenarios that currently stop at
the status check (token-based-ratelimit.feature occurrences around the shown
diff); append the step 'And the response body should contain "Rate limit
exceeded"' immediately after the 429 assertion in each of the affected scenarios
so the failure mode is verified by both status and body content.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
gateway/it/features/basic-ratelimit.feature (1)

241-318: Commented-out scope-based scenario — consider tracking as a follow-up.

This commented-out scenario tests API-level vs route-level policy scope interaction, which is a valuable edge case. If this is intentionally deferred, consider adding a tracking issue or TODO reference so it doesn't get lost.

Would you like me to open an issue to track enabling this scope-based rate limit scenario?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gateway/it/features/basic-ratelimit.feature` around lines 241 - 318, The
commented-out scenario "Rate limit scope based on policy attachment level" is a
valuable test left disabled; either re-enable it or add a tracking TODO/issue so
it isn't lost. Create a short tracking ticket (e.g., "enable scope-based rate
limit scenario") and then add a one-line TODO above the commented block
referencing that ticket ID (or add a comment with the issue URL and the scenario
title) so reviewers can find and prioritize it later; alternatively, if you plan
to enable now, uncomment the scenario and ensure test data and limits in the
scenario content match current policy behavior before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@gateway/it/features/basic-ratelimit.feature`:
- Around line 241-318: The commented-out scenario "Rate limit scope based on
policy attachment level" is a valuable test left disabled; either re-enable it
or add a tracking TODO/issue so it isn't lost. Create a short tracking ticket
(e.g., "enable scope-based rate limit scenario") and then add a one-line TODO
above the commented block referencing that ticket ID (or add a comment with the
issue URL and the scenario title) so reviewers can find and prioritize it later;
alternatively, if you plan to enable now, uncomment the scenario and ensure test
data and limits in the scenario content match current policy behavior before
committing.

@@ -1,5 +1,5 @@
name: advanced-ratelimit
version: v0.1.4
version: v0.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We are going with 0.3.0 right?
@Krishanx92 @Tharsanan1

@renuka-fernando renuka-fernando merged commit 31f9ebf into wso2:main Feb 18, 2026
5 checks passed
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.

3 participants

Comments