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

Provide a crate with utilities for testing futures based code #1169

Closed
1 of 3 tasks
Nemo157 opened this issue Aug 3, 2018 · 19 comments
Closed
1 of 3 tasks

Provide a crate with utilities for testing futures based code #1169

Nemo157 opened this issue Aug 3, 2018 · 19 comments

Comments

@Nemo157
Copy link
Member

Nemo157 commented Aug 3, 2018

Related to rustasync/team#38

I think it's worth futures-rs itself providing a crate with basic test utilities. Over time I would expect more complex utilities to be provided by crates out of tree, but having something available around the time of the 0.3 release for at least basic testing would be very useful for the ecosystem.

Ideas suggested:

  • more introspect-able Context/Waker/Spawn implementations
  • in-memory IO primitives that can be induced to return Poll::Pending sometimes
  • #[async_test] proc-macro
  • (feel free to throw more ideas out for a first cut)
@Nemo157
Copy link
Member Author

Nemo157 commented Aug 3, 2018

I just threw together a start on a crate to provide something more useful for discussion, it trys to implement something around the second bullet above: https://github.com/Nemo157/futures-rs/tree/testing-utilities

@cramertj
Copy link
Member

cramertj commented Aug 3, 2018

I think everything in futures/tests/support is really handy and should be exposed to the public somehow, especially the waker counter context, which is super handy for testing readiness notifications from futures primitives.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 3, 2018

especially the waker counter context

Heh, that’s basically exactly what I implemented on that branch 😄, oh well, it was a good chance for me to finally look at how the 0.3 waker system fits together.

@MajorBreakfast
Copy link
Contributor

I agree that we should aim to provide first party testing utilities. Testing is important and every respectable library needs to do it. We can avoid a lot of work duplication throughout the ecosystem if we provide common utilities. And the fewer project specific utilities a project uses, the easier it is of course for others to get started and contribute. Furthermore, we can write a guide that explains how it's done.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 3, 2018

Cool, leads are on board 😁, I’ll take a look at what’s in the futures test support currently and try and pull out a first pass on a crate this weekend.

@MajorBreakfast
Copy link
Contributor

Do we want to put this into futures_util::test or into a futures-test crate?

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 3, 2018

IMO separate crate that’s not in the facade to be used as a dev-dependency

@MajorBreakfast
Copy link
Contributor

IMO separate crate that’s not in the facade to be used as a dev-dependency

I agree

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 27, 2018

Another idea I just had, provide an #[async_test] proc-macro that takes an async fn() and wraps that into a #[test] fn() { block_on(original_fn()) }.

EDIT: Added to top post, feel free to expand the list with new ideas that come in, I'm gonna advertise this as a brainstorm issue in the blog post about the crate.

@MajorBreakfast
Copy link
Contributor

#[async_test] proc-macro
@Nemo157 Interesting idea! I like it 😃

@cramertj
Copy link
Member

I'm slightly less sure about this idea-- I'd think most "real-ish" futures written using async/await syntax wouldn't work with this sort of thing because they'd interact with executor specific primitives that wouldn't interoperate with futures_executor::block_on. it seems easy for someone to be mislead into thinking that they could use #[async_test] with a tokio future.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 28, 2018

That’s a good point. First thought is to make the executor configurable in the attribute (where executor in this case simply means for<F: Future> Fn(F) -> F::Output). I wonder if there’s a way the project could configure which executor should be used for its async tests, so the attribute would give a compile time error if you hadn’t telling you how to globally configure it, but you don’t have the overhead of having to set it on every test.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 29, 2018

Just had another thought: https://crates.io/crates/futures-test

@carllerche do you have any plans for this crate name?

@carllerche
Copy link
Member

The plan has been to release this once I need to: https://github.com/carllerche/better-future/tree/master/futures-test

It is currently being depended on via git from tower, and other repos.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 29, 2018

@carllerche I don't suppose you'd want to use 0.1.* versions for that crate and allow using 0.3.* for the crate we have here once futures 0.3 is actually released? Seems like there's probably too much chance for confusion since they're independent crates.

@MajorBreakfast, @cramertj any ideas for an alternative name? I'd suggest not publishing this crate with the alpha.4 release so we have time to rename it before advertising it.

@cramertj
Copy link
Member

futures-testing/futures-test[ing]-utils?

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 31, 2018

futures-test-utils was my first thought as well.

@carllerche
Copy link
Member

I can hand off futures-utils, it isn't a big deal.

@cramertj
Copy link
Member

futures-test-preview exists in our tree now.

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

No branches or pull requests

4 participants