Fix the krate::downloads test
#1545
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
jtgeibel
approved these changes
Oct 30, 2018
bors-voyager bot
added a commit
that referenced
this pull request
Oct 30, 2018
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]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test wasn't technically wrong, it was just very confusing. The
assert_dl_countfunction was testing the length of the array returnedby 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_countexpect 2 downloads. But it wasn't testing the numberof 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:
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.