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

(WIP): Adding Stream mock to tokio-test #4463

Closed
wants to merge 18 commits into from

Conversation

Grubba27
Copy link
Contributor

@Grubba27 Grubba27 commented Feb 2, 2022

Motivation

This issue relates to #4106

Solution

Test the Stream with the mock as sugested in issue thread

This is a draft. I got stuck creating the solution, created this PR in hope that I could get some help

@Grubba27 Grubba27 changed the title (WIP): Draft of PR (WIP): Add Stream mock to tokio-test Feb 2, 2022
@Grubba27 Grubba27 changed the title (WIP): Add Stream mock to tokio-test (WIP): Adding Stream mock to tokio-test Feb 2, 2022
@Darksonn Darksonn added the A-tokio-test Area: The tokio-test crate label Feb 6, 2022
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.

Let me know if there's any other help you need.

Comment on lines 56 to 60
#[derive(Debug, Clone, Default)]
pub struct StreamBuilder {
// Sequence of actions for the Mock to take
actions: VecDeque<Action>,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should go in its own file.

Comment on lines 158 to 161
/// call next value ?
pub fn poll_next(&mut self, buf: &[u8]) {
/// TODO
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not be able to call poll_next or next on the builder. You have to actually build it first. The io builder also doesn't implement AsyncRead or AsyncWrite.

@Grubba27
Copy link
Contributor Author

Grubba27 commented Feb 8, 2022

Let me know if there's any other help you need.

Sure! I'm trying to get to know Tokio code base and its patterns.
Should I create also a file just for io_stream ? also I'm not sure how I would implement stream in Mock and how I would test it if not by let val = stream.next().await as stream here would be building mock as stream ? like in the return of mock -> impl Stream<value = &str>

Comment on lines 215 to 217
fn read(&mut self, dst: &mut ReadBuf<'_>) -> io::Result<()> {
match self.action() {
Some(&mut Action::Read(ref mut data)) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You are implementing a Stream. Streams do not have a read or write method. Instead, you should make an impl Stream for Mock block so that Mock becomes a stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now I see where I need to go. In impl Steam for Mock i should use :

    type Item = String; // like this because my mock would only use Strings as their items ?

then i would do something like pop from an array or like a loop from async read or async write within poll_next ? like this:

    fn poll_next(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
        // TODO
        // Pop item from array who could i iterate self? or do a  while loop like in AsyncRead /AsyncWrite ?
        Poll::Pending
    }

Comment on lines 50 to 59
#[derive(Debug, Clone, Default)]
pub struct Builder {
// Sequence of actions for the Mock to take
actions: VecDeque<Action>,
}
#[derive(Debug, Clone, Default)]
pub struct StreamBuilder {
// Sequence of actions for the Mock to take
actions: VecDeque<Action>,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there two builders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry :/ I was sleppy but I did remove these two now

@Darksonn
Copy link
Contributor

Darksonn commented Feb 8, 2022

also I'm not sure how I would implement stream in Mock and how I would test it if not by let val = stream.next().await as stream here would be building mock as stream

You would test it by calling next on the Mock value as you suggested.

Comment on lines 274 to 283
impl Stream for Mock {
type Item = String;

fn poll_next(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
// TODO
// Pop item from array who could i iterate self? or do a while loop like in AsyncRead /AsyncWrite ?
Poll::Pending
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically you need to return the next action from the list of actions, but if that action is a sleep, then you need to sleep instead. And if you are polled during a sleep, you need to keep sleeping until the sleep completes.

The other mock type implements this by having a loop, where the loop first checks if its currently sleeping, then if the ready! macro lets it get past the sleep (i.e. if the sleep has completed), then it pops the next item. If the next item is a sleep, then it sets up that new sleep and goes around the loop. Otherwise it returns the item.

Comment on lines 240 to 242
match ready!(self.inner.poll_action(_cx)) {
None => {
return Poll::Pending;
Copy link
Contributor

Choose a reason for hiding this comment

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

If poll_action returns None, then isn't that because we are at the end and the stream is complete? Then you should return Poll::Ready(None) instead of Poll::Pending.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay now I see what I was doing wrong!!
I don't have any ideia why my mock.next.await is aways returning None for me. perhaps because of the cx ?
Maybe I should use a mock Context too for using pool_next(cx) ? or keep with next() but change the way that I call Action it at io_steram ?

@Darksonn
Copy link
Contributor

Sorry, this PR appears to have gotten lost. Do you want to finish it? There are some CI failures.

@Grubba27
Copy link
Contributor Author

Sorry, this PR appears to have gotten lost. Do you want to finish it? There are some CI failures.

Yeah, I Would love to finish. It has been some time since I haven't touched Rust but I can give it another try again guess this last note can help me :

I don't have any ideia why my mock.next.await is aways returning None for me. perhaps because of the cx ? Maybe I should use a mock Context too for using pool_next(cx) ? or keep with next() but change the way that I call Action it at io_steram ?

Comment on lines 278 to 302
impl Stream for Mock {
type Item = String;

fn poll_next(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {

loop {
if let Some(ref mut sleep) = self.inner.sleep {
ready!(Pin::new(sleep).poll(_cx));
}


// If a sleep is set, it has already fired
self.inner.sleep = None;
match ready!(self.inner.poll_action(_cx)) {
Some(action) => {
self.inner.actions.push_back(action);
continue;
}
None => {
return Poll::Ready(None);
}
}
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

The poll_next function never returns Poll::Ready(Some(...)) anywhere, so the stream never produces any items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OMG!! I think I got it(copilot also helped me understand)
Guess the only thing that is missing is

                        let now = Instant::now();

                        if now < until {
                            break;
                        }
                    } else {
                        self.waiting = Some(Instant::now() + *dur);
                        break;
                    }

for this method:

                    self.inner.sleep = Some(delay_for(dur));
                }

copilot could not help in those.

Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that you're trying to use delay_for shows that you're looking at old documentation. It was renamed to sleep in Tokio 1.x.y.

Please check out the documentation for Sleep. You need to use the reset method to update the deadline. There are examples in the docs.

Co-authored-by: devensiv <devensiv@gmail.com>
@Darksonn
Copy link
Contributor

Hey, I'm really sorry that this got lost. Do you still want to work on it? If not, that's ok.

@Darksonn
Copy link
Contributor

This has been superseded by #5915.

@Darksonn Darksonn closed this Nov 25, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants