refactor(policy): refactor listNamespaces sort tests#3263
Conversation
added new helpers createSortTestNamespaces() and createNamedSortTestNamespaces() (timebased and name based applications respectively), and refactored existing tests to use the new helpers. Also added 3 new tests for true full test coverage
added Test_ListNamespacesRequest_Sort in namespaces_test.go. This test validates the proto message via its protovalidate restrictions. No sort, one item, and 2 items are tested.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves the testing suite for the ListNamespaces functionality. By introducing helper methods for test data generation, the codebase becomes cleaner and more modular. Additionally, the PR enhances test coverage for various sorting scenarios and adds necessary API-level validation to ensure request constraints are correctly enforced. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The tests were long and hard to read, With helpers now, they take the lead. Sorting logic, clear and bright, Ensures the code is running right. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the namespace integration tests by introducing helper methods for namespace creation, which reduces code duplication across sorting tests. It also adds new test cases to verify sorting by creation date (DESC), FQN (DESC), and update date (ASC), along with validation logic for the sort request parameters. Feedback is provided regarding the use of Go 1.22 features and the architectural placement of test helper methods to avoid the use of lint suppressions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors namespace sorting integration tests by introducing helper methods for deterministic namespace creation, adds tests for additional sort directions (ASC/DESC) and updated_at behavior, and adds a validator unit test for ListNamespacesRequest.Sort cardinality. Changes
Sequence Diagram(s)(omitted — changes are test additions/refactors without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
hardcoded 3 instead. createSortTestNamespaces wont be used outside of this API so it doesnt need to have a count param
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Proposed Changes
As per integration tests in listAttribute, this PR does some refactoring of existing tests to use helpers, adds fuller test coverage, and protovalidate tests.
Exact changes were as follows:
Checklist
Testing Instructions
Summary by CodeRabbit