Skip to content

Commit

Permalink
Remove test.py, use cargo test as test frontend (#2967)
Browse files Browse the repository at this point in the history
Fixes #2933
  • Loading branch information
ry authored Sep 19, 2019
1 parent 1b1ae65 commit 56ac638
Show file tree
Hide file tree
Showing 24 changed files with 545 additions and 414 deletions.
2 changes: 1 addition & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 --all-targets --locked

after_test:
# Stop sccache and show stats.
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ jobs:
run: sccache --start-server

- name: Build
run: cargo build -vv --release --locked --all-targets
run: cargo build --release --locked --all-targets

- name: Test
run: python ./tools/test.py
run: cargo test --release --locked --all-targets

- name: Stop sccache
run: sccache --stop-server
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
- cargo test -vv --release --all-targets --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
Expand Down
25 changes: 22 additions & 3 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ mod tests {

#[test]
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(|| {
Expand Down Expand Up @@ -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::test_util::http_server();
let (temp_dir, fetcher) = test_setup();
// http_util::fetch_sync_string requires tokio
tokio_util::init(|| {
Expand Down Expand Up @@ -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::test_util::http_server();
let (_temp_dir, fetcher) = test_setup();
// http_util::fetch_sync_string requires tokio
tokio_util::init(|| {
Expand Down Expand Up @@ -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::test_util::http_server();
let (_temp_dir, fetcher) = test_setup();
// Test basic follow and headers recording
tokio_util::init(|| {
Expand Down Expand Up @@ -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::test_util::http_server();
let (_temp_dir, fetcher) = test_setup();
// Test double redirects and headers recording
tokio_util::init(|| {
Expand Down Expand Up @@ -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::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(|| {
Expand Down Expand Up @@ -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::test_util::http_server();
let (_temp_dir, fetcher) = test_setup();
// Test that redirections can be limited
tokio_util::init(|| {
Expand All @@ -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::test_util::http_server();
let (_temp_dir, fetcher) = test_setup();
tokio_util::init(|| {
let module_url =
Expand All @@ -1110,11 +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() {
// http_util::fetch_sync_string requires tokio
let http_server_guard = crate::test_util::http_server();
tokio_util::init(|| {
let (_temp_dir, fetcher) = test_setup();
let module_url =
Expand Down Expand Up @@ -1152,11 +1168,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() {
// http_util::fetch_sync_string requires tokio
let http_server_guard = crate::test_util::http_server();
tokio_util::init(|| {
let (_temp_dir, fetcher) = test_setup();
let module_url =
Expand Down Expand Up @@ -1189,11 +1206,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() {
// http_util::fetch_sync_string requires tokio
let http_server_guard = crate::test_util::http_server();
tokio_util::init(|| {
let (_temp_dir, fetcher) = test_setup();
let module_url =
Expand Down Expand Up @@ -1249,6 +1267,7 @@ mod tests {
"text/typescript"
);
});
drop(http_server_guard);
}

#[test]
Expand Down
4 changes: 4 additions & 0 deletions cli/http_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ mod tests {

#[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() {
Expand All @@ -154,10 +155,12 @@ mod tests {
}
_ => unreachable!(),
});
drop(http_server_guard);
}

#[test]
fn test_fetch_string_once_with_redirect() {
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:4546/package.json").unwrap();
// Dns resolver substitutes `127.0.0.1` with `localhost`
Expand All @@ -166,6 +169,7 @@ mod tests {
let result = fetch_string_once_sync(&url).unwrap();
assert_eq!(result, FetchOnceResult::Redirect(target_url));
});
drop(http_server_guard);
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions cli/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ mod signal;
pub mod source_maps;
mod startup_data;
pub mod state;
pub mod test_util;
mod tokio_read;
mod tokio_util;
mod tokio_write;
Expand Down
77 changes: 77 additions & 0 deletions cli/test_util.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.

// TODO(ry) Make this file test-only. Somehow it's very difficult to export
// methods to tests/integration_tests.rs and tests/tty_tests.rs if this
// is enabled...
// #![cfg(test)]

use std::path::PathBuf;
use std::process::Child;
use std::process::Command;
use std::sync::Mutex;
use std::sync::MutexGuard;

lazy_static! {
static ref GUARD: Mutex<()> = Mutex::new(());
}

pub fn root_path() -> PathBuf {
PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR"), "/.."))
}

pub fn target_dir() -> PathBuf {
let current_exe = std::env::current_exe().unwrap();
let target_dir = current_exe.parent().unwrap().parent().unwrap();
println!("target_dir {}", target_dir.display());
target_dir.into()
}

pub fn deno_exe_path() -> PathBuf {
// Something like /Users/rld/src/deno/target/debug/deps/deno
let mut p = target_dir().join("deno");
if cfg!(windows) {
p.set_extension("exe");
}
p
}

pub struct HttpServerGuard<'a> {
#[allow(dead_code)]
g: MutexGuard<'a, ()>,
child: Child,
}

impl<'a> Drop for HttpServerGuard<'a> {
fn drop(&mut self) {
match self.child.try_wait() {
Ok(None) => {
self.child.kill().expect("failed to kill http_server.py");
}
Ok(Some(status)) => {
panic!("http_server.py exited unexpectedly {}", status)
}
Err(e) => panic!("http_server.py err {}", e),
}
}
}

/// Starts tools/http_server.py when the returned guard is dropped, the server
/// will be killed.
pub fn http_server<'a>() -> HttpServerGuard<'a> {
// TODO(ry) Allow tests to use the http server in parallel.
let g = GUARD.lock().unwrap();

println!("tools/http_server.py starting...");
let child = Command::new("python")
.current_dir(root_path())
.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.
std::thread::sleep(std::time::Duration::from_secs(2));

println!("tools/http_server.py ready");

HttpServerGuard { child, g }
}
2 changes: 1 addition & 1 deletion cli/tests/006_url_imports.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { printHello } from "http://localhost:4545/tests/subdir/mod2.ts";
import { printHello } from "http://localhost:4545/cli/tests/subdir/mod2.ts";
printHello();
console.log("success");
16 changes: 8 additions & 8 deletions cli/tests/019_media_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// based on the URL containing `.t#.` strings, which exercises the different
// mapping of media types end to end.

import { loaded as loadedTs1 } from "http://localhost:4545/tests/subdir/mt_text_typescript.t1.ts";
import { loaded as loadedTs2 } from "http://localhost:4545/tests/subdir/mt_video_vdn.t2.ts";
import { loaded as loadedTs3 } from "http://localhost:4545/tests/subdir/mt_video_mp2t.t3.ts";
import { loaded as loadedTs4 } from "http://localhost:4545/tests/subdir/mt_application_x_typescript.t4.ts";
import { loaded as loadedJs1 } from "http://localhost:4545/tests/subdir/mt_text_javascript.j1.js";
import { loaded as loadedJs2 } from "http://localhost:4545/tests/subdir/mt_application_ecmascript.j2.js";
import { loaded as loadedJs3 } from "http://localhost:4545/tests/subdir/mt_text_ecmascript.j3.js";
import { loaded as loadedJs4 } from "http://localhost:4545/tests/subdir/mt_application_x_javascript.j4.js";
import { loaded as loadedTs1 } from "http://localhost:4545/cli/tests/subdir/mt_text_typescript.t1.ts";
import { loaded as loadedTs2 } from "http://localhost:4545/cli/tests/subdir/mt_video_vdn.t2.ts";
import { loaded as loadedTs3 } from "http://localhost:4545/cli/tests/subdir/mt_video_mp2t.t3.ts";
import { loaded as loadedTs4 } from "http://localhost:4545/cli/tests/subdir/mt_application_x_typescript.t4.ts";
import { loaded as loadedJs1 } from "http://localhost:4545/cli/tests/subdir/mt_text_javascript.j1.js";
import { loaded as loadedJs2 } from "http://localhost:4545/cli/tests/subdir/mt_application_ecmascript.j2.js";
import { loaded as loadedJs3 } from "http://localhost:4545/cli/tests/subdir/mt_text_ecmascript.j3.js";
import { loaded as loadedJs4 } from "http://localhost:4545/cli/tests/subdir/mt_application_x_javascript.j4.js";

console.log(
"success",
Expand Down
18 changes: 9 additions & 9 deletions cli/tests/022_info_flag_script.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ type: TypeScript
compiled: [WILDCARD].js
map: [WILDCARD].js.map
deps:
http://127.0.0.1:4545/tests/019_media_types.ts
├── http://localhost:4545/tests/subdir/mt_text_typescript.t1.ts
├── http://localhost:4545/tests/subdir/mt_video_vdn.t2.ts
├── http://localhost:4545/tests/subdir/mt_video_mp2t.t3.ts
├── http://localhost:4545/tests/subdir/mt_application_x_typescript.t4.ts
├── http://localhost:4545/tests/subdir/mt_text_javascript.j1.js
├── http://localhost:4545/tests/subdir/mt_application_ecmascript.j2.js
├── http://localhost:4545/tests/subdir/mt_text_ecmascript.j3.js
└── http://localhost:4545/tests/subdir/mt_application_x_javascript.j4.js
http://127.0.0.1:4545/cli/tests/019_media_types.ts
├── http://localhost:4545/cli/tests/subdir/mt_text_typescript.t1.ts
├── http://localhost:4545/cli/tests/subdir/mt_video_vdn.t2.ts
├── http://localhost:4545/cli/tests/subdir/mt_video_mp2t.t3.ts
├── http://localhost:4545/cli/tests/subdir/mt_application_x_typescript.t4.ts
├── http://localhost:4545/cli/tests/subdir/mt_text_javascript.j1.js
├── http://localhost:4545/cli/tests/subdir/mt_application_ecmascript.j2.js
├── http://localhost:4545/cli/tests/subdir/mt_text_ecmascript.j3.js
└── http://localhost:4545/cli/tests/subdir/mt_application_x_javascript.j4.js
2 changes: 1 addition & 1 deletion cli/tests/035_no_fetch_flag.out
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Cannot resolve module "http://127.0.0.1:4545/tests/019_media_types.ts"
Cannot resolve module "http://127.0.0.1:4545/cli/tests/019_media_types.ts"
4 changes: 2 additions & 2 deletions cli/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
This path contains integration tests. See integration_tests.rs for the index.

TODO(ry) Currently //tests is a symlink to //cli/tests, to simplify transition.
In the future //tests should be removed when all references to //tests are
updated.
In the future the symlink should be removed when all the many references have
been updated to the new path.
Loading

1 comment on commit 56ac638

@ry
Copy link
Member Author

@ry ry commented on 56ac638 Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks to @kt3k - he's responsible for a large amount of this patch (see #2955)

(I'm sorry for not attributing correctly in the author or co-authored-by lines - I always forget when I squash into master on the Github interface...)

Please sign in to comment.