Skip to content

Commit edcde83

Browse files
Merge #1545
1545: Fix the `krate::downloads` test r=jtgeibel a=sgrif 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. Co-authored-by: Sean Griffin <[email protected]>
2 parents ed04601 + e97908e commit edcde83

File tree

1 file changed

+12
-8
lines changed

1 file changed

+12
-8
lines changed

src/tests/krate.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,14 +1185,19 @@ 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
1196+
.version_downloads
1197+
.iter()
1198+
.map(|vd| vd.downloads)
1199+
.sum::<i32>();
1200+
assert_eq!(total_downloads, count);
11961201
};
11971202

11981203
let download = |name_and_version: &str| {
@@ -1206,20 +1211,19 @@ fn download() {
12061211
assert_dl_count("foo_download", None, 1);
12071212

12081213
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?
1214+
assert_dl_count("FOO_DOWNLOAD/1.0.0", None, 2);
1215+
assert_dl_count("FOO_DOWNLOAD", None, 2);
12121216

12131217
let yesterday = (Utc::today() + Duration::days(-1)).format("%F");
12141218
let query = format!("before_date={}", yesterday);
12151219
assert_dl_count("FOO_DOWNLOAD/1.0.0", Some(&query), 0);
12161220
// crate/downloads always returns the last 90 days and ignores date params
1217-
assert_dl_count("FOO_DOWNLOAD", Some(&query), 1);
1221+
assert_dl_count("FOO_DOWNLOAD", Some(&query), 2);
12181222

12191223
let tomorrow = (Utc::today() + Duration::days(1)).format("%F");
12201224
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);
1225+
assert_dl_count("FOO_DOWNLOAD/1.0.0", Some(&query), 2);
1226+
assert_dl_count("FOO_DOWNLOAD", Some(&query), 2);
12231227
}
12241228

12251229
#[test]

0 commit comments

Comments
 (0)