From 5c04f61db5cfc1fba2f55eed07ce423a6c114909 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Sun, 15 Sep 2019 09:48:16 +0900 Subject: [PATCH 01/10] tools: reorg tests --- Cargo.lock | 4 ++ Cargo.toml | 1 + cli/main.rs | 2 + cli/misc_tests.rs | 63 +++++++++++++++++++++++++++++++ cli/tty_tests/Cargo.toml | 8 ++++ cli/tty_tests/lib.rs | 23 +++++++++++ tools/complex_permissions_test.py | 3 +- tools/fetch_test.py | 3 +- tools/target_test.py | 8 ---- tools/test.py | 46 +++++----------------- tools/unit_tests.py | 3 +- 11 files changed, 114 insertions(+), 50 deletions(-) create mode 100644 cli/misc_tests.rs create mode 100644 cli/tty_tests/Cargo.toml create mode 100644 cli/tty_tests/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 05632eba376136..fe7d5e17327b67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -318,6 +318,10 @@ dependencies = [ "deno_typescript 0.18.0", ] +[[package]] +name = "deno_tty_tests" +version = "0.18.0" + [[package]] name = "deno_typescript" version = "0.18.0" diff --git a/Cargo.toml b/Cargo.toml index 084258c324fcfd..a6c139fecf39a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [workspace] members = [ "cli", + "cli/tty_tests", "core", "tools/hyper_hello", "deno_typescript", diff --git a/cli/main.rs b/cli/main.rs index 30acb3ebd1bbe7..abbb84c49eaf8c 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -20,6 +20,8 @@ extern crate url; #[cfg(test)] mod integration_tests; +#[cfg(test)] +mod misc_tests; mod ansi; mod assets; diff --git a/cli/misc_tests.rs b/cli/misc_tests.rs new file mode 100644 index 00000000000000..2e0d24972ffa43 --- /dev/null +++ b/cli/misc_tests.rs @@ -0,0 +1,63 @@ +// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +use std::process::Command; + +fn run_python_script(script: &str) -> bool { + let mut child = Command::new("python") + .current_dir("../") + .arg(script) + .spawn() + .expect("failed to spawn script"); + + let ecode = child.wait().expect("failed to wait for the child process"); + + ecode.success() +} + +#[test] +fn benchmark_test() { + assert!(run_python_script("tools/benchmark_test.py")) +} + +#[test] +fn deno_dir_test() { + assert!(run_python_script("tools/deno_dir_test.py")) +} + +// TODO(#2933): Rewrite this test in rust. +#[test] +fn fetch_test() { + assert!(run_python_script("tools/fetch_test.py")) +} + +// TODO(#2933): Rewrite this test in rust. +#[test] +fn fmt_test() { + assert!(run_python_script("tools/fmt_test.py")) +} + +// TODO(#2933): Rewrite this test in rust. +#[test] +fn js_unit_tests() { + assert!(run_python_script("tools/unit_tests.py")) +} + +// TODO(#2933): Rewrite this test in rust. +#[test] +fn repl_test() { + assert!(run_python_script("tools/repl_test.py")) +} + +#[test] +fn setup_test() { + assert!(run_python_script("tools/setup_test.py")) +} + +#[test] +fn target_test() { + assert!(run_python_script("tools/target_test.py")) +} + +#[test] +fn util_test() { + assert!(run_python_script("tools/util_test.py")) +} diff --git a/cli/tty_tests/Cargo.toml b/cli/tty_tests/Cargo.toml new file mode 100644 index 00000000000000..7ad3beee9cb4f9 --- /dev/null +++ b/cli/tty_tests/Cargo.toml @@ -0,0 +1,8 @@ +# Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +[lib] +path = "lib.rs" + +[package] +name = "deno_tty_tests" +version = "0.18.0" +edition = "2018" diff --git a/cli/tty_tests/lib.rs b/cli/tty_tests/lib.rs new file mode 100644 index 00000000000000..259f6be07d29f1 --- /dev/null +++ b/cli/tty_tests/lib.rs @@ -0,0 +1,23 @@ +// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +use std::process::Command; + +pub fn run_python_script(script: &str) -> bool { + let mut child = Command::new("python") + .current_dir("../../") + .arg(script) + .spawn() + .expect("failed to spawn script"); + + let ecode = child.wait().expect("failed to wait for the child process"); + + ecode.success() +} + +// TODO(#2933): Rewrite these tests in rust. +#[test] +fn tty_tests() { + // FIXME: These tests can't run in parallel. + assert!(run_python_script("tools/complex_permissions_test.py")); + assert!(run_python_script("tools/is_tty_test.py")); + assert!(run_python_script("tools/permission_prompt_test.py")); +} diff --git a/tools/complex_permissions_test.py b/tools/complex_permissions_test.py index 32385649bad1bd..cd9407259a6303 100755 --- a/tools/complex_permissions_test.py +++ b/tools/complex_permissions_test.py @@ -215,5 +215,4 @@ def complex_permissions_tests(): if __name__ == "__main__": - with http_server.spawn(): - run_tests() + run_tests() diff --git a/tools/fetch_test.py b/tools/fetch_test.py index b4bf1836c95e1f..e6e5cb6a0f7e2d 100755 --- a/tools/fetch_test.py +++ b/tools/fetch_test.py @@ -29,5 +29,4 @@ def test_fetch(self): if __name__ == "__main__": - with http_server.spawn(): - run_tests() + run_tests() diff --git a/tools/target_test.py b/tools/target_test.py index cd5e1b7d117513..886c04a4071215 100644 --- a/tools/target_test.py +++ b/tools/target_test.py @@ -21,14 +21,6 @@ def _test(self, executable): self.check_exists(bin_file) run([bin_file], quiet=True) - def test_cargo_test(self): - cargo_test = ["cargo", "test", "--all", "--locked"] - if "DENO_BUILD_MODE" in os.environ and \ - os.environ["DENO_BUILD_MODE"] == "release": - run(cargo_test + ["--release"]) - else: - run(cargo_test) - def test_libdeno(self): self._test("libdeno_test") diff --git a/tools/test.py b/tools/test.py index 9cd2f7a46bfcd8..47a369346d1e93 100755 --- a/tools/test.py +++ b/tools/test.py @@ -4,24 +4,9 @@ # Usage: ./tools/test.py out/Debug import os -from benchmark_test import TestBenchmark -from deno_dir_test import TestDenoDir -from fetch_test import TestFetch -from fmt_test import TestFmt -from repl_test import TestRepl -from setup_test import TestSetup -from target_test import TestTarget -from unit_tests import JsUnitTests -from util_test import TestUtil -# NOTE: These tests are skipped on Windows -from is_tty_test import TestIsTty -from permission_prompt_test import permission_prompt_tests -from complex_permissions_test import complex_permissions_tests - import http_server -from util import (enable_ansi_colors, build_path, RESET, FG_RED, FG_GREEN, - executable_suffix, rmtree, tests_path) -from test_util import parse_test_args, run_tests +from util import enable_ansi_colors, rmtree, run +from test_util import parse_test_args def main(): @@ -34,25 +19,14 @@ def main(): enable_ansi_colors() - test_cases = [ - TestSetup, - TestUtil, - TestTarget, - JsUnitTests, - TestFetch, - TestRepl, - TestDenoDir, - TestBenchmark, - TestIsTty, - ] - test_cases += permission_prompt_tests() - test_cases += complex_permissions_tests() - # It is very slow, so do TestFmt at the end. - test_cases += [TestFmt] - - with http_server.spawn(): - run_tests(test_cases) + cargo_test = ["cargo", "test", "--locked"] + if "DENO_BUILD_MODE" in os.environ and \ + os.environ["DENO_BUILD_MODE"] == "release": + run(cargo_test + ["--release"]) + else: + run(cargo_test) if __name__ == '__main__': - main() + with http_server.spawn(): + main() diff --git a/tools/unit_tests.py b/tools/unit_tests.py index 859afcea33bbcb..528d018c043180 100755 --- a/tools/unit_tests.py +++ b/tools/unit_tests.py @@ -24,5 +24,4 @@ def test_unit_test_runner(self): if __name__ == '__main__': - with http_server.spawn(): - run_tests() + run_tests() From 026d3fa48c508375dcf7ec8c53610b64a4a96b78 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Mon, 16 Sep 2019 23:47:12 +0900 Subject: [PATCH 02/10] refactor: make test.py a simple wrapper of cargo test --- tools/test.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/test.py b/tools/test.py index 47a369346d1e93..f77a82893ec164 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1,30 +1,30 @@ #!/usr/bin/env python # Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. # Runs the full test suite. -# Usage: ./tools/test.py out/Debug +# This script is wrapper of `cargo test`. +# +# Usage: ./tools/test.py +# +# See `./tools/test.py -h` for the available options. import os +import sys import http_server -from util import enable_ansi_colors, rmtree, run -from test_util import parse_test_args +from util import build_path, enable_ansi_colors, rmtree, run def main(): - args = parse_test_args() - - deno_dir = os.path.join(args.build_dir, ".deno_test") + deno_dir = os.path.join(build_path(), ".deno_test") if os.path.isdir(deno_dir): rmtree(deno_dir) os.environ["DENO_DIR"] = deno_dir enable_ansi_colors() - cargo_test = ["cargo", "test", "--locked"] - if "DENO_BUILD_MODE" in os.environ and \ - os.environ["DENO_BUILD_MODE"] == "release": - run(cargo_test + ["--release"]) - else: - run(cargo_test) + cmd = ["cargo", "test", "--locked"] + sys.argv[1:] + if "--release" in cmd: + os.environ["DENO_BUILD_MODE"] = "release" + run(cmd) if __name__ == '__main__': From a3d45c494a21c8ce38035d751b3f544ecd8f37e4 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Tue, 17 Sep 2019 02:19:10 +0900 Subject: [PATCH 03/10] fix: build mode release in travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index ea2bcc321a8179..6e87e217b04194 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,7 +55,7 @@ before_script: # Default script for release builds. script: - cargo build -vv --release --locked --all-targets -- DENO_BUILD_MODE=release ./tools/test.py +- ./tools/test.py --release # For some reason it's faster to run clippy after build. - rustup component add clippy - cargo clippy --all-targets --release --locked -- -D clippy::all From 8d72ef966f138fc7d73f43c04cc2a13626e34553 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 12:06:31 -0400 Subject: [PATCH 04/10] WIP allow http_server.py to be started from rust --- cli/file_fetcher.rs | 22 +++++++++++++++++++++ cli/integration_tests.rs | 15 +++++++++++++++ cli/main.rs | 2 ++ cli/misc_tests.rs | 12 +++++++++--- cli/mock_http_server.rs | 41 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 cli/mock_http_server.rs diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index c062009a41c667..7163b204687964 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -724,6 +724,7 @@ mod tests { #[test] fn test_get_source_code_1() { + let http_server_guard = crate::mock_http_server::run(); let (temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -800,10 +801,12 @@ mod tests { assert_eq!(&(r4.media_type), &msg::MediaType::TypeScript); assert!(fs::read_to_string(&headers_file_name).is_err()); }); + drop(http_server_guard); } #[test] fn test_get_source_code_2() { + let http_server_guard = crate::mock_http_server::run(); let (temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -866,10 +869,12 @@ mod tests { "text/javascript" ); }); + drop(http_server_guard); } #[test] fn test_get_source_code_multiple_downloads_of_same_file() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -910,10 +915,12 @@ mod tests { assert_eq!(headers_file_modified, headers_file_modified_2); }); + drop(http_server_guard); } #[test] fn test_get_source_code_3() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test basic follow and headers recording tokio_util::init(|| { @@ -960,10 +967,12 @@ mod tests { // Examine the meta result. assert_eq!(mod_meta.url.clone(), target_module_url); }); + drop(http_server_guard); } #[test] fn test_get_source_code_4() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test double redirects and headers recording tokio_util::init(|| { @@ -1022,10 +1031,12 @@ mod tests { // Examine the meta result. assert_eq!(mod_meta.url.clone(), target_url); }); + drop(http_server_guard); } #[test] fn test_get_source_code_5() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test that redirect target is not downloaded twice for different redirect source. tokio_util::init(|| { @@ -1067,10 +1078,12 @@ mod tests { assert_eq!(file_modified, file_modified_2); }); + drop(http_server_guard); } #[test] fn test_get_source_code_6() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test that redirections can be limited tokio_util::init(|| { @@ -1087,10 +1100,12 @@ mod tests { let err = result.err().unwrap(); assert_eq!(err.kind(), ErrorKind::TooManyRedirects); }); + drop(http_server_guard); } #[test] fn test_get_source_code_no_fetch() { + let http_server_guard = crate::mock_http_server::run(); let (_temp_dir, fetcher) = test_setup(); tokio_util::init(|| { let module_url = @@ -1110,10 +1125,12 @@ mod tests { let result = fetcher.get_source_file(&module_url, true, true); assert!(result.is_ok()); }); + drop(http_server_guard); } #[test] fn test_fetch_source_async_1() { + let http_server_guard = crate::mock_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); @@ -1152,10 +1169,12 @@ mod tests { // Not MediaType::TypeScript due to .headers.json modification assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); }); + drop(http_server_guard); } #[test] fn test_fetch_source_1() { + let http_server_guard = crate::mock_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); @@ -1189,10 +1208,12 @@ mod tests { // Not MediaType::TypeScript due to .headers.json modification assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript); }); + drop(http_server_guard); } #[test] fn test_fetch_source_2() { + let http_server_guard = crate::mock_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); @@ -1249,6 +1270,7 @@ mod tests { "text/typescript" ); }); + drop(http_server_guard); } #[test] diff --git a/cli/integration_tests.rs b/cli/integration_tests.rs index b03f695961b019..7eb8d7e882f1ce 100644 --- a/cli/integration_tests.rs +++ b/cli/integration_tests.rs @@ -50,6 +50,7 @@ itest!(_005_more_imports { itest!(_006_url_imports { args: "run --reload 006_url_imports.ts", output: "tests/006_url_imports.ts.out", + http_server: true, }); itest!(_012_async { @@ -90,6 +91,7 @@ itest!(_018_async_catch { itest!(_019_media_types { args: "run --reload 019_media_types.ts", output: "tests/019_media_types.ts.out", + http_server: true, }); itest!(_020_json_modules { @@ -105,6 +107,7 @@ itest!(_021_mjs_modules { itest!(_022_info_flag_script { args: "info http://127.0.0.1:4545/tests/019_media_types.ts", output: "tests/022_info_flag_script.out", + http_server: true, }); itest!(_023_no_ext_with_headers { @@ -131,6 +134,7 @@ itest!(_025_reload_js_type_error { itest!(_026_redirect_javascript { args: "run --reload 026_redirect_javascript.js", output: "tests/026_redirect_javascript.js.out", + http_server: true, }); itest!(_026_workers { @@ -141,6 +145,7 @@ itest!(_026_workers { itest!(_027_redirect_typescript { args: "run --reload 027_redirect_typescript.ts", output: "tests/027_redirect_typescript.ts.out", + http_server: true, }); itest!(_028_args { @@ -359,6 +364,7 @@ itest!(error_016_dynamic_import_permissions2 { output: "tests/error_016_dynamic_import_permissions2.out", check_stderr: true, exit_code: 1, + http_server: true, }); itest!(error_stack { @@ -473,6 +479,7 @@ struct IntegrationTest { input: Option<&'static str>, exit_code: i32, check_stderr: bool, + http_server: bool, } impl IntegrationTest { @@ -491,6 +498,12 @@ impl IntegrationTest { debug!("root path {}", root.display()); debug!("bin path {}", bin.display()); + let http_server_guard = if self.http_server { + Some(crate::mock_http_server::run()) + } else { + None + }; + let (mut reader, writer) = pipe().unwrap(); let mut command = Command::new(bin); command.args(args); @@ -524,6 +537,8 @@ impl IntegrationTest { let status = process.wait().expect("failed to finish process"); let exit_code = status.code().unwrap(); + drop(http_server_guard); + actual = strip_ansi_codes(&actual).to_string(); if self.exit_code != exit_code { diff --git a/cli/main.rs b/cli/main.rs index 83d195f76be0d1..4aece00e89ea52 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -22,6 +22,8 @@ extern crate url; mod integration_tests; #[cfg(test)] mod misc_tests; +#[cfg(test)] +mod mock_http_server; mod colors; pub mod compilers; diff --git a/cli/misc_tests.rs b/cli/misc_tests.rs index 2e0d24972ffa43..332502692ea4b0 100644 --- a/cli/misc_tests.rs +++ b/cli/misc_tests.rs @@ -20,13 +20,17 @@ fn benchmark_test() { #[test] fn deno_dir_test() { - assert!(run_python_script("tools/deno_dir_test.py")) + let g = crate::mock_http_server::run(); + assert!(run_python_script("tools/deno_dir_test.py")); + drop(g); } // TODO(#2933): Rewrite this test in rust. #[test] fn fetch_test() { - assert!(run_python_script("tools/fetch_test.py")) + let g = crate::mock_http_server::run(); + assert!(run_python_script("tools/fetch_test.py")); + drop(g); } // TODO(#2933): Rewrite this test in rust. @@ -38,7 +42,9 @@ fn fmt_test() { // TODO(#2933): Rewrite this test in rust. #[test] fn js_unit_tests() { - assert!(run_python_script("tools/unit_tests.py")) + let g = crate::mock_http_server::run(); + assert!(run_python_script("tools/unit_tests.py")); + drop(g); } // TODO(#2933): Rewrite this test in rust. diff --git a/cli/mock_http_server.rs b/cli/mock_http_server.rs new file mode 100644 index 00000000000000..2c21b6c72e79a1 --- /dev/null +++ b/cli/mock_http_server.rs @@ -0,0 +1,41 @@ +// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +#![cfg(test)] + +use std::process::Child; +use std::process::Command; +use std::sync::Mutex; +use std::sync::MutexGuard; +use std::thread; +use std::time; + +lazy_static! { + static ref GUARD: Mutex<()> = Mutex::new(()); +} + +pub struct MockHttpServerGuard<'a> { + g: MutexGuard<'a, ()>, + child: Child, +} + +impl<'a> Drop for MockHttpServerGuard<'a> { + fn drop(&mut self) { + self.child.kill().expect("failed to kill http_server.py"); + drop(&self.g); + } +} + +// TODO(ry) Allow tests to use the http server in parallel. +pub fn run<'a>() -> MockHttpServerGuard<'a> { + let g = GUARD.lock().unwrap(); + + let child = Command::new("python") + .current_dir("../") + .arg("tools/http_server.py") + .spawn() + .expect("failed to execute child"); + + // Wait 1 second for the server to come up. TODO(ry) this is Racy. + thread::sleep(time::Duration::from_secs(1)); + + MockHttpServerGuard { child, g } +} From 364ac0354c37081a927aeefc9d2e516fa7b6813e Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 13:26:04 -0400 Subject: [PATCH 05/10] bite the bullet and remove tools/test.py --- .appveyor.yml | 2 +- .travis.yml | 2 +- tools/test.py | 32 -------------------------------- 3 files changed, 2 insertions(+), 34 deletions(-) delete mode 100755 tools/test.py diff --git a/.appveyor.yml b/.appveyor.yml index 8522cfaa0b5c60..3eb28006cfa3d3 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -166,7 +166,7 @@ build_script: test_script: - python tools\lint.py - python tools\test_format.py - - python tools\test.py + - cargo test -vv --release --locked after_test: # Stop sccache and show stats. diff --git a/.travis.yml b/.travis.yml index 6e87e217b04194..e364cc3d697369 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,7 +55,7 @@ before_script: # Default script for release builds. script: - cargo build -vv --release --locked --all-targets -- ./tools/test.py --release +- cargo test -vv --release --locked # For some reason it's faster to run clippy after build. - rustup component add clippy - cargo clippy --all-targets --release --locked -- -D clippy::all diff --git a/tools/test.py b/tools/test.py deleted file mode 100755 index f77a82893ec164..00000000000000 --- a/tools/test.py +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env python -# Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -# Runs the full test suite. -# This script is wrapper of `cargo test`. -# -# Usage: ./tools/test.py -# -# See `./tools/test.py -h` for the available options. -import os -import sys - -import http_server -from util import build_path, enable_ansi_colors, rmtree, run - - -def main(): - deno_dir = os.path.join(build_path(), ".deno_test") - if os.path.isdir(deno_dir): - rmtree(deno_dir) - os.environ["DENO_DIR"] = deno_dir - - enable_ansi_colors() - - cmd = ["cargo", "test", "--locked"] + sys.argv[1:] - if "--release" in cmd: - os.environ["DENO_BUILD_MODE"] = "release" - run(cmd) - - -if __name__ == '__main__': - with http_server.spawn(): - main() From fa465ca2fa10eb66a8af8fdf646cd280c3731a0f Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 13:28:52 -0400 Subject: [PATCH 06/10] s/mock_http_server/test_http_server/g --- cli/file_fetcher.rs | 22 +++++++++---------- cli/integration_tests.rs | 2 +- cli/main.rs | 2 +- cli/misc_tests.rs | 6 ++--- ...ock_http_server.rs => test_http_server.rs} | 0 5 files changed, 16 insertions(+), 16 deletions(-) rename cli/{mock_http_server.rs => test_http_server.rs} (100%) diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 7163b204687964..31e55c9cafeede 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -724,7 +724,7 @@ mod tests { #[test] fn test_get_source_code_1() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -806,7 +806,7 @@ mod tests { #[test] fn test_get_source_code_2() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -874,7 +874,7 @@ mod tests { #[test] fn test_get_source_code_multiple_downloads_of_same_file() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { @@ -920,7 +920,7 @@ mod tests { #[test] fn test_get_source_code_3() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test basic follow and headers recording tokio_util::init(|| { @@ -972,7 +972,7 @@ mod tests { #[test] fn test_get_source_code_4() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test double redirects and headers recording tokio_util::init(|| { @@ -1036,7 +1036,7 @@ mod tests { #[test] fn test_get_source_code_5() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test that redirect target is not downloaded twice for different redirect source. tokio_util::init(|| { @@ -1083,7 +1083,7 @@ mod tests { #[test] fn test_get_source_code_6() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); // Test that redirections can be limited tokio_util::init(|| { @@ -1105,7 +1105,7 @@ mod tests { #[test] fn test_get_source_code_no_fetch() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); let (_temp_dir, fetcher) = test_setup(); tokio_util::init(|| { let module_url = @@ -1130,7 +1130,7 @@ mod tests { #[test] fn test_fetch_source_async_1() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); @@ -1174,7 +1174,7 @@ mod tests { #[test] fn test_fetch_source_1() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); @@ -1213,7 +1213,7 @@ mod tests { #[test] fn test_fetch_source_2() { - let http_server_guard = crate::mock_http_server::run(); + let http_server_guard = crate::test_http_server::run(); // http_util::fetch_sync_string requires tokio tokio_util::init(|| { let (_temp_dir, fetcher) = test_setup(); diff --git a/cli/integration_tests.rs b/cli/integration_tests.rs index 7eb8d7e882f1ce..382a34c3d75b14 100644 --- a/cli/integration_tests.rs +++ b/cli/integration_tests.rs @@ -499,7 +499,7 @@ impl IntegrationTest { debug!("bin path {}", bin.display()); let http_server_guard = if self.http_server { - Some(crate::mock_http_server::run()) + Some(crate::test_http_server::run()) } else { None }; diff --git a/cli/main.rs b/cli/main.rs index 4aece00e89ea52..cdb8190da9fef1 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -23,7 +23,7 @@ mod integration_tests; #[cfg(test)] mod misc_tests; #[cfg(test)] -mod mock_http_server; +mod test_http_server; mod colors; pub mod compilers; diff --git a/cli/misc_tests.rs b/cli/misc_tests.rs index 332502692ea4b0..dcb2249883f792 100644 --- a/cli/misc_tests.rs +++ b/cli/misc_tests.rs @@ -20,7 +20,7 @@ fn benchmark_test() { #[test] fn deno_dir_test() { - let g = crate::mock_http_server::run(); + let g = crate::test_http_server::run(); assert!(run_python_script("tools/deno_dir_test.py")); drop(g); } @@ -28,7 +28,7 @@ fn deno_dir_test() { // TODO(#2933): Rewrite this test in rust. #[test] fn fetch_test() { - let g = crate::mock_http_server::run(); + let g = crate::test_http_server::run(); assert!(run_python_script("tools/fetch_test.py")); drop(g); } @@ -42,7 +42,7 @@ fn fmt_test() { // TODO(#2933): Rewrite this test in rust. #[test] fn js_unit_tests() { - let g = crate::mock_http_server::run(); + let g = crate::test_http_server::run(); assert!(run_python_script("tools/unit_tests.py")); drop(g); } diff --git a/cli/mock_http_server.rs b/cli/test_http_server.rs similarity index 100% rename from cli/mock_http_server.rs rename to cli/test_http_server.rs From 85d222b89db4ea22a100414f92919567ed3f5c42 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 13:35:18 -0400 Subject: [PATCH 07/10] cleanup --- cli/misc_tests.rs | 2 ++ cli/test_http_server.rs | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cli/misc_tests.rs b/cli/misc_tests.rs index dcb2249883f792..64269d5120bd22 100644 --- a/cli/misc_tests.rs +++ b/cli/misc_tests.rs @@ -1,4 +1,6 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +#![cfg(test)] + use std::process::Command; fn run_python_script(script: &str) -> bool { diff --git a/cli/test_http_server.rs b/cli/test_http_server.rs index 2c21b6c72e79a1..e995f55f301427 100644 --- a/cli/test_http_server.rs +++ b/cli/test_http_server.rs @@ -12,12 +12,12 @@ lazy_static! { static ref GUARD: Mutex<()> = Mutex::new(()); } -pub struct MockHttpServerGuard<'a> { +pub struct Guard<'a> { g: MutexGuard<'a, ()>, child: Child, } -impl<'a> Drop for MockHttpServerGuard<'a> { +impl<'a> Drop for Guard<'a> { fn drop(&mut self) { self.child.kill().expect("failed to kill http_server.py"); drop(&self.g); @@ -25,7 +25,7 @@ impl<'a> Drop for MockHttpServerGuard<'a> { } // TODO(ry) Allow tests to use the http server in parallel. -pub fn run<'a>() -> MockHttpServerGuard<'a> { +pub fn run<'a>() -> Guard<'a> { let g = GUARD.lock().unwrap(); let child = Command::new("python") @@ -37,5 +37,5 @@ pub fn run<'a>() -> MockHttpServerGuard<'a> { // Wait 1 second for the server to come up. TODO(ry) this is Racy. thread::sleep(time::Duration::from_secs(1)); - MockHttpServerGuard { child, g } + Guard { child, g } } From 278d97b2d355b7de860c565aa92e8323dd298bcd Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 13:36:38 -0400 Subject: [PATCH 08/10] cleanup --- website/manual.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/manual.md b/website/manual.md index 87fe68ff06ceaa..247adeb50f3145 100644 --- a/website/manual.md +++ b/website/manual.md @@ -167,7 +167,7 @@ cargo build -vv ./target/debug/deno tests/002_hello.ts # Test. -./tools/test.py +cargo test # Format code. ./tools/format.py @@ -1144,7 +1144,7 @@ Before submitting, please make sure the following is done: 1. That there is a related issue and it is referenced in the PR text. 2. There are tests that cover the changes. -3. Ensure `./tools/test.py` passes. +3. Ensure `cargo test` passes. 4. Format your code with `tools/format.py` 5. Make sure `./tools/lint.py` passes. From f43b2f0c45765961f0bf2c16a8e80bf948f3fb1b Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 16 Sep 2019 14:06:25 -0400 Subject: [PATCH 09/10] clippy --- cli/test_http_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/test_http_server.rs b/cli/test_http_server.rs index e995f55f301427..9eccbacd49b55f 100644 --- a/cli/test_http_server.rs +++ b/cli/test_http_server.rs @@ -13,6 +13,7 @@ lazy_static! { } pub struct Guard<'a> { + #[allow(dead_code)] g: MutexGuard<'a, ()>, child: Child, } @@ -20,7 +21,6 @@ pub struct Guard<'a> { impl<'a> Drop for Guard<'a> { fn drop(&mut self) { self.child.kill().expect("failed to kill http_server.py"); - drop(&self.g); } } From 107c0083d3371a91442816d458e851762aa74eaf Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Tue, 17 Sep 2019 09:48:56 +0900 Subject: [PATCH 10/10] fix: add http_server guards --- cli/http_util.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/http_util.rs b/cli/http_util.rs index 6411a9ad63c7c8..b976d98b74f4cc 100644 --- a/cli/http_util.rs +++ b/cli/http_util.rs @@ -145,6 +145,7 @@ mod tests { #[test] fn test_fetch_sync_string() { + let g = crate::test_http_server::run(); // 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() { @@ -154,10 +155,12 @@ mod tests { } _ => unreachable!(), }); + drop(g); } #[test] fn test_fetch_string_once_with_redirect() { + let g = crate::test_http_server::run(); // Relies on external http server. See tools/http_server.py let url = Url::parse("http://127.0.0.1:4546/package.json").unwrap(); // Dns resolver substitutes `127.0.0.1` with `localhost` @@ -166,6 +169,7 @@ mod tests { let result = fetch_string_once_sync(&url).unwrap(); assert_eq!(result, FetchOnceResult::Redirect(target_url)); }); + drop(g); } #[test]