-
Notifications
You must be signed in to change notification settings - Fork 24
fix(test): Check method when comparing matchers #430
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
Conversation
/autofix
|
📝 WalkthroughWalkthroughThe pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HttpMocker
participant MatcherStore as "HTTP Matcher Store (dict)"
Client->>HttpMocker: Call mock_request_method(new_matcher with method\n(e.g., GET))
HttpMocker->>MatcherStore: Retrieve matchers for GET
alt Matcher exists?
MatcherStore-->>HttpMocker: Existing matcher(s)
HttpMocker->>Client: Raise ValueError("Matcher already exists")
else
MatcherStore-->>HttpMocker: No conflict
HttpMocker->>MatcherStore: Append new_matcher to GET list
HttpMocker->>Client: Return success response
end
sequenceDiagram
participant Validator
participant HttpMocker
participant MatcherStore as "HTTP Matcher Store (dict)"
Validator->>HttpMocker: Invoke _validate_all_matchers_called
HttpMocker->>MatcherStore: Retrieve all matchers via _get_matchers
MatcherStore-->>HttpMocker: Return matcher list
HttpMocker->>Validator: Confirm all matchers have been called
Possibly related PRs
Suggested labels
Suggested reviewers
Would you like to add these reviewers to the PR, wdyt? Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Definitions (1)unit_tests/test/mock_http/test_mocker.py (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (11)
✨ Finishing Touches
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. 🪧 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 (
|
What
This test (and others but I'm using this one as an example) were failing after updating the CDK version. The reason is that the polling and delete requests are identical except from the method.
How
So maybe HttpRequest should have the method but it does not seem like an easy change for now as this would be a breaking change (i.e. requiring the method in the HttpRequest) and it also goes against how the HttpMocker was structure by having one method per method. Those are probably errors from the past and things we would do differently in the future but eh...!
So for now, I've just:
assert_number_of_calls
Summary by CodeRabbit
Refactor
Tests