Skip to content
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

Add rustdoc JS non-std tests #58330

Merged
merged 7 commits into from
Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ impl<'a> Builder<'a> {
test::Miri,
test::Clippy,
test::CompiletestTest,
test::RustdocJS,
test::RustdocJSStd,
test::RustdocJSNotStd,
test::RustdocTheme,
// Run bootstrap close to the end as it's unlikely to fail
test::Bootstrap,
Expand Down
62 changes: 53 additions & 9 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,22 +574,22 @@ impl Step for RustdocTheme {
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub struct RustdocJS {
pub struct RustdocJSStd {
pub host: Interned<String>,
pub target: Interned<String>,
}

impl Step for RustdocJS {
impl Step for RustdocJSStd {
type Output = ();
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("src/test/rustdoc-js")
run.path("src/test/rustdoc-js-std")
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(RustdocJS {
run.builder.ensure(RustdocJSStd {
host: run.host,
target: run.target,
});
Expand All @@ -598,12 +598,55 @@ impl Step for RustdocJS {
fn run(self, builder: &Builder<'_>) {
if let Some(ref nodejs) = builder.config.nodejs {
let mut command = Command::new(nodejs);
command.args(&["src/tools/rustdoc-js/tester.js", &*self.host]);
command.args(&["src/tools/rustdoc-js-std/tester.js", &*self.host]);
builder.ensure(crate::doc::Std {
target: self.target,
stage: builder.top_stage,
});
builder.run(&mut command);
} else {
builder.info(
"No nodejs found, skipping \"src/test/rustdoc-js-std\" tests"
);
}
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub struct RustdocJSNotStd {
pub host: Interned<String>,
pub target: Interned<String>,
pub compiler: Compiler,
}

impl Step for RustdocJSNotStd {
type Output = ();
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path("src/test/rustdoc-js")
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.host);
run.builder.ensure(RustdocJSNotStd {
host: run.host,
target: run.target,
compiler,
});
}

fn run(self, builder: &Builder<'_>) {
if builder.config.nodejs.is_some() {
builder.ensure(Compiletest {
compiler: self.compiler,
target: self.target,
mode: "js-doc-test",
suite: "rustdoc-js",
path: None,
compare_mode: None,
});
} else {
builder.info(
"No nodejs found, skipping \"src/test/rustdoc-js\" tests"
Expand Down Expand Up @@ -990,12 +1033,13 @@ impl Step for Compiletest {
.arg(builder.sysroot_libdir(compiler, target));
cmd.arg("--rustc-path").arg(builder.rustc(compiler));

let is_rustdoc_ui = suite.ends_with("rustdoc-ui");
let is_rustdoc = suite.ends_with("rustdoc-ui") || suite.ends_with("rustdoc-js");
Copy link
Member

Choose a reason for hiding this comment

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

It feels like this isn't needed? The compiletest invocation above is has the mode as js-doc-test, which you've also checked below.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more convenient to check for instance:

let mut flags = if is_rustdoc {
    Vec::new()
} else {
    vec!["-Crpath".to_string()]
};

or:

if !is_rustdoc {
    if builder.config.rust_optimize_tests {
        flags.push("-O".to_string());
    }
    if builder.config.rust_debuginfo_tests {
        flags.push("-g".to_string());
    }
}


// Avoid depending on rustdoc when we don't need it.
if mode == "rustdoc"
|| (mode == "run-make" && suite.ends_with("fulldeps"))
|| (mode == "ui" && is_rustdoc_ui)
|| (mode == "ui" && is_rustdoc)
|| mode == "js-doc-test"
{
cmd.arg("--rustdoc-path")
.arg(builder.rustdoc(compiler.host));
Expand Down Expand Up @@ -1029,12 +1073,12 @@ impl Step for Compiletest {
cmd.arg("--nodejs").arg(nodejs);
}

let mut flags = if is_rustdoc_ui {
let mut flags = if is_rustdoc {
Vec::new()
} else {
vec!["-Crpath".to_string()]
};
if !is_rustdoc_ui {
if !is_rustdoc {
if builder.config.rust_optimize_tests {
flags.push("-O".to_string());
}
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
15 changes: 15 additions & 0 deletions src/test/rustdoc-js-std/basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const QUERY = 'String';

const EXPECTED = {
'others': [
{ 'path': 'std::string', 'name': 'String' },
{ 'path': 'std::ffi', 'name': 'CString' },
{ 'path': 'std::ffi', 'name': 'OsString' },
],
'in_args': [
{ 'path': 'std::str', 'name': 'eq' },
],
'returned': [
{ 'path': 'std::string::String', 'name': 'add' },
],
};
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
12 changes: 2 additions & 10 deletions src/test/rustdoc-js/basic.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
const QUERY = 'String';
const QUERY = 'Fo';

const EXPECTED = {
'others': [
{ 'path': 'std::string', 'name': 'String' },
{ 'path': 'std::ffi', 'name': 'CString' },
{ 'path': 'std::ffi', 'name': 'OsString' },
],
'in_args': [
{ 'path': 'std::str', 'name': 'eq' },
],
'returned': [
{ 'path': 'std::string::String', 'name': 'add' },
{ 'path': 'basic', 'name': 'Foo' },
],
};
2 changes: 2 additions & 0 deletions src/test/rustdoc-js/basic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/// Foo
pub struct Foo;
3 changes: 3 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub enum Mode {
Incremental,
RunMake,
Ui,
JsDocTest,
MirOpt,
}

Expand Down Expand Up @@ -59,6 +60,7 @@ impl FromStr for Mode {
"incremental" => Ok(Incremental),
"run-make" => Ok(RunMake),
"ui" => Ok(Ui),
"js-doc-test" => Ok(JsDocTest),
"mir-opt" => Ok(MirOpt),
_ => Err(()),
}
Expand All @@ -82,6 +84,7 @@ impl fmt::Display for Mode {
Incremental => "incremental",
RunMake => "run-make",
Ui => "ui",
JsDocTest => "js-doc-test",
MirOpt => "mir-opt",
};
fmt::Display::fmt(s, f)
Expand Down
30 changes: 27 additions & 3 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::common::{output_base_dir, output_base_name, output_testname_unique};
use crate::common::{Codegen, CodegenUnits, DebugInfoBoth, DebugInfoGdb, DebugInfoLldb, Rustdoc};
use crate::common::{CompileFail, Pretty, RunFail, RunPass, RunPassValgrind};
use crate::common::{Config, TestPaths};
use crate::common::{Incremental, MirOpt, RunMake, Ui};
use crate::common::{Incremental, MirOpt, RunMake, Ui, JsDocTest};
use diff;
use crate::errors::{self, Error, ErrorKind};
use filetime::FileTime;
Expand Down Expand Up @@ -275,6 +275,7 @@ impl<'test> TestCx<'test> {
RunMake => self.run_rmake_test(),
RunPass | Ui => self.run_ui_test(),
MirOpt => self.run_mir_opt_test(),
JsDocTest => self.run_js_doc_test(),
}
}

Expand All @@ -291,6 +292,7 @@ impl<'test> TestCx<'test> {
match self.config.mode {
CompileFail => self.props.compile_pass,
RunPass => true,
JsDocTest => true,
Ui => self.props.compile_pass,
Incremental => {
let revision = self.revision
Expand Down Expand Up @@ -1712,7 +1714,8 @@ impl<'test> TestCx<'test> {
}

fn make_compile_args(&self, input_file: &Path, output_file: TargetLocation) -> Command {
let is_rustdoc = self.config.src_base.ends_with("rustdoc-ui");
let is_rustdoc = self.config.src_base.ends_with("rustdoc-ui") ||
self.config.src_base.ends_with("rustdoc-js");
let mut rustc = if !is_rustdoc {
Command::new(&self.config.rustc_path)
} else {
Expand Down Expand Up @@ -1802,7 +1805,7 @@ impl<'test> TestCx<'test> {
rustc.arg(dir_opt);
}
RunFail | RunPassValgrind | Pretty | DebugInfoBoth | DebugInfoGdb | DebugInfoLldb
| Codegen | Rustdoc | RunMake | CodegenUnits => {
| Codegen | Rustdoc | RunMake | CodegenUnits | JsDocTest => {
// do not use JSON output
}
}
Expand Down Expand Up @@ -2710,6 +2713,27 @@ impl<'test> TestCx<'test> {
fs::remove_dir(path)
}

fn run_js_doc_test(&self) {
if let Some(nodejs) = &self.config.nodejs {
Copy link
Member

Choose a reason for hiding this comment

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

Is self.config loaded from config.toml? I'm wondering how the path to the node executable is set.

Copy link
Member Author

Choose a reason for hiding this comment

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

I took the same code I used previously. But iirc, it is from the config.toml file.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, and checked in bootstrap (this is a config "passed through" from bootstrap).

Copy link
Member

Choose a reason for hiding this comment

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

Can we make this call self.fatal("no nodeJS") or some such? If we're in compiletest we should fail if we don't have a nodeJS here I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok!

let out_dir = self.output_base_dir();

self.document(&out_dir);

let root = self.config.find_rust_src_root().unwrap();
let res = self.cmd2procres(
Command::new(&nodejs)
.arg(root.join("src/tools/rustdoc-js/tester.js"))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be rustdoc-js-std?

Either way, I'm confused why we've only migrated one of the suites and not both to compiletest?

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Mar 6, 2019

Choose a reason for hiding this comment

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

The std one doesn't require to run rustdoc, just to run some JS. So it's not really required on this side.

.arg(out_dir.parent().expect("no parent"))
.arg(&self.testpaths.file.file_stem().expect("couldn't get file stem")),
);
if !res.status.success() {
self.fatal_proc_rec("rustdoc-js test failed!", &res);
}
} else {
self.fatal("no nodeJS");
}
}

fn run_ui_test(&self) {
// if the user specified a format in the ui test
// print the output to the stderr file, otherwise extract
Expand Down
Loading