-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Show data as text when an tokio_test::io assert fails #6690
base: master
Are you sure you want to change the base?
Conversation
The asserts used to show the bytes as numbers, which is unnecessary hard to debug when the data is textual. It seems generally, though not universally more useful to show it as text when possible.
The CI fails because of an error in regex-syntax v0.8.4 but I don't think that has anything to do with me |
@@ -23,6 +23,7 @@ async-stream = "0.3.3" | |||
|
|||
bytes = "1.0.0" | |||
futures-core = "0.3.0" | |||
bstr = "1.6.2" |
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.
The MSRV failure is because you are adding this dependency.
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.
Thanks! I'll take a look if it is possible to downgrade bstr. Otherwise i'll just copy paste* the relevant formatting code or cobble something together myself.
That said, taking a step back, do you think this change is a good idea in the first place?
- Checking the license first, of course
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.
You should not add a new dependency for this. But you can put together something simple yourself.
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.
I think all looks good.
Kidding is just random user testing, sorry.
@@ -18,6 +18,7 @@ | |||
//! [`AsyncRead`]: tokio::io::AsyncRead | |||
//! [`AsyncWrite`]: tokio::io::AsyncWrite | |||
|
|||
use bstr::BStr; |
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.
Testing! thanks
Any updates on this PR? |
Motivation
The mock object mimics a reader or writer. If the code under test performs different reads or writes than expected, an assertion fails. Currently, the assertion shows the expected and actual data as a series of numbers representing bytes. If the protocol is textual, the numbers are quite hard to read.
Solution
Essentially wrap the data in a BStr. This shows text as-is and shows non-text bytes as \xAA\xBB etc.
The newly added struct
ShowBytes
contains a byte slice and a length limit. It has a PartialEq implementation and a fmt::Debug implementation. The Debug implementation shows only the given number of bytes, possibly followed by "plus N more bytes".The PartialEq implementation compares only the given number of bytes.
This is sufficient for the existing three
assert!
statements in the code.