From 55add2d366c5b3e19bd91958f3e3a36b4439839d Mon Sep 17 00:00:00 2001 From: Gurwinder Singh Date: Wed, 1 Jan 2020 20:21:27 +0530 Subject: [PATCH] cleanup after tokio upgrade (#3571) tokio_util::run and tokio::run_on_current_thread should accept Future instead of Future>. Currently, all the passed futures have to add Ok(()) or futures::future::ok(()) unnecessarily to call this method. --- cli/compilers/ts.rs | 2 -- cli/file_fetcher.rs | 51 +++++++++++++++------------------------------ cli/http_util.rs | 6 ++---- cli/lib.rs | 6 ------ cli/tokio_util.rs | 8 +++---- cli/worker.rs | 13 ++++++------ 6 files changed, 29 insertions(+), 57 deletions(-) diff --git a/cli/compilers/ts.rs b/cli/compilers/ts.rs index c6528dd5b4..fda98a7941 100644 --- a/cli/compilers/ts.rs +++ b/cli/compilers/ts.rs @@ -641,7 +641,6 @@ mod tests { .code .as_bytes() .starts_with("console.log(\"Hello World\");".as_bytes())); - Ok(()) }; tokio_util::run(fut.boxed()) @@ -675,7 +674,6 @@ mod tests { .await; assert!(result.is_ok()); - Ok(()) }; tokio_util::run(fut.boxed()) } diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 0d89ab2602..382ced24fd 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -929,7 +929,7 @@ mod tests { let fetcher = setup_file_fetcher(temp_dir.path()); fetcher.get_source_file_async(&module_url_2, false, false, false) }) - .then(move |result4| { + .map(move |result4| { assert!(result4.is_ok()); let r4 = result4.unwrap(); let expected4 = @@ -938,7 +938,6 @@ mod tests { // 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()); - futures::future::ok(()) }); // http_util::fetch_sync_string requires tokio @@ -1001,7 +1000,7 @@ mod tests { let fetcher = setup_file_fetcher(temp_dir.path()); fetcher.get_source_file_async(&module_url_1, false, false, false) }) - .then(move |result3| { + .map(move |result3| { assert!(result3.is_ok()); let r3 = result3.unwrap(); let expected3 = "export const loaded = true;\n".as_bytes(); @@ -1016,7 +1015,6 @@ mod tests { .unwrap(), "text/javascript" ); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1038,10 +1036,9 @@ mod tests { ); // first download - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_ok()); - futures::future::ok(()) }, )); @@ -1055,10 +1052,9 @@ mod tests { // download file again, it should use already fetched file even though `use_disk_cache` is set to // false, this can be verified using source header file creation timestamp (should be // the same as after first download) - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_ok()); - futures::future::ok(()) }, )); @@ -1100,7 +1096,7 @@ mod tests { // Test basic follow and headers recording let fut = fetcher .get_source_file_async(&redirect_module_url, true, false, false) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); let mod_meta = result.unwrap(); // File that requires redirection is not downloaded. @@ -1123,7 +1119,6 @@ mod tests { // Examine the meta result. assert_eq!(mod_meta.url, target_module_url); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1161,7 +1156,7 @@ mod tests { // Test double redirects and headers recording let fut = fetcher .get_source_file_async(&double_redirect_url, true, false, false) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); let mod_meta = result.unwrap(); assert!(fs::read_to_string(&double_redirect_path).is_err()); @@ -1190,7 +1185,6 @@ mod tests { // Examine the meta result. assert_eq!(mod_meta.url, target_url); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1235,7 +1229,7 @@ mod tests { .get_source_file_async(&redirect_url, true, false, false) .map(move |r| (r, file_modified)) }) - .then(move |(result, file_modified)| { + .map(move |(result, file_modified)| { assert!(result.is_ok()); let result = fs::File::open(&target_path_); assert!(result.is_ok()); @@ -1245,7 +1239,6 @@ mod tests { let file_modified_2 = file_metadata_2.modified().unwrap(); assert_eq!(file_modified, file_modified_2); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1267,11 +1260,10 @@ mod tests { assert!(result.is_ok()); fetcher.fetch_remote_source_async(&double_redirect_url, false, false, 1) }) - .then(move |result| { + .map(move |result| { assert!(result.is_err()); let err = result.err().unwrap(); assert_eq!(err.kind(), ErrorKind::TooManyRedirects); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1287,11 +1279,10 @@ mod tests { // Remote modules are not allowed let fut = fetcher .get_source_file_async(&module_url, true, true, false) - .then(move |result| { + .map(move |result| { assert!(result.is_err()); let err = result.err().unwrap(); assert_eq!(err.kind(), ErrorKind::NotFound); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1324,9 +1315,8 @@ mod tests { // module is already cached, should be ok even with `cached_only` fetcher_2.get_source_file_async(&module_url_2, true, false, true) }) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1348,7 +1338,7 @@ mod tests { let fut = fetcher .fetch_remote_source_async(&module_url, false, false, 10) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); let r = result.unwrap(); assert_eq!(r.source_code, b"export const loaded = true;\n"); @@ -1367,7 +1357,6 @@ mod tests { 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); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1390,7 +1379,7 @@ mod tests { let fut = fetcher .fetch_remote_source_async(&module_url, false, false, 10) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); let r = result.unwrap(); assert_eq!(r.source_code, "export const loaded = true;\n".as_bytes()); @@ -1410,7 +1399,6 @@ mod tests { 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); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1467,7 +1455,7 @@ mod tests { // test unknown extension fetcher_2.fetch_remote_source_async(&module_url_3, false, false, 10) }) - .then(move |result| { + .map(move |result| { assert!(result.is_ok()); let r3 = result.unwrap(); assert_eq!(r3.source_code, "export const loaded = true;\n".as_bytes()); @@ -1480,7 +1468,6 @@ mod tests { .unwrap(), "text/typescript" ); - futures::future::ok(()) }); tokio_util::run(fut); @@ -1494,10 +1481,9 @@ mod tests { // Test failure case. let specifier = ModuleSpecifier::resolve_url(file_url!("/baddir/hello.ts")).unwrap(); - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_err()); - futures::future::ok(()) }, )); @@ -1505,10 +1491,9 @@ mod tests { std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("js/main.ts"); let specifier = ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap(); - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_ok()); - futures::future::ok(()) }, )); } @@ -1521,10 +1506,9 @@ mod tests { // Test failure case. let specifier = ModuleSpecifier::resolve_url(file_url!("/baddir/hello.ts")).unwrap(); - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_err()); - futures::future::ok(()) }, )); @@ -1532,10 +1516,9 @@ mod tests { std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("js/main.ts"); let specifier = ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap(); - tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then( + tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).map( |r| { assert!(r.is_ok()); - futures::future::ok(()) }, )); } diff --git a/cli/http_util.rs b/cli/http_util.rs index 4a925e3d9a..a1c2fa3ac2 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -215,11 +215,10 @@ mod tests { let url = Url::parse("http://127.0.0.1:4545/cli/tests/fixture.json").unwrap(); - let fut = fetch_string_once(&url).then(|result| match result { + let fut = fetch_string_once(&url).map(|result| match result { Ok(FetchOnceResult::Code(code, maybe_content_type)) => { assert!(!code.is_empty()); assert_eq!(maybe_content_type, Some("application/json".to_string())); - futures::future::ok(()) } _ => panic!(), }); @@ -237,10 +236,9 @@ mod tests { // Dns resolver substitutes `127.0.0.1` with `localhost` let target_url = Url::parse("http://localhost:4545/cli/tests/fixture.json").unwrap(); - let fut = fetch_string_once(&url).then(move |result| match result { + let fut = fetch_string_once(&url).map(move |result| match result { Ok(FetchOnceResult::Redirect(url)) => { assert_eq!(url, target_url); - futures::future::ok(()) } _ => panic!(), }); diff --git a/cli/lib.rs b/cli/lib.rs index 096bd7abfa..85133158eb 100644 --- a/cli/lib.rs +++ b/cli/lib.rs @@ -269,7 +269,6 @@ fn info_command(flags: DenoFlags) { print_file_info(worker.clone(), main_module.clone()).await; let result = worker.await; js_check(result); - Ok(()) }; tokio_util::run(main_future); @@ -287,7 +286,6 @@ fn fetch_command(flags: DenoFlags) { let main_future = async move { let result = worker.execute_mod_async(&main_module, None, true).await; js_check(result); - Ok(()) }; tokio_util::run(main_future); @@ -315,7 +313,6 @@ fn eval_command(flags: DenoFlags) { let result = worker.await; js_check(result); js_check(worker_.execute("window.dispatchEvent(new Event('unload'))")); - Ok(()) }; tokio_util::run(main_future); @@ -341,7 +338,6 @@ fn bundle_command(flags: DenoFlags) { print_err_and_exit(err); } debug!(">>>>> bundle_async END"); - Ok(()) }; tokio_util::run(main_future); } @@ -358,7 +354,6 @@ fn run_repl(flags: DenoFlags) { let main_future = async move { let result = worker.await; js_check(result); - Ok(()) }; tokio_util::run(main_future); } @@ -400,7 +395,6 @@ fn run_script(flags: DenoFlags) { let result = worker.await; js_check(result); js_check(worker_.execute("window.dispatchEvent(new Event('unload'))")); - Ok(()) }; if use_current_thread { diff --git a/cli/tokio_util.rs b/cli/tokio_util.rs index ec1e0e3cf3..0b8f601524 100644 --- a/cli/tokio_util.rs +++ b/cli/tokio_util.rs @@ -5,7 +5,7 @@ use tokio::runtime; pub fn run(future: F) where - F: Future> + Send + 'static, + F: Future + Send + 'static, { let mut rt = runtime::Builder::new() .threaded_scheduler() @@ -13,12 +13,12 @@ where .thread_name("deno") .build() .expect("Unable to create Tokio runtime"); - rt.block_on(future).unwrap(); + rt.block_on(future); } pub fn run_on_current_thread(future: F) where - F: Future> + Send + 'static, + F: Future + Send + 'static, { let mut rt = runtime::Builder::new() .basic_scheduler() @@ -26,5 +26,5 @@ where .thread_name("deno") .build() .expect("Unable to create Tokio runtime"); - rt.block_on(future).unwrap(); + rt.block_on(future); } diff --git a/cli/worker.rs b/cli/worker.rs index c44dfbb39a..2e995ebe63 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -222,20 +222,19 @@ mod tests { where F: FnOnce() + Send + 'static, { - let fut = futures::future::lazy(move |_cx| { - f(); - Ok(()) - }); - + let fut = futures::future::lazy(move |_cx| f()); tokio_util::run(fut) } - pub fn panic_on_error(f: F) -> impl Future> + pub async fn panic_on_error(f: F) -> I where F: Future>, E: std::fmt::Debug, { - f.map_err(|err| panic!("Future got unexpected error: {:?}", err)) + match f.await { + Ok(v) => v, + Err(e) => panic!("Future got unexpected error: {:?}", e), + } } #[test]