From ae446f91e490152229a2c6f95f7b3c127ededa42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 6 Oct 2019 21:03:30 +0200 Subject: [PATCH] remove more calls to tokio_util::block_on (#3059) towards #2960 --- cli/compilers/ts.rs | 92 ++--- cli/file_fetcher.rs | 921 +++++++++++++++++++++++--------------------- cli/http_util.rs | 25 +- cli/worker.rs | 50 +-- 4 files changed, 568 insertions(+), 520 deletions(-) diff --git a/cli/compilers/ts.rs b/cli/compilers/ts.rs index 3e92e5ccfdf357..9cbaaae09bdfc3 100644 --- a/cli/compilers/ts.rs +++ b/cli/compilers/ts.rs @@ -656,50 +656,46 @@ mod tests { use crate::fs as deno_fs; use crate::tokio_util; use deno::ModuleSpecifier; + use futures::future::lazy; use std::path::PathBuf; use tempfile::TempDir; - impl TsCompiler { - fn compile_sync( - self: &Self, - state: ThreadSafeState, - source_file: &SourceFile, - ) -> Result { - tokio_util::block_on(self.compile_async(state, source_file)) - } - } - #[test] - fn test_compile_sync() { - tokio_util::init(|| { - let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .parent() - .unwrap() - .join("tests/002_hello.ts") - .to_owned(); - let specifier = - ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap(); - - let out = SourceFile { - url: specifier.as_url().clone(), - filename: PathBuf::from(p.to_str().unwrap().to_string()), - media_type: msg::MediaType::TypeScript, - source_code: include_bytes!("../tests/002_hello.ts").to_vec(), - }; + fn test_compile_async() { + let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap() + .join("tests/002_hello.ts") + .to_owned(); + let specifier = + ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap(); + + let out = SourceFile { + url: specifier.as_url().clone(), + filename: PathBuf::from(p.to_str().unwrap().to_string()), + media_type: msg::MediaType::TypeScript, + source_code: include_bytes!("../tests/002_hello.ts").to_vec(), + }; - let mock_state = ThreadSafeState::mock(vec![ - String::from("deno"), - String::from("hello.js"), - ]); - let compiled = mock_state + let mock_state = ThreadSafeState::mock(vec![ + String::from("deno"), + String::from("hello.js"), + ]); + + tokio_util::run(lazy(move || { + mock_state .ts_compiler - .compile_sync(mock_state.clone(), &out) - .unwrap(); - assert!(compiled - .code - .as_bytes() - .starts_with("console.log(\"Hello World\");".as_bytes())); - }) + .compile_async(mock_state.clone(), &out) + .then(|result| { + assert!(result.is_ok()); + assert!(result + .unwrap() + .code + .as_bytes() + .starts_with("console.log(\"Hello World\");".as_bytes())); + Ok(()) + }) + })) } #[test] @@ -719,12 +715,20 @@ mod tests { p.to_string_lossy().into(), String::from("$deno$/bundle.js"), ]); - let out = state.ts_compiler.bundle_async( - state.clone(), - module_name, - String::from("$deno$/bundle.js"), - ); - assert!(tokio_util::block_on(out).is_ok()); + + tokio_util::run(lazy(move || { + state + .ts_compiler + .bundle_async( + state.clone(), + module_name, + String::from("$deno$/bundle.js"), + ) + .then(|result| { + assert!(result.is_ok()); + Ok(()) + }) + })) } #[test] diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 4ae48c4222cecc..e865bf9450733e 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -612,38 +612,6 @@ mod tests { use crate::fs as deno_fs; use tempfile::TempDir; - impl SourceFileFetcher { - /// Fetch remote source code. - fn fetch_remote_source( - self: &Self, - module_url: &Url, - use_disk_cache: bool, - no_remote_fetch: bool, - redirect_limit: i64, - ) -> Result { - tokio_util::block_on(self.fetch_remote_source_async( - module_url, - use_disk_cache, - no_remote_fetch, - redirect_limit, - )) - } - - /// Synchronous version of get_source_file_async - fn get_source_file( - self: &Self, - module_url: &Url, - use_disk_cache: bool, - no_remote_fetch: bool, - ) -> Result { - tokio_util::block_on(self.get_source_file_async( - module_url, - use_disk_cache, - no_remote_fetch, - )) - } - } - fn setup_file_fetcher(dir_path: &Path) -> SourceFileFetcher { SourceFileFetcher::new( DiskCache::new(&dir_path.to_path_buf().join("deps")), @@ -728,81 +696,100 @@ mod tests { fn test_get_source_code_1() { let http_server_guard = crate::test_util::http_server(); let (temp_dir, fetcher) = test_setup(); - // http_util::fetch_sync_string requires tokio - tokio_util::init(|| { - let module_url = - Url::parse("http://localhost:4545/tests/subdir/mod2.ts").unwrap(); - let headers_file_name = fetcher.deps_cache.location.join( - fetcher - .deps_cache - .get_cache_filename_with_extension(&module_url, "headers.json"), - ); + let fetcher_1 = fetcher.clone(); + let fetcher_2 = fetcher.clone(); + let module_url = + Url::parse("http://localhost:4545/tests/subdir/mod2.ts").unwrap(); + let module_url_1 = module_url.clone(); + let module_url_2 = module_url.clone(); + let headers_file_name = fetcher.deps_cache.location.join( + fetcher + .deps_cache + .get_cache_filename_with_extension(&module_url, "headers.json"), + ); + let headers_file_name_1 = headers_file_name.clone(); + let headers_file_name_2 = headers_file_name.clone(); + let headers_file_name_3 = headers_file_name.clone(); - let result = fetcher.get_source_file(&module_url, true, false); - assert!(result.is_ok()); - let r = result.unwrap(); - assert_eq!( - r.source_code, - "export { printHello } from \"./print_hello.ts\";\n".as_bytes() - ); - assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); - // Should not create .headers.json file due to matching ext - assert!(fs::read_to_string(&headers_file_name).is_err()); - - // Modify .headers.json, write using fs write and read using save_source_code_headers - let _ = - fs::write(&headers_file_name, "{ \"mime_type\": \"text/javascript\" }"); - let result2 = fetcher.get_source_file(&module_url, true, false); - assert!(result2.is_ok()); - let r2 = result2.unwrap(); - assert_eq!( - r2.source_code, - "export { printHello } from \"./print_hello.ts\";\n".as_bytes() - ); - // If get_source_file does not call remote, this should be JavaScript - // as we modified before! (we do not overwrite .headers.json due to no http fetch) - assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); - assert_eq!( - fetcher - .get_source_code_headers(&module_url) - .mime_type - .unwrap(), - "text/javascript" - ); + let fut = fetcher + .get_source_file_async(&module_url, true, false) + .then(move |result| { + assert!(result.is_ok()); + let r = result.unwrap(); + assert_eq!( + r.source_code, + "export { printHello } from \"./print_hello.ts\";\n".as_bytes() + ); + assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); + // Should not create .headers.json file due to matching ext + assert!(fs::read_to_string(&headers_file_name_1).is_err()); + + // Modify .headers.json, write using fs write and read using save_source_code_headers + let _ = fs::write( + &headers_file_name_1, + "{ \"mime_type\": \"text/javascript\" }", + ); + fetcher_1.get_source_file_async(&module_url, true, false) + }) + .then(move |result2| { + assert!(result2.is_ok()); + let r2 = result2.unwrap(); + assert_eq!( + r2.source_code, + "export { printHello } from \"./print_hello.ts\";\n".as_bytes() + ); + // If get_source_file_async does not call remote, this should be JavaScript + // as we modified before! (we do not overwrite .headers.json due to no http fetch) + assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); + assert_eq!( + fetcher_2 + .get_source_code_headers(&module_url_1) + .mime_type + .unwrap(), + "text/javascript" + ); + + // Modify .headers.json again, but the other way around + let _ = fetcher_2.save_source_code_headers( + &module_url_1, + Some("application/json".to_owned()), + None, + ); + fetcher_2.get_source_file_async(&module_url_1, true, false) + }) + .then(move |result3| { + assert!(result3.is_ok()); + let r3 = result3.unwrap(); + assert_eq!( + r3.source_code, + "export { printHello } from \"./print_hello.ts\";\n".as_bytes() + ); + // If get_source_file_async does not call remote, this should be JavaScript + // as we modified before! (we do not overwrite .headers.json due to no http fetch) + assert_eq!(&(r3.media_type), &msg::MediaType::Json); + assert!(fs::read_to_string(&headers_file_name_2) + .unwrap() + .contains("application/json")); + + // let's create fresh instance of DenoDir (simulating another freshh Deno process) + // and don't use cache + let fetcher = setup_file_fetcher(temp_dir.path()); + fetcher.get_source_file_async(&module_url_2, false, false) + }) + .then(move |result4| { + assert!(result4.is_ok()); + let r4 = result4.unwrap(); + let expected4 = + "export { printHello } from \"./print_hello.ts\";\n".as_bytes(); + assert_eq!(r4.source_code, expected4); + // Now the old .headers.json file should have gone! Resolved back to TypeScript + assert_eq!(&(r4.media_type), &msg::MediaType::TypeScript); + assert!(fs::read_to_string(&headers_file_name_3).is_err()); + Ok(()) + }); - // Modify .headers.json again, but the other way around - let _ = fetcher.save_source_code_headers( - &module_url, - Some("application/json".to_owned()), - None, - ); - let result3 = fetcher.get_source_file(&module_url, true, false); - assert!(result3.is_ok()); - let r3 = result3.unwrap(); - assert_eq!( - r3.source_code, - "export { printHello } from \"./print_hello.ts\";\n".as_bytes() - ); - // If get_source_file does not call remote, this should be JavaScript - // as we modified before! (we do not overwrite .headers.json due to no http fetch) - assert_eq!(&(r3.media_type), &msg::MediaType::Json); - assert!(fs::read_to_string(&headers_file_name) - .unwrap() - .contains("application/json")); - - // let's create fresh instance of DenoDir (simulating another freshh Deno process) - // and don't use cache - let fetcher = setup_file_fetcher(temp_dir.path()); - let result4 = fetcher.get_source_file(&module_url, false, false); - assert!(result4.is_ok()); - let r4 = result4.unwrap(); - let expected4 = - "export { printHello } from \"./print_hello.ts\";\n".as_bytes(); - assert_eq!(r4.source_code, expected4); - // Now the old .headers.json file should have gone! Resolved back to TypeScript - assert_eq!(&(r4.media_type), &msg::MediaType::TypeScript); - assert!(fs::read_to_string(&headers_file_name).is_err()); - }); + // http_util::fetch_sync_string requires tokio + tokio_util::run(fut); drop(http_server_guard); } @@ -810,67 +797,76 @@ mod tests { fn test_get_source_code_2() { let http_server_guard = crate::test_util::http_server(); let (temp_dir, fetcher) = test_setup(); - // http_util::fetch_sync_string requires tokio - tokio_util::init(|| { - let module_url = - Url::parse("http://localhost:4545/tests/subdir/mismatch_ext.ts") - .unwrap(); - let headers_file_name = fetcher.deps_cache.location.join( - fetcher - .deps_cache - .get_cache_filename_with_extension(&module_url, "headers.json"), - ); + let fetcher_1 = fetcher.clone(); + let module_url = + Url::parse("http://localhost:4545/tests/subdir/mismatch_ext.ts").unwrap(); + let module_url_1 = module_url.clone(); + let module_url_2 = module_url.clone(); + let headers_file_name = fetcher.deps_cache.location.join( + fetcher + .deps_cache + .get_cache_filename_with_extension(&module_url, "headers.json"), + ); - let result = fetcher.get_source_file(&module_url, true, false); - assert!(result.is_ok()); - let r = result.unwrap(); - let expected = "export const loaded = true;\n".as_bytes(); - assert_eq!(r.source_code, expected); - // Mismatch ext with content type, create .headers.json - assert_eq!(&(r.media_type), &msg::MediaType::JavaScript); - assert_eq!( - fetcher - .get_source_code_headers(&module_url) - .mime_type - .unwrap(), - "text/javascript" - ); + let fut = fetcher + .get_source_file_async(&module_url, true, false) + .then(move |result| { + assert!(result.is_ok()); + let r = result.unwrap(); + let expected = "export const loaded = true;\n".as_bytes(); + assert_eq!(r.source_code, expected); + // Mismatch ext with content type, create .headers.json + assert_eq!(&(r.media_type), &msg::MediaType::JavaScript); + assert_eq!( + fetcher + .get_source_code_headers(&module_url) + .mime_type + .unwrap(), + "text/javascript" + ); + + // Modify .headers.json + let _ = fetcher.save_source_code_headers( + &module_url, + Some("text/typescript".to_owned()), + None, + ); + fetcher.get_source_file_async(&module_url, true, false) + }) + .then(move |result2| { + assert!(result2.is_ok()); + let r2 = result2.unwrap(); + let expected2 = "export const loaded = true;\n".as_bytes(); + assert_eq!(r2.source_code, expected2); + // If get_source_file_async does not call remote, this should be TypeScript + // as we modified before! (we do not overwrite .headers.json due to no http fetch) + assert_eq!(&(r2.media_type), &msg::MediaType::TypeScript); + assert!(fs::read_to_string(&headers_file_name).is_err()); + + // let's create fresh instance of DenoDir (simulating another freshh Deno process) + // and don't use cache + let fetcher = setup_file_fetcher(temp_dir.path()); + fetcher.get_source_file_async(&module_url_1, false, false) + }) + .then(move |result3| { + assert!(result3.is_ok()); + let r3 = result3.unwrap(); + let expected3 = "export const loaded = true;\n".as_bytes(); + assert_eq!(r3.source_code, expected3); + // Now the old .headers.json file should be overwritten back to JavaScript! + // (due to http fetch) + assert_eq!(&(r3.media_type), &msg::MediaType::JavaScript); + assert_eq!( + fetcher_1 + .get_source_code_headers(&module_url_2) + .mime_type + .unwrap(), + "text/javascript" + ); + Ok(()) + }); - // Modify .headers.json - let _ = fetcher.save_source_code_headers( - &module_url, - Some("text/typescript".to_owned()), - None, - ); - let result2 = fetcher.get_source_file(&module_url, true, false); - assert!(result2.is_ok()); - let r2 = result2.unwrap(); - let expected2 = "export const loaded = true;\n".as_bytes(); - assert_eq!(r2.source_code, expected2); - // If get_source_file does not call remote, this should be TypeScript - // as we modified before! (we do not overwrite .headers.json due to no http fetch) - assert_eq!(&(r2.media_type), &msg::MediaType::TypeScript); - assert!(fs::read_to_string(&headers_file_name).is_err()); - - // let's create fresh instance of DenoDir (simulating another freshh Deno process) - // and don't use cache - let fetcher = setup_file_fetcher(temp_dir.path()); - let result3 = fetcher.get_source_file(&module_url, false, false); - assert!(result3.is_ok()); - let r3 = result3.unwrap(); - let expected3 = "export const loaded = true;\n".as_bytes(); - assert_eq!(r3.source_code, expected3); - // Now the old .headers.json file should be overwritten back to JavaScript! - // (due to http fetch) - assert_eq!(&(r3.media_type), &msg::MediaType::JavaScript); - assert_eq!( - fetcher - .get_source_code_headers(&module_url) - .mime_type - .unwrap(), - "text/javascript" - ); - }); + tokio_util::run(fut); drop(http_server_guard); } @@ -924,51 +920,56 @@ mod tests { fn test_get_source_code_3() { let http_server_guard = crate::test_util::http_server(); let (_temp_dir, fetcher) = test_setup(); - // Test basic follow and headers recording - tokio_util::init(|| { - let redirect_module_url = - Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") - .unwrap(); - let redirect_source_filepath = fetcher - .deps_cache - .location - .join("http/localhost_PORT4546/tests/subdir/redirects/redirect1.js"); - let redirect_source_filename = - redirect_source_filepath.to_str().unwrap().to_string(); - let target_module_url = - Url::parse("http://localhost:4545/tests/subdir/redirects/redirect1.js") - .unwrap(); - let redirect_target_filepath = fetcher - .deps_cache - .location - .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); - let redirect_target_filename = - redirect_target_filepath.to_str().unwrap().to_string(); - let mod_meta = fetcher - .get_source_file(&redirect_module_url, true, false) + let redirect_module_url = + Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") .unwrap(); - // File that requires redirection is not downloaded. - assert!(fs::read_to_string(&redirect_source_filename).is_err()); - // ... but its .headers.json is created. - let redirect_source_headers = - fetcher.get_source_code_headers(&redirect_module_url); - assert_eq!( - redirect_source_headers.redirect_to.unwrap(), - "http://localhost:4545/tests/subdir/redirects/redirect1.js" - ); - // The target of redirection is downloaded instead. - assert_eq!( - fs::read_to_string(&redirect_target_filename).unwrap(), - "export const redirect = 1;\n" - ); - let redirect_target_headers = - fetcher.get_source_code_headers(&target_module_url); - assert!(redirect_target_headers.redirect_to.is_none()); + let redirect_source_filepath = fetcher + .deps_cache + .location + .join("http/localhost_PORT4546/tests/subdir/redirects/redirect1.js"); + let redirect_source_filename = + redirect_source_filepath.to_str().unwrap().to_string(); + let target_module_url = + Url::parse("http://localhost:4545/tests/subdir/redirects/redirect1.js") + .unwrap(); + let redirect_target_filepath = fetcher + .deps_cache + .location + .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); + let redirect_target_filename = + redirect_target_filepath.to_str().unwrap().to_string(); - // Examine the meta result. - assert_eq!(mod_meta.url.clone(), target_module_url); - }); + // Test basic follow and headers recording + let fut = fetcher + .get_source_file_async(&redirect_module_url, true, false) + .then(move |result| { + assert!(result.is_ok()); + let mod_meta = result.unwrap(); + // File that requires redirection is not downloaded. + assert!(fs::read_to_string(&redirect_source_filename).is_err()); + // ... but its .headers.json is created. + let redirect_source_headers = + fetcher.get_source_code_headers(&redirect_module_url); + assert_eq!( + redirect_source_headers.redirect_to.unwrap(), + "http://localhost:4545/tests/subdir/redirects/redirect1.js" + ); + // The target of redirection is downloaded instead. + assert_eq!( + fs::read_to_string(&redirect_target_filename).unwrap(), + "export const redirect = 1;\n" + ); + let redirect_target_headers = + fetcher.get_source_code_headers(&target_module_url); + assert!(redirect_target_headers.redirect_to.is_none()); + + // Examine the meta result. + assert_eq!(mod_meta.url.clone(), target_module_url); + Ok(()) + }); + + tokio_util::run(fut); drop(http_server_guard); } @@ -976,63 +977,66 @@ mod tests { fn test_get_source_code_4() { let http_server_guard = crate::test_util::http_server(); let (_temp_dir, fetcher) = test_setup(); - // Test double redirects and headers recording - tokio_util::init(|| { - let double_redirect_url = - Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") - .unwrap(); - let double_redirect_path = fetcher - .deps_cache - .location - .join("http/localhost_PORT4548/tests/subdir/redirects/redirect1.js"); - - let redirect_url = - Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") - .unwrap(); - let redirect_path = fetcher - .deps_cache - .location - .join("http/localhost_PORT4546/tests/subdir/redirects/redirect1.js"); - - let target_url = - Url::parse("http://localhost:4545/tests/subdir/redirects/redirect1.js") - .unwrap(); - let target_path = fetcher - .deps_cache - .location - .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); - - let mod_meta = fetcher - .get_source_file(&double_redirect_url, true, false) + let double_redirect_url = + Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") .unwrap(); + let double_redirect_path = fetcher + .deps_cache + .location + .join("http/localhost_PORT4548/tests/subdir/redirects/redirect1.js"); - assert!(fs::read_to_string(&double_redirect_path).is_err()); - assert!(fs::read_to_string(&redirect_path).is_err()); + let redirect_url = + Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") + .unwrap(); + let redirect_path = fetcher + .deps_cache + .location + .join("http/localhost_PORT4546/tests/subdir/redirects/redirect1.js"); - let double_redirect_headers = - fetcher.get_source_code_headers(&double_redirect_url); - assert_eq!( - double_redirect_headers.redirect_to.unwrap(), - redirect_url.to_string() - ); - let redirect_headers = fetcher.get_source_code_headers(&redirect_url); - assert_eq!( - redirect_headers.redirect_to.unwrap(), - target_url.to_string() - ); + let target_url = + Url::parse("http://localhost:4545/tests/subdir/redirects/redirect1.js") + .unwrap(); + let target_path = fetcher + .deps_cache + .location + .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); - // The target of redirection is downloaded instead. - assert_eq!( - fs::read_to_string(&target_path).unwrap(), - "export const redirect = 1;\n" - ); - let redirect_target_headers = - fetcher.get_source_code_headers(&target_url); - assert!(redirect_target_headers.redirect_to.is_none()); + // Test double redirects and headers recording + let fut = fetcher + .get_source_file_async(&double_redirect_url, true, false) + .then(move |result| { + assert!(result.is_ok()); + let mod_meta = result.unwrap(); + assert!(fs::read_to_string(&double_redirect_path).is_err()); + assert!(fs::read_to_string(&redirect_path).is_err()); + + let double_redirect_headers = + fetcher.get_source_code_headers(&double_redirect_url); + assert_eq!( + double_redirect_headers.redirect_to.unwrap(), + redirect_url.to_string() + ); + let redirect_headers = fetcher.get_source_code_headers(&redirect_url); + assert_eq!( + redirect_headers.redirect_to.unwrap(), + target_url.to_string() + ); + + // The target of redirection is downloaded instead. + assert_eq!( + fs::read_to_string(&target_path).unwrap(), + "export const redirect = 1;\n" + ); + let redirect_target_headers = + fetcher.get_source_code_headers(&target_url); + assert!(redirect_target_headers.redirect_to.is_none()); + + // Examine the meta result. + assert_eq!(mod_meta.url.clone(), target_url); + Ok(()) + }); - // Examine the meta result. - assert_eq!(mod_meta.url.clone(), target_url); - }); + tokio_util::run(fut); drop(http_server_guard); } @@ -1040,46 +1044,55 @@ mod tests { fn test_get_source_code_5() { let http_server_guard = crate::test_util::http_server(); let (_temp_dir, fetcher) = test_setup(); - // Test that redirect target is not downloaded twice for different redirect source. - tokio_util::init(|| { - let double_redirect_url = - Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") - .unwrap(); - - let redirect_url = - Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") - .unwrap(); - let target_path = fetcher - .deps_cache - .location - .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); - - fetcher - .get_source_file(&double_redirect_url, true, false) + let double_redirect_url = + Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") .unwrap(); - let result = fs::File::open(&target_path); - assert!(result.is_ok()); - let file = result.unwrap(); - // save modified timestamp for headers file of redirect target - let file_metadata = file.metadata().unwrap(); - let file_modified = file_metadata.modified().unwrap(); + let redirect_url = + Url::parse("http://localhost:4546/tests/subdir/redirects/redirect1.js") + .unwrap(); - // When another file is fetched that also point to redirect target, then redirect target - // shouldn't be downloaded again. It can be verified using source header file creation - // timestamp (should be the same as after first `get_source_file`) - fetcher.get_source_file(&redirect_url, true, false).unwrap(); + let target_path = fetcher + .deps_cache + .location + .join("http/localhost_PORT4545/tests/subdir/redirects/redirect1.js"); + let target_path_ = target_path.clone(); - let result = fs::File::open(&target_path); - assert!(result.is_ok()); - let file_2 = result.unwrap(); - // save modified timestamp for headers file - let file_metadata_2 = file_2.metadata().unwrap(); - let file_modified_2 = file_metadata_2.modified().unwrap(); + // Test that redirect target is not downloaded twice for different redirect source. + let fut = fetcher + .get_source_file_async(&double_redirect_url, true, false) + .then(move |result| { + assert!(result.is_ok()); + let result = fs::File::open(&target_path); + assert!(result.is_ok()); + let file = result.unwrap(); + // save modified timestamp for headers file of redirect target + let file_metadata = file.metadata().unwrap(); + let file_modified = file_metadata.modified().unwrap(); + + // When another file is fetched that also point to redirect target, then redirect target + // shouldn't be downloaded again. It can be verified using source header file creation + // timestamp (should be the same as after first `get_source_file`) + fetcher + .get_source_file_async(&redirect_url, true, false) + .map(move |r| (r, file_modified)) + }) + .then(move |result| { + assert!(result.is_ok()); + let (_, file_modified) = result.unwrap(); + let result = fs::File::open(&target_path_); + assert!(result.is_ok()); + let file_2 = result.unwrap(); + // save modified timestamp for headers file + let file_metadata_2 = file_2.metadata().unwrap(); + let file_modified_2 = file_metadata_2.modified().unwrap(); + + assert_eq!(file_modified, file_modified_2); + Ok(()) + }); - assert_eq!(file_modified, file_modified_2); - }); + tokio_util::run(fut); drop(http_server_guard); } @@ -1087,21 +1100,25 @@ mod tests { fn test_get_source_code_6() { let http_server_guard = crate::test_util::http_server(); let (_temp_dir, fetcher) = test_setup(); + let double_redirect_url = + Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") + .unwrap(); + // Test that redirections can be limited - tokio_util::init(|| { - let double_redirect_url = - Url::parse("http://localhost:4548/tests/subdir/redirects/redirect1.js") - .unwrap(); + let fut = fetcher + .fetch_remote_source_async(&double_redirect_url, false, false, 2) + .then(move |result| { + assert!(result.is_ok()); + fetcher.fetch_remote_source_async(&double_redirect_url, false, false, 1) + }) + .then(move |result| { + assert!(result.is_err()); + let err = result.err().unwrap(); + assert_eq!(err.kind(), ErrorKind::TooManyRedirects); + Ok(()) + }); - let result = - fetcher.fetch_remote_source(&double_redirect_url, false, false, 2); - assert!(result.is_ok()); - let result = - fetcher.fetch_remote_source(&double_redirect_url, false, false, 1); - assert!(result.is_err()); - let err = result.err().unwrap(); - assert_eq!(err.kind(), ErrorKind::TooManyRedirects); - }); + tokio_util::run(fut); drop(http_server_guard); } @@ -1109,166 +1126,188 @@ mod tests { fn test_get_source_code_no_fetch() { let http_server_guard = crate::test_util::http_server(); let (_temp_dir, fetcher) = test_setup(); - tokio_util::init(|| { - let module_url = - Url::parse("http://localhost:4545/tests/002_hello.ts").unwrap(); - - // file hasn't been cached before and remote downloads are not allowed - let result = fetcher.get_source_file(&module_url, true, true); - assert!(result.is_err()); - let err = result.err().unwrap(); - assert_eq!(err.kind(), ErrorKind::NotFound); + let fetcher_1 = fetcher.clone(); + let fetcher_2 = fetcher.clone(); + let module_url = + Url::parse("http://localhost:4545/tests/002_hello.ts").unwrap(); + let module_url_1 = module_url.clone(); + let module_url_2 = module_url.clone(); + // file hasn't been cached before and remote downloads are not allowed + let fut = fetcher + .get_source_file_async(&module_url, true, true) + .then(move |result| { + assert!(result.is_err()); + let err = result.err().unwrap(); + assert_eq!(err.kind(), ErrorKind::NotFound); - // download and cache file - let result = fetcher.get_source_file(&module_url, true, false); - assert!(result.is_ok()); + // download and cache file + fetcher_1.get_source_file_async(&module_url_1, true, false) + }) + .then(move |result| { + assert!(result.is_ok()); + // module is already cached, should be ok even with `no_remote_fetch` + fetcher_2.get_source_file_async(&module_url_2, true, true) + }) + .then(move |result| { + assert!(result.is_ok()); + Ok(()) + }); - // module is already cached, should be ok even with `no_remote_fetch` - let result = fetcher.get_source_file(&module_url, true, true); - assert!(result.is_ok()); - }); + tokio_util::run(fut); drop(http_server_guard); } #[test] fn test_fetch_source_async_1() { let http_server_guard = crate::test_util::http_server(); - tokio_util::init(|| { - let (_temp_dir, fetcher) = test_setup(); - let module_url = - Url::parse("http://127.0.0.1:4545/tests/subdir/mt_video_mp2t.t3.ts") - .unwrap(); - let headers_file_name = fetcher.deps_cache.location.join( - fetcher - .deps_cache - .get_cache_filename_with_extension(&module_url, "headers.json"), - ); + let (_temp_dir, fetcher) = test_setup(); + let module_url = + Url::parse("http://127.0.0.1:4545/tests/subdir/mt_video_mp2t.t3.ts") + .unwrap(); + let headers_file_name = fetcher.deps_cache.location.join( + fetcher + .deps_cache + .get_cache_filename_with_extension(&module_url, "headers.json"), + ); - let result = tokio_util::block_on(fetcher.fetch_remote_source_async( - &module_url, - false, - false, - 10, - )); - assert!(result.is_ok()); - let r = result.unwrap(); - assert_eq!(r.source_code, b"export const loaded = true;\n"); - assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); - // matching ext, no .headers.json file created - assert!(fs::read_to_string(&headers_file_name).is_err()); - - // Modify .headers.json, make sure read from local - let _ = fetcher.save_source_code_headers( - &module_url, - Some("text/javascript".to_owned()), - None, - ); - let result2 = fetcher.fetch_cached_remote_source(&module_url); - assert!(result2.is_ok()); - let r2 = result2.unwrap().unwrap(); - assert_eq!(r2.source_code, b"export const loaded = true;\n"); - // Not MediaType::TypeScript due to .headers.json modification - assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); - }); + let fut = fetcher + .fetch_remote_source_async(&module_url, false, false, 10) + .then(move |result| { + assert!(result.is_ok()); + let r = result.unwrap(); + assert_eq!(r.source_code, b"export const loaded = true;\n"); + assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); + // matching ext, no .headers.json file created + assert!(fs::read_to_string(&headers_file_name).is_err()); + // Modify .headers.json, make sure read from local + let _ = fetcher.save_source_code_headers( + &module_url, + Some("text/javascript".to_owned()), + None, + ); + let result2 = fetcher.fetch_cached_remote_source(&module_url); + assert!(result2.is_ok()); + let r2 = result2.unwrap().unwrap(); + assert_eq!(r2.source_code, b"export const loaded = true;\n"); + // Not MediaType::TypeScript due to .headers.json modification + assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); + Ok(()) + }); + + tokio_util::run(fut); drop(http_server_guard); } #[test] fn test_fetch_source_1() { let http_server_guard = crate::test_util::http_server(); - tokio_util::init(|| { - let (_temp_dir, fetcher) = test_setup(); - let module_url = - Url::parse("http://localhost:4545/tests/subdir/mt_video_mp2t.t3.ts") - .unwrap(); - let headers_file_name = fetcher.deps_cache.location.join( - fetcher - .deps_cache - .get_cache_filename_with_extension(&module_url, "headers.json"), - ); - let result = fetcher.fetch_remote_source(&module_url, false, false, 10); - assert!(result.is_ok()); - let r = result.unwrap(); - assert_eq!(r.source_code, "export const loaded = true;\n".as_bytes()); - assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); - // matching ext, no .headers.json file created - assert!(fs::read_to_string(&headers_file_name).is_err()); - - // Modify .headers.json, make sure read from local - let _ = fetcher.save_source_code_headers( - &module_url, - Some("text/javascript".to_owned()), - None, - ); - let result2 = fetcher.fetch_cached_remote_source(&module_url); - assert!(result2.is_ok()); - let r2 = result2.unwrap().unwrap(); - assert_eq!(r2.source_code, "export const loaded = true;\n".as_bytes()); - // Not MediaType::TypeScript due to .headers.json modification - assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); - }); + let (_temp_dir, fetcher) = test_setup(); + let module_url = + Url::parse("http://localhost:4545/tests/subdir/mt_video_mp2t.t3.ts") + .unwrap(); + let headers_file_name = fetcher.deps_cache.location.join( + fetcher + .deps_cache + .get_cache_filename_with_extension(&module_url, "headers.json"), + ); + + let fut = fetcher + .fetch_remote_source_async(&module_url, false, false, 10) + .then(move |result| { + assert!(result.is_ok()); + let r = result.unwrap(); + assert_eq!(r.source_code, "export const loaded = true;\n".as_bytes()); + assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); + // matching ext, no .headers.json file created + assert!(fs::read_to_string(&headers_file_name).is_err()); + + // Modify .headers.json, make sure read from local + let _ = fetcher.save_source_code_headers( + &module_url, + Some("text/javascript".to_owned()), + None, + ); + let result2 = fetcher.fetch_cached_remote_source(&module_url); + assert!(result2.is_ok()); + let r2 = result2.unwrap().unwrap(); + assert_eq!(r2.source_code, "export const loaded = true;\n".as_bytes()); + // Not MediaType::TypeScript due to .headers.json modification + assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); + Ok(()) + }); + + tokio_util::run(fut); drop(http_server_guard); } #[test] fn test_fetch_source_2() { let http_server_guard = crate::test_util::http_server(); - tokio_util::init(|| { - let (_temp_dir, fetcher) = test_setup(); - let module_url = - Url::parse("http://localhost:4545/tests/subdir/no_ext").unwrap(); - let result = fetcher.fetch_remote_source(&module_url, false, false, 10); - assert!(result.is_ok()); - let r = result.unwrap(); - assert_eq!(r.source_code, "export const loaded = true;\n".as_bytes()); - assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); - // no ext, should create .headers.json file - assert_eq!( - fetcher - .get_source_code_headers(&module_url) - .mime_type - .unwrap(), - "text/typescript" - ); + let (_temp_dir, fetcher) = test_setup(); + let fetcher_1 = fetcher.clone(); + let fetcher_2 = fetcher.clone(); + let fetcher_3 = fetcher.clone(); + let module_url = + Url::parse("http://localhost:4545/tests/subdir/no_ext").unwrap(); + let module_url_2 = + Url::parse("http://localhost:4545/tests/subdir/mismatch_ext.ts").unwrap(); + let module_url_2_ = module_url_2.clone(); + let module_url_3 = + Url::parse("http://localhost:4545/tests/subdir/unknown_ext.deno") + .unwrap(); + let module_url_3_ = module_url_3.clone(); - let module_url_2 = - Url::parse("http://localhost:4545/tests/subdir/mismatch_ext.ts") - .unwrap(); - let result_2 = - fetcher.fetch_remote_source(&module_url_2, false, false, 10); - assert!(result_2.is_ok()); - let r2 = result_2.unwrap(); - assert_eq!(r2.source_code, "export const loaded = true;\n".as_bytes()); - assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); - // mismatch ext, should create .headers.json file - assert_eq!( - fetcher - .get_source_code_headers(&module_url_2) - .mime_type - .unwrap(), - "text/javascript" - ); + let fut = fetcher + .fetch_remote_source_async(&module_url, false, false, 10) + .then(move |result| { + assert!(result.is_ok()); + let r = result.unwrap(); + assert_eq!(r.source_code, "export const loaded = true;\n".as_bytes()); + assert_eq!(&(r.media_type), &msg::MediaType::TypeScript); + // no ext, should create .headers.json file + assert_eq!( + fetcher_1 + .get_source_code_headers(&module_url) + .mime_type + .unwrap(), + "text/typescript" + ); + fetcher_1.fetch_remote_source_async(&module_url_2, false, false, 10) + }) + .then(move |result| { + assert!(result.is_ok()); + let r2 = result.unwrap(); + assert_eq!(r2.source_code, "export const loaded = true;\n".as_bytes()); + assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); + // mismatch ext, should create .headers.json file + assert_eq!( + fetcher_2 + .get_source_code_headers(&module_url_2_) + .mime_type + .unwrap(), + "text/javascript" + ); + // test unknown extension + fetcher_2.fetch_remote_source_async(&module_url_3, false, false, 10) + }) + .then(move |result| { + assert!(result.is_ok()); + let r3 = result.unwrap(); + assert_eq!(r3.source_code, "export const loaded = true;\n".as_bytes()); + assert_eq!(&(r3.media_type), &msg::MediaType::TypeScript); + // unknown ext, should create .headers.json file + assert_eq!( + fetcher_3 + .get_source_code_headers(&module_url_3_) + .mime_type + .unwrap(), + "text/typescript" + ); + futures::future::ok(()) + }); - // test unknown extension - let module_url_3 = - Url::parse("http://localhost:4545/tests/subdir/unknown_ext.deno") - .unwrap(); - let result_3 = - fetcher.fetch_remote_source(&module_url_3, false, false, 10); - assert!(result_3.is_ok()); - let r3 = result_3.unwrap(); - assert_eq!(r3.source_code, "export const loaded = true;\n".as_bytes()); - assert_eq!(&(r3.media_type), &msg::MediaType::TypeScript); - // unknown ext, should create .headers.json file - assert_eq!( - fetcher - .get_source_code_headers(&module_url_3) - .mime_type - .unwrap(), - "text/typescript" - ); - }); + tokio_util::run(fut); drop(http_server_guard); } diff --git a/cli/http_util.rs b/cli/http_util.rs index abeeffb7a339dc..754cd60d28ca91 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -139,22 +139,22 @@ mod tests { use super::*; use crate::tokio_util; - pub fn fetch_string_once_sync(url: &Url) -> Result { - tokio_util::block_on(fetch_string_once(url)) - } - #[test] fn test_fetch_sync_string() { let http_server_guard = crate::test_util::http_server(); // Relies on external http server. See tools/http_server.py let url = Url::parse("http://127.0.0.1:4545/package.json").unwrap(); - tokio_util::init(|| match fetch_string_once_sync(&url).unwrap() { - FetchOnceResult::Code(code, maybe_content_type) => { + + let fut = fetch_string_once(&url).then(|result| match result { + Ok(FetchOnceResult::Code(code, maybe_content_type)) => { assert!(!code.is_empty()); assert_eq!(maybe_content_type, Some("application/json".to_string())); + Ok(()) } - _ => unreachable!(), + _ => panic!(), }); + + tokio_util::run(fut); drop(http_server_guard); } @@ -165,10 +165,15 @@ mod tests { let url = Url::parse("http://127.0.0.1:4546/package.json").unwrap(); // Dns resolver substitutes `127.0.0.1` with `localhost` let target_url = Url::parse("http://localhost:4545/package.json").unwrap(); - tokio_util::init(|| { - let result = fetch_string_once_sync(&url).unwrap(); - assert_eq!(result, FetchOnceResult::Redirect(target_url)); + let fut = fetch_string_once(&url).then(move |result| match result { + Ok(FetchOnceResult::Redirect(url)) => { + assert_eq!(url, target_url); + Ok(()) + } + _ => panic!(), }); + + tokio_util::run(fut); drop(http_server_guard); } diff --git a/cli/worker.rs b/cli/worker.rs index 9bdf2ae08b5293..41de7d1ed232a8 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -4,7 +4,6 @@ use crate::ops::json_op; use crate::ops::minimal_op; use crate::ops::*; use crate::state::ThreadSafeState; -use crate::tokio_util; use deno; use deno::ErrBox; use deno::ModuleSpecifier; @@ -345,15 +344,6 @@ impl Worker { } }) } - - /// Executes the provided JavaScript module. - pub fn execute_mod( - &mut self, - module_specifier: &ModuleSpecifier, - is_prefetch: bool, - ) -> Result<(), ErrBox> { - tokio_util::block_on(self.execute_mod_async(module_specifier, is_prefetch)) - } } impl Future for Worker { @@ -399,11 +389,14 @@ mod tests { tokio_util::run(lazy(move || { let mut worker = Worker::new("TEST".to_string(), StartupData::None, state); - let result = worker.execute_mod(&module_specifier, false); - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) + worker + .execute_mod_async(&module_specifier, false) + .then(|result| { + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker) + }) })); let metrics = &state_.metrics; @@ -433,11 +426,14 @@ mod tests { tokio_util::run(lazy(move || { let mut worker = Worker::new("TEST".to_string(), StartupData::None, state); - let result = worker.execute_mod(&module_specifier, false); - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) + worker + .execute_mod_async(&module_specifier, false) + .then(|result| { + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker) + }) })); let metrics = &state_.metrics; @@ -449,6 +445,7 @@ mod tests { #[test] fn execute_006_url_imports() { let http_server_guard = crate::test_util::http_server(); + let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) .parent() .unwrap() @@ -469,11 +466,14 @@ mod tests { state, ); worker.execute("denoMain()").unwrap(); - let result = worker.execute_mod(&module_specifier, false); - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) + worker + .execute_mod_async(&module_specifier, false) + .then(|result| { + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker) + }) })); let metrics = &state_.metrics;