Skip to content

refactor: optimize limitadorLimitsContain to accept pointer parameter#2104

Open
zyzzmohit wants to merge 1 commit into
Kuadrant:mainfrom
zyzzmohit:optimize-test-helper
Open

refactor: optimize limitadorLimitsContain to accept pointer parameter#2104
zyzzmohit wants to merge 1 commit into
Kuadrant:mainfrom
zyzzmohit:optimize-test-helper

Conversation

@zyzzmohit

@zyzzmohit zyzzmohit commented Jul 12, 2026

Copy link
Copy Markdown

Fixes #1891 and #2034. Changes the signature of limitadorLimitsContain to accept a pointer instead of copying the struct, optimizing performance and following Go best practices.

Summary by CodeRabbit

  • Tests
    • Updated rate-limit policy test assertions to use pointer-based limit data consistently.
    • Improved validation of limit presence and removal scenarios without changing end-user behaviour.

Fixes Kuadrant#1891

Signed-off-by: zyzzmohit <mohitray949@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1a82de69-c1d0-4a2b-8735-20486209cd0b

📥 Commits

Reviewing files that changed from the base of the PR and between 41ffdf0 and 97afb11.

📒 Files selected for processing (1)
  • tests/common/ratelimitpolicy/ratelimitpolicy_grpcroute_test.go

📝 Walkthrough

Walkthrough

The test helper now accepts a *Limitador, and all related Gateway, GRPCRoute, combined-policy, and deletion assertions pass the pointer directly.

Changes

Limitador helper refactor

Layer / File(s) Summary
Update helper and assertions
tests/common/ratelimitpolicy/ratelimitpolicy_grpcroute_test.go
limitadorLimitsContain accepts a pointer, and all presence and deletion checks use the pointer-based signature.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Poem

A bunny hops through tests with care,
No copied structs now fill the air.
Pointers pass from check to check,
Limits vanish when policies trek.
“Less copying!” cheers the hare.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the pointer-parameter refactor in the test helper.
Linked Issues check ✅ Passed The helper signature and call sites were updated as requested for [#1891].
Out of Scope Changes check ✅ Passed The changes stay within the test helper refactor and its call sites, with no unrelated edits.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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.

refactor: optimize test helper to accept pointer parameter

1 participant