-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Add version option for upgrade #5156
Conversation
now the previous listed missing features have been implemented |
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 feature is welcome - thanks. Can you add a test tho? I think you can base it on this test:
deno/cli/tests/integration_tests.rs
Lines 203 to 225 in 4e5e6da
// Warning: this test requires internet access. | |
#[test] | |
fn upgrade_in_tmpdir() { | |
let temp_dir = TempDir::new().unwrap(); | |
let exe_path = if cfg!(windows) { | |
temp_dir.path().join("deno") | |
} else { | |
temp_dir.path().join("deno.exe") | |
}; | |
let _ = std::fs::copy(util::deno_exe_path(), &exe_path).unwrap(); | |
assert!(exe_path.exists()); | |
let _mtime1 = std::fs::metadata(&exe_path).unwrap().modified().unwrap(); | |
let status = Command::new(&exe_path) | |
.arg("upgrade") | |
.arg("--force") | |
.spawn() | |
.unwrap() | |
.wait() | |
.unwrap(); | |
assert!(status.success()); | |
let _mtime2 = std::fs::metadata(&exe_path).unwrap().modified().unwrap(); | |
// TODO(ry) assert!(mtime1 < mtime2); | |
} |
@ry will this suffice? |
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 - nice patch!
This would close #5146.
It isnt exactly complete yet:
The checking for latest version should only happen if
version
is not passed