-
Notifications
You must be signed in to change notification settings - Fork 158
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
tests(db): test compareScore()
and getAreVotedBySessionId()
#300
tests(db): test compareScore()
and getAreVotedBySessionId()
#300
Conversation
Signed-off-by: Niklas Metje <22395665+niklasmtj@users.noreply.github.com>
assertEquals(compareScore(item1, item2), 1); | ||
}); | ||
|
||
Deno.test("[db] getAreVotedBySessionId()", async () => { |
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.
Can you please add cases for when sessionId
and sessionUser
are falsy?
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.
Check the commit 19e3e33 if this is what you thought :)
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.
Or wait you'll probably mean something like
assertArrayIncludes(await getAreVotedBySessionId([item], null), []);
Do you?
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.
More like:
assertEquals(await getAreVotedBySessionId([item], undefined), []);
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.
Got you. That example came to my mind just before I opened this PR 😄
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.
What do you prefer assertEquals
or assertArrayIncludes
in this case?
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.
assertEquals
will do the trick.
@@ -205,3 +208,49 @@ Deno.test("[db] createComment() + getCommentsByItem()", async () => { | |||
await assertRejects(async () => await createComment(comment2)); | |||
assertArrayIncludes(await getCommentsByItem(itemId), [comment1, comment2]); | |||
}); | |||
|
|||
Deno.test("[db] compareScore()", () => { |
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.
Another way we could do this is to create an items array in the wrong order by score, then sort it via compareScore()
and check the new order. This doesn't improve anything in a technical sense but does convey the intent and behaviour of the use of compareScore()
to the reader. WDYT?
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.
Yes, that sounds like a good idea! Will implement it towards the end of the week :)
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.
No worries!
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.
Updated it in 153f423. This is way better than before. I also added a third item to not just switch the items from left to right.
compareScore()
and getAreVotedBySessionId()
Signed-off-by: Niklas Metje <22395665+niklasmtj@users.noreply.github.com>
Signed-off-by: Niklas Metje <22395665+niklasmtj@users.noreply.github.com>
Interesting that the tests are failing on Linux. Is there something I can do about it? |
The failure on Linux is due to an endpoint error when downloading imports. Unfortunately, it happens occasionally. |
Signed-off-by: Niklas Metje <22395665+niklasmtj@users.noreply.github.com>
Signed-off-by: Niklas Metje <22395665+niklasmtj@users.noreply.github.com>
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.
LGTM! Nicely done! Thank you.
This should help a little with #267