-
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
Changes from all commits
a84426f
69d9ad5
19e3e33
153f423
8a01086
9585414
f37a628
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
// Copyright 2023 the Deno authors. All rights reserved. MIT license. | ||
import { | ||
type Comment, | ||
compareScore, | ||
createComment, | ||
createItem, | ||
createUser, | ||
|
@@ -11,6 +12,7 @@ import { | |
deleteVote, | ||
formatDate, | ||
getAllItems, | ||
getAreVotedBySessionId, | ||
getCommentsByItem, | ||
getDatesSince, | ||
getItem, | ||
|
@@ -236,3 +238,70 @@ Deno.test("[db] getDatesSince()", () => { | |
formatDate(new Date()), | ||
]); | ||
}); | ||
|
||
Deno.test("[db] compareScore()", () => { | ||
const item1: Item = { | ||
userId: crypto.randomUUID(), | ||
title: crypto.randomUUID(), | ||
url: `http://${crypto.randomUUID()}.com`, | ||
...newItemProps(), | ||
score: 1, | ||
}; | ||
const item2: Item = { | ||
userId: crypto.randomUUID(), | ||
title: crypto.randomUUID(), | ||
url: `http://${crypto.randomUUID()}.com`, | ||
...newItemProps(), | ||
score: 2, | ||
}; | ||
const item3: Item = { | ||
userId: crypto.randomUUID(), | ||
title: crypto.randomUUID(), | ||
url: `http://${crypto.randomUUID()}.com`, | ||
...newItemProps(), | ||
score: 5, | ||
}; | ||
|
||
const aa = [item2, item3, item1]; | ||
const sorted = aa.toSorted(compareScore); | ||
|
||
assertArrayIncludes(sorted, [item1, item2, item3]); | ||
}); | ||
|
||
Deno.test("[db] getAreVotedBySessionId()", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add cases for when There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Or wait you'll probably mean something like
Do you? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. What do you prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
const item: Item = { | ||
userId: crypto.randomUUID(), | ||
title: crypto.randomUUID(), | ||
url: `http://${crypto.randomUUID()}.com`, | ||
...newItemProps(), | ||
score: 1, | ||
}; | ||
|
||
const user = genNewUser(); | ||
const vote = { item, user }; | ||
|
||
assertEquals(await getUserBySession(user.sessionId), null); | ||
assertEquals(await getItem(item.id), null); | ||
assertEquals(await getAreVotedBySessionId([item], user.sessionId), []); | ||
assertEquals(await getAreVotedBySessionId([item], undefined), []); | ||
assertEquals( | ||
await getAreVotedBySessionId([item], "not-a-session"), | ||
[], | ||
); | ||
assertEquals( | ||
await getAreVotedBySessionId([item], crypto.randomUUID()), | ||
[], | ||
); | ||
|
||
await createItem(item); | ||
|
||
await createUser(user); | ||
await createVote(vote); | ||
|
||
assertEquals(await getItem(item.id), item); | ||
assertEquals(await getUserBySession(user.sessionId), user); | ||
|
||
assertEquals(await getAreVotedBySessionId([item], user.sessionId), [ | ||
true, | ||
]); | ||
}); |
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 ofcompareScore()
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.