-
Notifications
You must be signed in to change notification settings - Fork 314
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
Test the laziness of our iterators #792
Test the laziness of our iterators #792
Conversation
0ee2319
to
acf92c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tremendous thanks for doing this work! There's no rush on this (as we have some time to go before we can merge this PR), but before merging, I'd like us to have distinct tests, rather than one big one. We can do so without too much trouble with a little macro magic:
mod laziness {
struct Panicking;
impl Iterator for Panicking {
type Item = ();
fn next(&mut self) -> Option<()> {
panic!("iterator adaptor is not lazy")
}
}
macro_rules! tests {
($(test $name:ident { $body:expr })*) => {
$(
#[deny(unused_must_use)]
#[test]
fn $name() {
let _ = $body;
}
)*
}
}
use itertools::*;
tests! {
test interleave {
Panicking.interleave(Panicking)
}
test interleave_shortest {
Panicking.interleave_shortest(Panicking)
}
test intersperse {
Panicking.intersperse(())
}
/* and so on */
}
}
The advantage of this approach is that the tests are run in parallel, are reported individually, and can be run individually.
I surely had doubts about my big test. Thanks for your review.
|
a8c19f0
to
5fa620e
Compare
0c723b5
to
1d2716e
Compare
1d2716e
to
6bf3ba7
Compare
6bf3ba7
to
ea45826
Compare
2d48ec0
to
7b5da33
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #792 +/- ##
==========================================
- Coverage 94.40% 94.29% -0.11%
==========================================
Files 48 48
Lines 6665 6665
==========================================
- Hits 6292 6285 -7
- Misses 373 380 +7 ☔ View full report in Codecov by Sentry. |
I'm not sure what to do about the remaining unlazy iterators yet. |
Ignore the 3 failing tests (for now). `ChunkBy` and `IntoChunks` are not iterators themselves but implement `IntoIterator` instead. Then `Groups` and `Chunks` are iterators. All four are lazy and must be used.
7b5da33
to
fee3f6d
Compare
Related to #791
Add a test to:
must_use
attributes with very little discipline.It's a long list of iterators and it has some edge cases. The error message help us understand how unlazy there are.
When
let _ = ...;
is not necessary, it indicates that amust_use
attribute is missing.CI will fail for some time.