-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: prototype faster test runner #3344
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
9155460
make test runner faster
bartlomieju f21a56c
cleanup
bartlomieju 69346f7
run std tests only in release build
bartlomieju 1792f7f
fmt
bartlomieju 4799656
fix root path
bartlomieju 71269aa
fix match
bartlomieju 7b19177
try
bartlomieju a150937
cfg, add comment explaining temp test file
bartlomieju 10b6a03
lint comment
bartlomieju b7a3181
fix
ry 85d9bed
try debug
bartlomieju 6771a12
Update std_tests.rs
bartlomieju c90cbe2
Update runner.ts
bartlomieju 3c0c1bd
Update runner.ts
bartlomieju 180f7d0
fmt
bartlomieju 82f44f2
try fix
bartlomieju 6e149a9
more debug
bartlomieju 714280a
more debug
bartlomieju 4390dbd
fix?
bartlomieju 76e988c
cleanup
bartlomieju 12ef6ab
cleanup
ry 2aa3905
test breaking a test
ry f233a0c
Revert "test breaking a test"
ry File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,32 @@ | ||
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. | ||
// TODO(ry) Current std tests are run in .github/workflows/build.yml but ideally | ||
// they would be called as part of "cargo test". "deno test" is too slow to do | ||
// this desierable thing: https://github.com/denoland/deno/issues/3088 | ||
/* | ||
#[macro_use] | ||
extern crate lazy_static; | ||
extern crate tempfile; | ||
mod util; | ||
use util::*; | ||
|
||
#[test] | ||
fn std_tests() { | ||
let mut deno = deno_cmd() | ||
.current_dir(root_path()) | ||
.arg("test") | ||
.arg("-A") | ||
.arg("std") | ||
.spawn() | ||
.expect("failed to spawn script"); | ||
let status = deno.wait().expect("failed to wait for the child process"); | ||
assert_eq!(Some(0), status.code()); | ||
assert!(status.success()); | ||
// TODO: fix tests in debug mode | ||
// Runs only on release build | ||
bartlomieju marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[cfg(not(debug_assertions))] | ||
mod tests { | ||
extern crate lazy_static; | ||
extern crate tempfile; | ||
use deno_cli::test_util::*; | ||
use std::process::Command; | ||
use tempfile::TempDir; | ||
|
||
#[test] | ||
fn std_tests() { | ||
let dir = TempDir::new().expect("tempdir fail"); | ||
let mut deno_cmd = Command::new(deno_exe_path()); | ||
deno_cmd.env("DENO_DIR", dir.path()); | ||
|
||
let mut cwd = root_path(); | ||
cwd.push("std"); | ||
let mut deno = deno_cmd | ||
.current_dir(cwd) // note: std tests expect to run from "std" dir | ||
.arg("-A") | ||
// .arg("-Ldebug") | ||
.arg("./testing/runner.ts") | ||
.arg("--exclude=testing/testdata") | ||
.spawn() | ||
.expect("failed to spawn script"); | ||
let status = deno.wait().expect("failed to wait for the child process"); | ||
assert!(status.success()); | ||
} | ||
} | ||
*/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,6 +116,28 @@ export interface RunTestModulesOptions extends RunTestsOptions { | |
allowNone?: boolean; | ||
} | ||
|
||
/** | ||
* Renders test file that will be run. | ||
* | ||
* It's done to optimize compilation of test files, because | ||
* dynamically importing them one by one takes very long time. | ||
* @TODO(bartlomieju): try to optimize compilation by reusing same compiler host | ||
* multiple times | ||
* @param testModules | ||
*/ | ||
function renderTestFile(testModules: string[]): string { | ||
let testFile = ""; | ||
|
||
for (const testModule of testModules) { | ||
// NOTE: this is intentional that template string is not used | ||
// because of TS compiler quirkness of trying to import it | ||
// rather than treating it like a variable | ||
testFile += 'import "' + testModule + '"\n'; | ||
} | ||
|
||
return testFile; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a hack but let's just go with it for now. It's much better to have the std tests running in "cargo test" |
||
|
||
/** | ||
* Import the specified test modules and run their tests as a suite. | ||
* | ||
|
@@ -153,8 +175,9 @@ export async function runTestModules({ | |
disableLog = false | ||
}: RunTestModulesOptions = {}): Promise<void> { | ||
let moduleCount = 0; | ||
const testModules = []; | ||
for await (const testModule of findTestModules(include, exclude)) { | ||
await import(testModule); | ||
testModules.push(testModule); | ||
moduleCount++; | ||
} | ||
|
||
|
@@ -168,6 +191,44 @@ export async function runTestModules({ | |
return; | ||
} | ||
|
||
// Create temporary test file which contains | ||
// all matched modules as import statements. | ||
const testFile = renderTestFile(testModules); | ||
// Select where temporary test file will be stored. | ||
// If `DENO_DIR` is set it means that user intentionally wants to store | ||
// modules there - so it's a sane default to store there. | ||
// Fallback is current directory which again seems like a sane default, | ||
// user is probably working on project in this directory or even | ||
// cd'ed into current directory to quickly run test from this directory. | ||
const root = Deno.env("DENO_DIR") || Deno.cwd(); | ||
const testFilePath = join(root, ".deno.test.ts"); | ||
bartlomieju marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const data = new TextEncoder().encode(testFile); | ||
await Deno.writeFile(testFilePath, data); | ||
|
||
// Import temporary test file and delete it immediately after importing so it's not cluttering disk. | ||
// | ||
// You may think that this will cause recompilation on each run, but this actually | ||
// tricks Deno to not recompile files if there's no need. | ||
// Eg. | ||
// 1. On first run of $DENO_DIR/.deno.test.ts Deno will compile and cache temporary test file and all of its imports | ||
// 2. Temporary test file is removed by test runner | ||
// 3. On next test run file is created again. If no new modules were added then temporary file contents are identical. | ||
// Deno will not compile temporary test file again, but load it directly into V8. | ||
// 4. Deno starts loading imports one by one. | ||
// 5. If imported file is outdated, Deno will recompile this single file. | ||
let err; | ||
try { | ||
await import(`file://${testFilePath}`); | ||
} catch (e) { | ||
err = e; | ||
} finally { | ||
await Deno.remove(testFilePath); | ||
} | ||
|
||
if (err) { | ||
throw err; | ||
} | ||
|
||
if (!disableLog) { | ||
console.log(`Found ${moduleCount} matching test modules.`); | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/ thank you