Skip to content

Optimize test helper to accept pointer parameter#2034

Open
RiteshGiram wants to merge 1 commit into
Kuadrant:mainfrom
RiteshGiram:test-limitador-pointer
Open

Optimize test helper to accept pointer parameter#2034
RiteshGiram wants to merge 1 commit into
Kuadrant:mainfrom
RiteshGiram:test-limitador-pointer

Conversation

@RiteshGiram

@RiteshGiram RiteshGiram commented Jun 6, 2026

Copy link
Copy Markdown

What problem does this PR solve?

Closes #1891

The limitadorLimitsContain() helper accepted a limitadorv1alpha1.Limitador value, causing the entire struct to be copied on each call.

What's changed and how does it work?

  • Updated limitadorLimitsContain() to accept *limitadorv1alpha1.Limitador.
  • Updated all call sites to pass the existing pointer directly.
  • Preserved existing behavior while avoiding unnecessary struct copies.

Testing

  • Existing tests pass
  • No functional behavior changes introduced

Summary by CodeRabbit

Release Notes

  • Tests
    • Updated GRPCRoute rate limit integration test implementation for improved consistency.

Signed-off-by: Ritesh Giram <riteshgiram9@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 6, 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: 9ef1a4be-adf8-4b9e-9731-9fbd9dc3935c

📥 Commits

Reviewing files that changed from the base of the PR and between b727d75 and de763f4.

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

📝 Walkthrough

Walkthrough

This PR refactors the GRPCRoute rate limit test to optimise the limitadorLimitsContain() helper function by changing it to accept a pointer parameter instead of a value copy. All five call sites are updated to pass the pointer directly without dereferencing.

Changes

Helper function pointer refactor

Layer / File(s) Summary
Helper function signature and call site updates
tests/common/ratelimitpolicy/ratelimitpolicy_grpcroute_test.go
limitadorLimitsContain() signature is updated to accept *limitadorv1alpha1.Limitador instead of a value copy. All five call sites in attachment and deletion test cases are updated to pass the pointer directly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit hops through test refactors with glee,
Pointers instead of copies—more efficient, you see!
No dereferencing dance, no value to clone,
The helper stays nimble, its signature grown. 🐇✨

🚥 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 accurately describes the main change: optimizing a test helper function to accept a pointer parameter instead of a value copy.
Linked Issues check ✅ Passed The pull request fully implements the requirements from issue #1891: changed function signature to accept pointer, updated all call sites, and preserved existing behaviour.
Out of Scope Changes check ✅ Passed All changes are directly related to the objective of optimizing the test helper function; no extraneous modifications are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ 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 and usage tips.

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