test(http): basic endpoint tests #325
Conversation
| @@ -286,11 +279,29 @@ export async function deleteVote(vote: Vote) { | |||
| vote.user.id, | |||
| ]; | |||
|
|
|||
| const [votedItemsByUserRes, votedUsersByItemRes] = await kv.getMany([ | |||
There was a problem hiding this comment.
this is something i stumbled upon by accident.
if we compare createVote and deleteVote, we were missing a check for the votedItemsByUserKey and votedUsersByItemKey results.
as the deletion for a non-existent key doesn't throw (anymore?), the mutation on the vote.item.score would've been abused via api calls.
this is why an explicit check for these keys and their respective results has been introduced.
There was a problem hiding this comment.
Good catch! Though, next time, let's keep the objectives per PR to 1 🙂
|
I half expected this - test coverage has taken a significant hit now that more files are included in the tested list. What to do... |
|
Let's ignore test coverage for We're more concerned with test coverage of logic, which lies in the |
mbhrznr
force-pushed
the
test/basic-endpoint-tests
branch
from
2afbb0f to
67fdbfb
Compare
added the didn't came to mind to check the coverage, yet it makes sense that we took this big hit. as for the now ignored dirs it might make sense to introduce e2e tests at some point, wdyt? |
| @@ -286,11 +279,29 @@ export async function deleteVote(vote: Vote) { | |||
| vote.user.id, | |||
| ]; | |||
|
|
|||
| const [votedItemsByUserRes, votedUsersByItemRes] = await kv.getMany([ | |||
There was a problem hiding this comment.
Good catch! Though, next time, let's keep the objectives per PR to 1 🙂
Yep! In fact, what if we move the newly created e2e tests from this PR to the root of the folder and name it |
closes #315.
this pr adds a bunch of simple tests to have some coverage on our endpoints.
as described in the issue, it simply follows the guide as described here.
additionally it tweaks the
deleteVotefunction, as theoretically, one could've downvoted an item into oblivion.