-
-
Notifications
You must be signed in to change notification settings - Fork 5
fixed existing tests #19
base: master
Are you sure you want to change the base?
Conversation
can you cancel hanging ci |
deno.jsonc
Outdated
@@ -7,7 +7,7 @@ | |||
"semiColons": false | |||
}, | |||
"tasks": { | |||
"test": "deno test --allow-net --allow-read --coverage=coverage --parallel", | |||
"test": "deno test --allow-net --allow-all --coverage=coverage --parallel", |
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.
--allow-all
is insecure to use. if you meant to add a permission add it via --allow-{perm}
extensions/res/download.ts
Outdated
@@ -9,21 +9,31 @@ export type DownloadOptions = | |||
headers: Record<string, unknown> | |||
}> | |||
|
|||
type Callback = (err?: any) => void |
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.
avoid any
wherever possible, use unknown
instead
extensions/res/send/sendFile.ts
Outdated
async ( | ||
path: string, | ||
{ signal, ...opts }: SendFileOptions = {}, | ||
cb?: (err?: any) => void, |
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.
better change it to unknown
, we don't know what kind of error could be thrown
@@ -74,7 +75,11 @@ async (path: string, { signal, ...opts }: SendFileOptions = {}) => { | |||
|
|||
res._init.status = status | |||
|
|||
const file = await Deno.readFile(path, { signal }) | |||
let file |
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.
this can be instead wrapped in try catch
tests/core/app.test.ts
Outdated
|
||
// await makeFetch(server)('/route').expect(200, 'A different route') | ||
const server = app.handler |
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.
app handler is not a server, I think it's okay to just pass the app.handler to makeFetch directly
No description provided.