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

refactor(rumqttc): make AsyncClient/Eventloop mockable #885

Closed
wants to merge 4 commits into from

Conversation

mbodmer
Copy link

@mbodmer mbodmer commented Jul 4, 2024

I have the need to mock the interface to the rumqttc client for testing/benching purposes.

There was no evident way I could achieve this other than introducing a new trait PollableEventLoop defining the poll interface on the EventLoop.

Now it is possible to reuse the AsyncClient and mock the EventLoop.

I have also added a mocked version of the asyncpubsub example.

This change will make it mandatory to use rumqttc::PollableEventLoop, that is why I mark this as a breaking change.

Type of change

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • Formatted with cargo fmt
  • Make an entry to CHANGELOG.md if it's relevant to the users of the library. If it's not relevant mention why.

@mbodmer
Copy link
Author

mbodmer commented Aug 28, 2024

Any chance this is going to be accepted?

@biglben
Copy link

biglben commented Aug 30, 2024

Great work. I'm also very interested in getting this merged as it would be incredibly helpful for our testing process. Looking forward to seeing this merged soon

@swanandx
Copy link
Member

Hey @mbodmer , thanks for the PR.

As per the example in PR, mock eventloop is just an wrapper and uses AsyncClient::from_sender internally, so I don't think we need the trait PollableEventLoop. For testing, all you need to do use use from_sender method and use the rx of that channel to do anything right? even recreating your own eventloop!

Please feel free to lmk if I'm missing something, thank you for understanding :)

@mbodmer
Copy link
Author

mbodmer commented Aug 30, 2024

Thanks so much for your feedback @swanandx

I am not sure I completely understand your suggestion. Also the example might not completely capture my situation. My client code is something like:

pub struct MqttClientService<S> {
    pub(crate) client: AsyncClient,
    pub(crate) eventloop: EventLoop,
    pub(crate) inner: S,
}

Here I cannot make the EventLoop generic when it is not a trait. How would I integrate with a mocked eventloop or how would your suggestion allow for this? I am not that experienced with rust, so please excuse me, I probably did not understand yet.

@swanandx
Copy link
Member

let me elaborate: for mock eventlloop

struct MockEventloop {
    rx: Receiver<Request>,
}

// impl MockEventloop as you wish

// To create client and eventloop you can do something like:
let (tx, rx) = flume::unbounded();
let client = AsyncClient::from_senders(tx);
let eventloop = MockEventloop { rx };

Coming to your code, you want to have Eventloop such that both rumqttc's Eventloop and your mock eventloop can integrate with your MqttClientService right?

For that, you can define and implement that trait inside your code as well right? for Eventloop you can just call it's poll internally, and for Mockeventloop you can do as you need. pseudocode:

impl Pollable for EventLoop {
    fn poll(..) {
        self.poll().await
    }
}

Otherwise, you can have MockMqttClient which holds the rx ( instead of eventloop ) which you can use for testing.

@mbodmer
Copy link
Author

mbodmer commented Aug 30, 2024

You are right, this might work. I will try this and report back.
I think I'd close the PR as it could still be reopened?

Thanks for maintaining rumqtt!

@swanandx
Copy link
Member

cool! feel free to reopen / comment if required, thanks :)

@swanandx swanandx closed this Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants