-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
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.