Skip to content

update permissions samples on testing.md - #1664

Merged
thisisjofrank merged 2 commits into
denoland:mainfrom
kuboon:patch-1
May 1, 2025
Merged

thisisjofrank merged 2 commits into
denoland:mainfrom
kuboon:patch-1

Conversation

@kuboon

@kuboon kuboon commented Apr 25, 2025

Copy link
Copy Markdown
Contributor

I was confused by the permissions option in Deno.test, so I’d like to suggest a correction.

In the early example, permissions: { read: true }, is included, but the test actually passes even without it. I found this point unclear, so I think it should be removed.

A more detailed explanation is provided in the ## Tests and Permissions section, but including the unnecessary permissions: { read: true }, there as well makes the line "allows you to specifically deny permissions, but does not grant them." harder to understand.

What do you think?

# Provide permission for deno to read from the filesystem, which is necessary
# for the final test above to pass
deno test --allow-read my_test.ts
deno test --allow-read=. my_test.ts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a personal preference, but I have a strong desire to promote the practice of not granting broad permissions too easily.

@thisisjofrank
thisisjofrank self-requested a review May 1, 2025 10:45

@thisisjofrank thisisjofrank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@thisisjofrank
thisisjofrank merged commit 560ce74 into denoland:main May 1, 2025
@kuboon
kuboon deleted the patch-1 branch May 1, 2025 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants