-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
Make it easier to debug close reason assertions #3659
Make it easier to debug close reason assertions #3659
Conversation
Reviewer's Guide by SourceryThis pull request enhances the debugging experience for WebSocket close reason assertions in tests. It replaces the Updated class diagram for WebSocketClient classesclassDiagram
class WebSocketClient {
+int close_code()
+bool closed()
+Optional~str~ close_reason
}
class AioHttpClient {
+int close_code()
+bool closed()
+Optional~str~ close_reason
}
class AsgiClient {
+int close_code()
+bool closed()
+Optional~str~ close_reason
}
class ChannelsClient {
+int close_code()
+bool closed()
+Optional~str~ close_reason
}
class LitestarClient {
+int close_code()
+bool closed()
+Optional~str~ close_reason
}
WebSocketClient <|-- AioHttpClient
WebSocketClient <|-- AsgiClient
WebSocketClient <|-- ChannelsClient
WebSocketClient <|-- LitestarClient
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @DoctorJohn - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3659 +/- ##
=======================================
Coverage 96.82% 96.82%
=======================================
Files 503 503
Lines 33409 33414 +5
Branches 5583 5587 +4
=======================================
+ Hits 32348 32354 +6
Misses 830 830
+ Partials 231 230 -1 |
CodSpeed Performance ReportMerging #3659 will not alter performanceComparing Summary
|
Description
Before this PR, Pytest could not display the diff between the expected and actual close reason because the assertion was hidden behind an
assert_reason
method.Now, Pytest shows a pretty diff, making it easier to debug unexpected WebSocket close reasons in tests.
Types of Changes
Summary by Sourcery
Enhance WebSocket test assertions by replacing the
assert_reason
method with aclose_reason
property, enabling Pytest to show detailed diffs for assertion errors, thus improving the debugging process.Enhancements:
assert_reason
method with aclose_reason
property to improve the readability of assertion errors in WebSocket tests.Tests:
close_reason
property instead of theassert_reason
method, allowing Pytest to display a detailed diff for easier debugging.