Skip to content

Commit c86e585

Browse files
committed
Fix the krate::downloads test
This test wasn't technically wrong, it was just very confusing. The `assert_dl_count` function was testing the length of the array returned by the downloads endpoint. When we test that case insensitivity works, this makes it look like there's a bug, since we'd intuitively expect `assert_dl_count` expect 2 downloads. But it wasn't testing the number of downloads, it was testing the number of records we returned (one record per version per day that there were downloads). So the JSON we returned was correct: [ { "version_id": 1, "date": "2018-10-30", "downloads": 2, } ] But the test didn't really reflect what we expected. This changes it to actually test the sum of the "downloads" field, which is intuitively what you'd expect this to be testing.
1 parent ed04601 commit c86e585

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

src/tests/krate.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,14 +1185,17 @@ fn download() {
11851185
.expect_build(&conn);
11861186
});
11871187

1188-
let assert_dl_count = |name_and_version: &str, query: Option<&str>, count: usize| {
1188+
let assert_dl_count = |name_and_version: &str, query: Option<&str>, count: i32| {
11891189
let url = format!("/api/v1/crates/{}/downloads", name_and_version);
11901190
let downloads: Downloads = if let Some(query) = query {
11911191
anon.get_with_query(&url, query).good()
11921192
} else {
11931193
anon.get(&url).good()
11941194
};
1195-
assert_eq!(downloads.version_downloads.len(), count);
1195+
let total_downloads = downloads.version_downloads.iter()
1196+
.map(|vd| vd.downloads)
1197+
.sum::<i32>();
1198+
assert_eq!(total_downloads, count);
11961199
};
11971200

11981201
let download = |name_and_version: &str| {
@@ -1206,20 +1209,19 @@ fn download() {
12061209
assert_dl_count("foo_download", None, 1);
12071210

12081211
download("FOO_DOWNLOAD/1.0.0");
1209-
assert_dl_count("FOO_DOWNLOAD/1.0.0", None, 1);
1210-
assert_dl_count("FOO_DOWNLOAD", None, 1);
1211-
// TODO: Is the behavior above a bug?
1212+
assert_dl_count("FOO_DOWNLOAD/1.0.0", None, 2);
1213+
assert_dl_count("FOO_DOWNLOAD", None, 2);
12121214

12131215
let yesterday = (Utc::today() + Duration::days(-1)).format("%F");
12141216
let query = format!("before_date={}", yesterday);
12151217
assert_dl_count("FOO_DOWNLOAD/1.0.0", Some(&query), 0);
12161218
// crate/downloads always returns the last 90 days and ignores date params
1217-
assert_dl_count("FOO_DOWNLOAD", Some(&query), 1);
1219+
assert_dl_count("FOO_DOWNLOAD", Some(&query), 2);
12181220

12191221
let tomorrow = (Utc::today() + Duration::days(1)).format("%F");
12201222
let query = format!("before_date={}", tomorrow);
1221-
assert_dl_count("FOO_DOWNLOAD/1.0.0", Some(&query), 1);
1222-
assert_dl_count("FOO_DOWNLOAD", Some(&query), 1);
1223+
assert_dl_count("FOO_DOWNLOAD/1.0.0", Some(&query), 2);
1224+
assert_dl_count("FOO_DOWNLOAD", Some(&query), 2);
12231225
}
12241226

12251227
#[test]

0 commit comments

Comments
 (0)