-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
fix: don't prompt when using Deno.permissions.request
with --no-prompt
#25811
Conversation
eec78a8
to
3884b9e
Compare
3884b9e
to
5cfd12e
Compare
Deno.permissions.request
when --no-prompt
is setDeno.permissions.request
with --no-prompt
Thanks for the PR, unfortunately I think this test is not correct - if you are in non-TTY environment (eg. in a CI) the prompting won't happen anyway. I believe we'll need to have this test use PTY - eg. check |
I've updated the test to use a PTY environment like suggested |
@@ -3438,7 +3441,8 @@ mod tests { | |||
fn test_request() { | |||
set_prompter(Box::new(TestPrompter)); | |||
let parser = TestPermissionDescriptorParser; | |||
let mut perms: Permissions = Permissions::none_without_prompt(); |
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.
I've changed this to none_with_prompt()
because it seemed like a mistake, since later in the test we manipulate the prompt value. I feel like this is why it has been overlooked in the first place, though I could be wrong
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.
LGTM. Thanks!
Closes #14668