Skip to content

Commit

Permalink
Rollup merge of rust-lang#113640 - jyn514:nodejs-defaults, r=Guillaum…
Browse files Browse the repository at this point in the history
…eGomez

Make `nodejs` control the default for RustdocJs tests instead of a hard-off switch

If someone says `x test rustdoc-js-std` explicitly on the command line, it's because they want to run the tests. Give an error instead of doing nothing and reporting success.

Before:
```
; x t rustdoc-js
No nodejs found, skipping "tests/rustdoc-js" tests
Build completed successfully in 0:00:00
```

After:
```
; x t rustdoc-js
thread 'main' panicked at 'need nodejs to run js-doc-test suite', test.rs:1566:13
Build completed unsuccessfully in 0:00:00
```

I recommend viewing the diff with whitespace changes disabled.

r? ```@GuillaumeGomez```
  • Loading branch information
matthiaskrgr authored Jul 13, 2023
2 parents 9d3c3e4 + 9d071b3 commit 847cea5
Showing 1 changed file with 39 additions and 43 deletions.
82 changes: 39 additions & 43 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,46 +867,43 @@ impl Step for RustdocJSStd {
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.suite_path("tests/rustdoc-js-std")
let default = run.builder.config.nodejs.is_some();
run.suite_path("tests/rustdoc-js-std").default_condition(default)
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(RustdocJSStd { target: run.target });
}

fn run(self, builder: &Builder<'_>) {
if let Some(ref nodejs) = builder.config.nodejs {
let mut command = Command::new(nodejs);
command
.arg(builder.src.join("src/tools/rustdoc-js/tester.js"))
.arg("--crate-name")
.arg("std")
.arg("--resource-suffix")
.arg(&builder.version)
.arg("--doc-folder")
.arg(builder.doc_out(self.target))
.arg("--test-folder")
.arg(builder.src.join("tests/rustdoc-js-std"));
for path in &builder.paths {
if let Some(p) =
util::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder)
{
if !p.ends_with(".js") {
eprintln!("A non-js file was given: `{}`", path.display());
panic!("Cannot run rustdoc-js-std tests");
}
command.arg("--test-file").arg(path);
let nodejs =
builder.config.nodejs.as_ref().expect("need nodejs to run rustdoc-js-std tests");
let mut command = Command::new(nodejs);
command
.arg(builder.src.join("src/tools/rustdoc-js/tester.js"))
.arg("--crate-name")
.arg("std")
.arg("--resource-suffix")
.arg(&builder.version)
.arg("--doc-folder")
.arg(builder.doc_out(self.target))
.arg("--test-folder")
.arg(builder.src.join("tests/rustdoc-js-std"));
for path in &builder.paths {
if let Some(p) = util::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder) {
if !p.ends_with(".js") {
eprintln!("A non-js file was given: `{}`", path.display());
panic!("Cannot run rustdoc-js-std tests");
}
command.arg("--test-file").arg(path);
}
builder.ensure(crate::doc::Std::new(
builder.top_stage,
self.target,
DocumentationFormat::HTML,
));
builder.run(&mut command);
} else {
builder.info("No nodejs found, skipping \"tests/rustdoc-js-std\" tests");
}
builder.ensure(crate::doc::Std::new(
builder.top_stage,
self.target,
DocumentationFormat::HTML,
));
builder.run(&mut command);
}
}

Expand All @@ -922,7 +919,8 @@ impl Step for RustdocJSNotStd {
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.suite_path("tests/rustdoc-js")
let default = run.builder.config.nodejs.is_some();
run.suite_path("tests/rustdoc-js").default_condition(default)
}

fn make_run(run: RunConfig<'_>) {
Expand All @@ -931,18 +929,14 @@ impl Step for RustdocJSNotStd {
}

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: "tests/rustdoc-js",
compare_mode: None,
});
} else {
builder.info("No nodejs found, skipping \"tests/rustdoc-js\" tests");
}
builder.ensure(Compiletest {
compiler: self.compiler,
target: self.target,
mode: "js-doc-test",
suite: "rustdoc-js",
path: "tests/rustdoc-js",
compare_mode: None,
});
}
}

Expand Down Expand Up @@ -1568,6 +1562,8 @@ note: if you're sure you want to do this, please open an issue as to why. In the

if let Some(ref nodejs) = builder.config.nodejs {
cmd.arg("--nodejs").arg(nodejs);
} else if mode == "js-doc-test" {
panic!("need nodejs to run js-doc-test suite");
}
if let Some(ref npm) = builder.config.npm {
cmd.arg("--npm").arg(npm);
Expand Down

0 comments on commit 847cea5

Please sign in to comment.