Skip to content

Commit d8f8b59

Browse files
authored
guide: add a testing section to the contributing guide (#598)
1 parent 0745a9b commit d8f8b59

File tree

2 files changed

+131
-3
lines changed

2 files changed

+131
-3
lines changed

CONTRIBUTING.md

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,104 @@ usually a good idea to first open an issue describing the change to solicit
112112
feedback and guidance. This will increasethe likelihood of the PR getting
113113
merged.
114114

115+
### Tests
116+
117+
If the change being proposed alters code (as opposed to only documentation for
118+
example), it is either adding new functionality to Tokio or it is fixing
119+
existing, broken functionality. In both of these cases, the pull request should
120+
include one or more tests to ensure that Tokio does not regress in the future.
121+
There are two ways to write tests: integration tests and documentation tests
122+
(Tokio avoids unit tests as much as possible).
123+
124+
#### Integration tests
125+
126+
Integration tests go in the same crate as the code they are testing. Each sub
127+
crate should have a `dev-dependency` on `tokio` itself. This makes all Tokio
128+
utilities available to use in tests, no matter the crate being tested.
129+
130+
The best strategy for writing a new integration test is to look at existing
131+
integration tests in the crate and following the style.
132+
133+
#### Documentation tests
134+
135+
Ideally, every API has at least one [documentation test] that demonstrates how to
136+
use the API. Documentation tests are run with `cargo test --doc`. This ensures
137+
that the example is correct and provides additional test coverage.
138+
139+
The trick to documentation tests is striking a balance between being succinct
140+
for a reader to understand and actually testing the API.
141+
142+
Same as with integration tests, when writing a documentation test, the full
143+
`tokio` crate is available. This is especially useful for getting access to the
144+
runtime to run the example.
145+
146+
The documentation tests will be visible from both the crate specific
147+
documentation **and** the `tokio` facade documentation via the re-export. The
148+
example should be written from the point of view of a user that is using the
149+
`tokio` crate. As such, the example should use the API via the facade and not by
150+
directly referencing the crate.
151+
152+
The type level example for `tokio_timer::Timeout` provides a good example of a
153+
documentation test:
154+
155+
```
156+
/// # extern crate futures;
157+
/// # extern crate tokio;
158+
/// // import the `timeout` function, usually this is done
159+
/// // with `use tokio::prelude::*`
160+
/// use tokio::prelude::FutureExt;
161+
/// use futures::Stream;
162+
/// use futures::sync::mpsc;
163+
/// use std::time::Duration;
164+
///
165+
/// # fn main() {
166+
/// let (tx, rx) = mpsc::unbounded();
167+
/// # tx.unbounded_send(()).unwrap();
168+
/// # drop(tx);
169+
///
170+
/// let process = rx.for_each(|item| {
171+
/// // do something with `item`
172+
/// # drop(item);
173+
/// # Ok(())
174+
/// });
175+
///
176+
/// # tokio::runtime::current_thread::block_on_all(
177+
/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds.
178+
/// process.timeout(Duration::from_millis(10))
179+
/// # ).unwrap();
180+
/// # }
181+
```
182+
183+
Given that this is a *type* level documentation test and the primary way users
184+
of `tokio` will create an instance of `Timeout` is by using
185+
`FutureExt::timeout`, this is how the documentation test is structured.
186+
187+
Lines that start with `/// #` are removed when the documentation is generated.
188+
They are only there to get the test to run. The `block_on_all` function is the
189+
easiest way to execute a future from a test.
190+
191+
If this were a documentation test for the `Timeout::new` function, then the
192+
example would explicitly use `Timeout::new`. For example:
193+
194+
```
195+
/// # extern crate futures;
196+
/// # extern crate tokio;
197+
/// use tokio::timer::Timeout;
198+
/// use futures::Future;
199+
/// use futures::sync::oneshot;
200+
/// use std::time::Duration;
201+
///
202+
/// # fn main() {
203+
/// let (tx, rx) = oneshot::channel();
204+
/// # tx.send(()).unwrap();
205+
///
206+
/// # tokio::runtime::current_thread::block_on_all(
207+
/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds.
208+
/// Timeout::new(rx, Duration::from_millis(10))
209+
/// # ).unwrap();
210+
/// # }
211+
```
212+
115213
### Commits
116214

117215
It is a recommended best practice to keep your changes as logically grouped as
@@ -286,3 +384,4 @@ _Adapted from the [Node.js contributing guide][node]_
286384

287385
[node]: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md.
288386
[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment
387+
[documentation test]: https://doc.rust-lang.org/rustdoc/documentation-tests.html

tokio-timer/src/timeout.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,22 +33,28 @@ use std::time::{Instant, Duration};
3333
/// ```rust
3434
/// # extern crate futures;
3535
/// # extern crate tokio;
36-
/// use tokio::timer::Timeout;
37-
/// use futures::{Future, Stream};
36+
/// // import the `timeout` function, usually this is done
37+
/// // with `use tokio::prelude::*`
38+
/// use tokio::prelude::FutureExt;
39+
/// use futures::Stream;
3840
/// use futures::sync::mpsc;
3941
/// use std::time::Duration;
4042
///
4143
/// # fn main() {
4244
/// let (tx, rx) = mpsc::unbounded();
4345
/// # tx.unbounded_send(()).unwrap();
4446
/// # drop(tx);
47+
///
4548
/// let process = rx.for_each(|item| {
4649
/// // do something with `item`
4750
/// # drop(item);
4851
/// # Ok(())
4952
/// });
5053
///
51-
/// Timeout::new(process, Duration::from_secs(1));
54+
/// # tokio::runtime::current_thread::block_on_all(
55+
/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds.
56+
/// process.timeout(Duration::from_millis(10))
57+
/// # ).unwrap();
5258
/// # }
5359
/// ```
5460
///
@@ -94,6 +100,29 @@ impl<T> Timeout<T> {
94100
/// See [type] level documentation for more details.
95101
///
96102
/// [type]: #
103+
///
104+
/// # Examples
105+
///
106+
/// Create a new `Timeout` set to expire in 10 milliseconds.
107+
///
108+
/// ```rust
109+
/// # extern crate futures;
110+
/// # extern crate tokio;
111+
/// use tokio::timer::Timeout;
112+
/// use futures::Future;
113+
/// use futures::sync::oneshot;
114+
/// use std::time::Duration;
115+
///
116+
/// # fn main() {
117+
/// let (tx, rx) = oneshot::channel();
118+
/// # tx.send(()).unwrap();
119+
///
120+
/// # tokio::runtime::current_thread::block_on_all(
121+
/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds.
122+
/// Timeout::new(rx, Duration::from_millis(10))
123+
/// # ).unwrap();
124+
/// # }
125+
/// ```
97126
pub fn new(value: T, timeout: Duration) -> Timeout<T> {
98127
let delay = Delay::new_timeout(now() + timeout, timeout);
99128

0 commit comments

Comments
 (0)