fix(kv): throw error if already closed #21459
Conversation
|
|
||
| queueTest("throw error if already closed", async (db) => { | ||
| db.close(); | ||
| await assertThrows( |
There was a problem hiding this comment.
Previously, I used assertRejects only, but it got failed 😅.
I don't know, test cases are running fine on my system but failed here.
| await assertRejects( | ||
| async () => { | ||
| await db.listenQueue(() => { | ||
| resolve(); |
There was a problem hiding this comment.
This resolve is not needed, since you don't destructure promise from Promise.withResolvers() it's useless in this context. I think you can give it an empty callback.
There was a problem hiding this comment.
First, I did the same, but it failed.
Did it again.
cli/tests/unit/kv_queue_test.ts
Outdated
| db.close(); | ||
| await assertRejects( | ||
| async () => { | ||
| await db.listenQueue(() => {}); |
There was a problem hiding this comment.
normally listenQueue is called without await. can we test here that it throws without await?
There was a problem hiding this comment.
Locally, it's failing without await. listenQueue return promise.
There was a problem hiding this comment.
Fixed, I handled using try-catch :).
There was a problem hiding this comment.
My test cases are passing, but something else is failing @igorzi can you please check?
cli/tests/unit/kv_queue_test.ts
Outdated
| await db.listenQueue(() => {}); | ||
| } catch (e) { | ||
| assertEquals(e.message, "already closed"); | ||
| } |
There was a problem hiding this comment.
What if it doesn't throw? Right now the test will pass if it doesn't throw.
There was a problem hiding this comment.
Nice catch :).
Added assertFalse(false) after line 9 so if doesn't throw it will fail.
Description
If KV is closed and tries to listen queue should throw an error.
Issue
.listenQueueshould throw once.closeis called #20991