-
-
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
Fix futures_io::AsyncSeek implementaion for Compat #5783
Fix futures_io::AsyncSeek implementaion for Compat #5783
Conversation
@@ -56,6 +58,7 @@ async-stream = "0.3.0" | |||
futures = "0.3.0" | |||
futures-test = "0.3.5" | |||
parking_lot = "0.12.0" | |||
tempfile = "3.1.0" |
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.
Adding a dependency to add a test feels overkill.
But the thing is, the test only fails for a tokio::fs::file::File
.
Using something like tokio::io::util::buf_writer::BufWriter
or std::io::cursor::Cursor
seems to be working fine on latest upstream/master
:
#[tokio::test]
async fn compat_seek() -> futures_util::io::Result<()> {
let mut cursor = Cursor::new(Vec::new()).compat_write();
cursor.write_all(&[0, 1, 2, 3, 4, 5]).await?;
cursor.write_all(&[6, 7]).await?;
assert_eq!(cursor.stream_position().await?, 8);
// Modify elements at position 2.
assert_eq!(cursor.seek(SeekFrom::Start(2)).await?, 2);
cursor.write_all(&[8, 9]).await?;
cursor.flush().await?;
// Verify we still have 8 elements
assert_eq!(cursor.seek(SeekFrom::End(0)).await?, 8);
// Seek back to the start to read and verify contents.
cursor.seek(SeekFrom::Start(0)).await?;
let mut buf = Vec::new();
let num_bytes = cursor.read_to_end(&mut buf).await?;
assert_eq!(&buf[..num_bytes], &[0, 1, 8, 9, 4, 5, 6, 7]);
Ok(())
}
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.
@Darksonn, is there some alternative way to test this? And would it more sense to skip adding one if there is none?
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.
This dependency already exists in the main Tokio crate, so adding it here doesn't actually add a new 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.
This PR upgrades a bunch of the esthri dependencies, and the main reason is that we are hoping it will fix a bug we encountered in Tokio: tokio-rs/tokio#5783
This PR upgrades a bunch of the esthri dependencies, and the main reason is that we are hoping it will fix a bug we encountered in Tokio: tokio-rs/tokio#5783
Motivation
Aims to address #5764.
Solution
Waiting for a seek operation to complete by calling
poll_complete
before callingstart_seek
.