Skip to content
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

Tokio test mock wait #5554

Merged
merged 6 commits into from
Mar 19, 2023
Merged

Conversation

devensiv
Copy link
Contributor

Clear waiting after completion of the current wait allowing subsequent waits to be registered instead of being discarded.

Motivation

The Mock in tokio-test::io won't work properly with multiple Action::Wait() actions.

Example using Builder::wait()

    let mut mock = Builder::new()
        .wait(Duration::from_secs(10))
        .read(b"hello ")
        .wait(Duration::from_secs(10))
        .read(b"world!")
        .build();

    let mut buf = [0; 256];

    let start = Instant::now();

    let n = mock.read(&mut buf).await.expect("read 1");
    let n = mock.read(&mut buf).await.expect("read 2");

    // `start.elapsed()` will be something around 10s instead of the expected 20s

The first commit in this PR includes two tests regarding the wait issue, one of which multiple_wait is equivalent to the example above and fails without the fix introduced in the second commit of this PR

Solution

The waiting option on Inner was set on the first Action::Wait() but never cleared, causing subsequent Action::Wait() to be discarded because the value of waiting was already in the past.

The fix clears self.waiting when the wait has finished

- wait(): test timing the read() test with an additional `Wait` action
- multiple_wait(): test timing the read() test with two additional `Wait` actions

! multiple_wait() fails
Clear `waiting` after completion of the current wait allowing subsequent waits to be registered
before they were being discarded
@Darksonn Darksonn added A-tokio-test Area: The tokio-test crate M-time Module: tokio/time labels Mar 18, 2023
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR. I have an improvement to your test, but otherwise it looks good.

tokio-test/tests/io.rs Outdated Show resolved Hide resolved
tokio-test/tests/io.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@Darksonn Darksonn enabled auto-merge (squash) March 19, 2023 11:31
@Darksonn Darksonn merged commit 4cd4b02 into tokio-rs:master Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-test Area: The tokio-test crate M-time Module: tokio/time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants