Skip to content

Add multiple iterations test for TokenRateLimitPolicy#749

Merged
averevki merged 1 commit intoKuadrant:mainfrom
emmaaroche:token-limit-multiple-iterations
Sep 12, 2025
Merged

Add multiple iterations test for TokenRateLimitPolicy#749
averevki merged 1 commit intoKuadrant:mainfrom
emmaaroche:token-limit-multiple-iterations

Conversation

@emmaaroche
Copy link
Copy Markdown
Contributor

@emmaaroche emmaaroche commented Sep 10, 2025

Description

This PR adds a test to verify that the TokenRateLimitPolicy correctly enforces its limits and properly resets after multiple iterations.

This PR also restructures the TRLP tests to separate shared and story-specific fixtures:

trlp/
  ├── user_story/
  │   ├── conftest.py
  │   └── test_trlp.py
  ├── conftest.py
  └── test_multiple_trlp_iterations.py

Related issue

@emmaaroche emmaaroche self-assigned this Sep 10, 2025
@emmaaroche emmaaroche added the Test case New test case label Sep 10, 2025
@emmaaroche emmaaroche requested a review from averevki September 10, 2025 13:21
@emmaaroche emmaaroche force-pushed the token-limit-multiple-iterations branch from 376b27a to 1dcec9b Compare September 10, 2025 13:22
@emmaaroche emmaaroche force-pushed the token-limit-multiple-iterations branch from 1dcec9b to e33b680 Compare September 11, 2025 11:03
@emmaaroche emmaaroche requested a review from averevki September 11, 2025 11:03
@emmaaroche emmaaroche force-pushed the token-limit-multiple-iterations branch from e33b680 to dd9d325 Compare September 11, 2025 15:12
@emmaaroche emmaaroche requested a review from averevki September 11, 2025 15:12
@emmaaroche emmaaroche force-pushed the token-limit-multiple-iterations branch 2 times, most recently from 1fdc714 to c2e4568 Compare September 11, 2025 15:31
Signed-off-by: emmaaroche <eroche@redhat.com>
@emmaaroche emmaaroche force-pushed the token-limit-multiple-iterations branch from c2e4568 to 3ce5634 Compare September 12, 2025 15:21
@emmaaroche emmaaroche requested a review from averevki September 12, 2025 15:21
Copy link
Copy Markdown
Contributor

@averevki averevki left a comment

Choose a reason for hiding this comment

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

>           assert (
                response.status_code == 429
            ), f"Iteration {i+1}: Expected 429 after {total_tokens} tokens, but got {response.status_code}"
E           AssertionError: Iteration 9: Expected 429 after 13 tokens, but got 200
E           assert 200 == 429
E            +  where 200 = <testsuite.httpx.Result object at 0x7fc2cc2798b0>.status_code

Failed for me one time randomly. Time to tokens relation might be a bit off for this test. But lets merge it and see how it runs on nightlies over this weekend. I'd prefer that and a smaller change later than to block this PR for a few more days. Good work! 😺

@averevki averevki merged commit 1242751 into Kuadrant:main Sep 12, 2025
3 checks passed
@emmaaroche emmaaroche deleted the token-limit-multiple-iterations branch September 15, 2025 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Test case New test case

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add multiple iterations test for TokenRateLimitPolicy

2 participants