Skip to content

Commit 2ce8d72

Browse files
committed
Test that previously-downloaded files are found and re-used
The test is only that the file-reuse notification is fired. In order to detect this in the test, I've pushed through the notification handler that gets given to `manifestation.update`, which in practice is nearly always a clone of the one in DownloadCfg (but notably not when called as in the test cases).
1 parent 8ab727a commit 2ce8d72

File tree

3 files changed

+66
-12
lines changed

3 files changed

+66
-12
lines changed

src/rustup-dist/src/dist.rs

+7-11
Original file line numberDiff line numberDiff line change
@@ -504,13 +504,9 @@ impl ops::Deref for File {
504504

505505
impl<'a> DownloadCfg<'a> {
506506

507-
fn notify_handler(&self, event: Notification) {
508-
(self.notify_handler)(event);
509-
}
510-
511-
pub fn download(&self, url: &Url, hash: &str) -> Result<File> {
507+
pub fn download(&self, url: &Url, hash: &str, notify_handler: &'a Fn(Notification)) -> Result<File> {
512508

513-
try!(utils::ensure_dir_exists("Download Directory", &self.download_dir, &|n| self.notify_handler(n.into())));
509+
try!(utils::ensure_dir_exists("Download Directory", &self.download_dir, &|n| notify_handler(n.into())));
514510
let target_file = self.download_dir.join(Path::new(hash));
515511

516512
if target_file.exists() {
@@ -528,11 +524,11 @@ impl<'a> DownloadCfg<'a> {
528524
}
529525
let cached_result = hasher.result_str();
530526
if hash == cached_result {
531-
self.notify_handler(Notification::FileAlreadyDownloaded);
532-
self.notify_handler(Notification::ChecksumValid(&url.to_string()));
527+
notify_handler(Notification::FileAlreadyDownloaded);
528+
notify_handler(Notification::ChecksumValid(&url.to_string()));
533529
return Ok(File { path: target_file, });
534530
} else {
535-
self.notify_handler(Notification::CachedFileChecksumFailed);
531+
notify_handler(Notification::CachedFileChecksumFailed);
536532
try!(fs::remove_file(&target_file).chain_err(|| "cleaning up previous download"));
537533
}
538534
}
@@ -542,7 +538,7 @@ impl<'a> DownloadCfg<'a> {
542538
try!(utils::download_file(&url,
543539
&target_file,
544540
Some(&mut hasher),
545-
&|n| self.notify_handler(n.into())));
541+
&|n| notify_handler(n.into())));
546542

547543
let actual_hash = hasher.result_str();
548544

@@ -554,7 +550,7 @@ impl<'a> DownloadCfg<'a> {
554550
calculated: actual_hash,
555551
}.into());
556552
} else {
557-
self.notify_handler(Notification::ChecksumValid(&url.to_string()));
553+
notify_handler(Notification::ChecksumValid(&url.to_string()));
558554
return Ok(File { path: target_file, })
559555
}
560556
}

src/rustup-dist/src/manifestation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl Manifestation {
140140

141141
let url_url = try!(utils::parse_url(&url));
142142

143-
let dowloaded_file = try!(download_cfg.download(&url_url, &hash).chain_err(|| {
143+
let dowloaded_file = try!(download_cfg.download(&url_url, &hash, &notify_handler).chain_err(|| {
144144
ErrorKind::ComponentDownloadFailed(component.clone())
145145
}));
146146
things_downloaded.push(hash);

src/rustup-dist/tests/dist.rs

+58
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ use rustup_dist::temp;
2626
use rustup_dist::manifestation::{Manifestation, UpdateStatus, Changes};
2727
use rustup_dist::manifest::{Manifest, Component};
2828
use url::Url;
29+
use std::cell::Cell;
2930
use std::fs;
3031
use std::path::Path;
32+
use std::sync::Arc;
33+
3134
use tempdir::TempDir;
3235

3336
// Creates a mock dist server populated with some test data
@@ -921,3 +924,58 @@ fn unable_to_download_component() {
921924
}
922925
});
923926
}
927+
928+
fn prevent_installation(prefix: &InstallPrefix) {
929+
utils::ensure_dir_exists("installation path", &prefix.path().join("lib"), &|_|{}).unwrap();
930+
let install_blocker = prefix.path().join("lib").join("rustlib");
931+
utils::write_file("install-blocker", &install_blocker, "fail-installation").unwrap();
932+
}
933+
934+
fn allow_installation(prefix: &InstallPrefix) {
935+
let install_blocker = prefix.path().join("lib").join("rustlib");
936+
utils::remove_file("install-blocker", &install_blocker).unwrap();
937+
}
938+
939+
#[test]
940+
fn reuse_downloaded_file() {
941+
setup(None, &|url, toolchain, prefix, download_cfg, temp_cfg| {
942+
943+
prevent_installation(prefix);
944+
945+
update_from_dist(url, toolchain, prefix, &[], &[], download_cfg, temp_cfg, &|_| {}).unwrap_err();
946+
947+
allow_installation(&prefix);
948+
949+
let reuse_notification_fired = Arc::new(Cell::new(false));
950+
update_from_dist(url, toolchain, prefix, &[], &[], download_cfg, temp_cfg, &|n| {
951+
if let Notification::FileAlreadyDownloaded = n {
952+
reuse_notification_fired.set(true);
953+
}
954+
}).unwrap();
955+
956+
assert!(reuse_notification_fired.get());
957+
})
958+
}
959+
960+
#[test]
961+
fn checks_files_hashes_before_reuse() {
962+
setup(None, &|url, toolchain, prefix, download_cfg, temp_cfg| {
963+
964+
let path = url.to_file_path().unwrap();
965+
let target_hash = utils::read_file("target hash", &path.join("dist/2016-02-02/rustc-nightly-x86_64-apple-darwin.tar.gz.sha256")).unwrap()[.. 64].to_owned();
966+
let prev_download = download_cfg.download_dir.join(target_hash);
967+
utils::ensure_dir_exists("download dir", &download_cfg.download_dir, &|_|{}).unwrap();
968+
utils::write_file("bad previous download", &prev_download, "bad content").unwrap();
969+
println!("wrote previous download to {}", prev_download.display());
970+
971+
let noticed_bad_checksum = Arc::new(Cell::new(false));
972+
update_from_dist(url, toolchain, prefix, &[], &[], download_cfg, temp_cfg, &|n| {
973+
println!("{:?}", n);
974+
if let Notification::CachedFileChecksumFailed = n {
975+
noticed_bad_checksum.set(true);
976+
}
977+
}).unwrap();
978+
979+
assert!(noticed_bad_checksum.get());
980+
})
981+
}

0 commit comments

Comments
 (0)