-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: fix mocks #181
build: fix mocks #181
Conversation
Warning Rate limit exceeded@james-d-elliott has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (38)
WalkthroughThe changes involve modifications to several test and generation files within the codebase. The Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Cases
participant Handler as Access Request Handler
participant Storage as Storage Interface
participant Mock as Mock Objects
Test->>Mock: Setup mock expectations
Mock->>Handler: Call NewAccessRequest()
Handler->>Storage: Validate request
Storage-->>Handler: Return validation result
Handler-->>Test: Return access request result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
6263dae
to
28ed23e
Compare
28ed23e
to
ee48852
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
generate.go (1)
Line range hint
14-14
: Fix typo in RefreshTokenStrategy mock generationThere's a typo in the interface name:
ReyfreshTokenStrategy
should beRefreshTokenStrategy
.-//go:generate go run go.uber.org/mock/mockgen -package internal -destination internal/refresh_token_strategy.go authelia.com/provider/oauth2/handler/oauth2 ReyfreshTokenStrategy +//go:generate go run go.uber.org/mock/mockgen -package internal -destination internal/refresh_token_strategy.go authelia.com/provider/oauth2/handler/oauth2 RefreshTokenStrategy
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
access_request_handler_test.go
(8 hunks)generate-mocks.sh
(1 hunks)generate.go
(1 hunks)testing/mock/hash.go
(0 hunks)testing/mock/pkce_storage.go
(2 hunks)
💤 Files with no reviewable changes (1)
- testing/mock/hash.go
🔇 Additional comments (6)
testing/mock/pkce_storage.go (2)
2-2
: LGTM: Source comment accurately reflects mock generation command
The source comment and mock generation command are properly aligned, indicating that this is a mock of the Storage
interface from the pkce
package with a custom mock name MockPKCERequestStorage
.
Also applies to: 6-6
Line range hint 20-24
: LGTM: Mock struct maintains consistent naming
The mock struct definition and its documentation correctly reflect that this is a mock of the Storage
interface while maintaining the custom name MockPKCERequestStorage
for backward compatibility.
generate-mocks.sh (1)
20-20
: LGTM: Mock generation command follows established pattern
The updated command follows the same pattern used for other storage mocks (e.g., RFC8628Storage) by using the -mock_names
flag to maintain a consistent naming convention while referencing the correct interface.
generate.go (1)
22-22
: LGTM: Internal mock generation maintains consistency
The mock generation command properly aligns with the changes in generate-mocks.sh
, ensuring consistent mock naming while generating into the internal package.
access_request_handler_test.go (2)
29-29
: LGTM! Mock function signatures simplified consistently
The removal of the hasher
parameter from mock function signatures has been implemented consistently across all test cases while maintaining the same test coverage and functionality.
Also applies to: 52-54, 80-82, 110-114, 130-135, 149-154, 176-179, 219-219
Line range hint 1-450
: Verify complete removal of MockHasher
Let's verify that all references to MockHasher have been removed from the codebase.
✅ Verification successful
Based on the search results, I can see that there are no remaining references to MockHasher in the codebase. The comprehensive search through the mock directory and the entire codebase using both rg
and ast-grep
shows no occurrences of MockHasher.
MockHasher has been completely removed
The removal of MockHasher is complete and consistent across the codebase. The test cases in access_request_handler_test.go continue to function correctly as they rely on bcrypt's direct functionality rather than a mock hasher.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to MockHasher
# Search for any remaining references to MockHasher
echo "Searching for remaining MockHasher references..."
rg "MockHasher"
# Search for any remaining references to mock.MockHasher
echo "Searching for remaining mock.MockHasher references..."
rg "mock\.MockHasher"
Length of output: 263
Script:
#!/bin/bash
# Let's try a more comprehensive search for any hash-related mocks or references
# Search for hash-related mock types and interfaces
echo "Searching for hash-related mock types..."
ast-grep --pattern 'type Mock$_Hash$_ $_'
# Search for hash-related imports and uses
echo "Searching for hash-related imports and uses..."
rg -i "hash" --type go
# Search specifically in mock directory
echo "Searching in mock directory..."
fd . -t f -e go "testing/mock" --exec cat {}
Length of output: 72595
No description provided.