Skip to content

Commit 8d6d566

Browse files
authored
Rollup merge of #113640 - jyn514:nodejs-defaults, r=GuillaumeGomez
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````
2 parents 8d1dd7e + 9d071b3 commit 8d6d566

File tree

1 file changed

+39
-43
lines changed

1 file changed

+39
-43
lines changed

src/bootstrap/test.rs

+39-43
Original file line numberDiff line numberDiff line change
@@ -867,46 +867,43 @@ impl Step for RustdocJSStd {
867867
const ONLY_HOSTS: bool = true;
868868

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

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

877878
fn run(self, builder: &Builder<'_>) {
878-
if let Some(ref nodejs) = builder.config.nodejs {
879-
let mut command = Command::new(nodejs);
880-
command
881-
.arg(builder.src.join("src/tools/rustdoc-js/tester.js"))
882-
.arg("--crate-name")
883-
.arg("std")
884-
.arg("--resource-suffix")
885-
.arg(&builder.version)
886-
.arg("--doc-folder")
887-
.arg(builder.doc_out(self.target))
888-
.arg("--test-folder")
889-
.arg(builder.src.join("tests/rustdoc-js-std"));
890-
for path in &builder.paths {
891-
if let Some(p) =
892-
util::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder)
893-
{
894-
if !p.ends_with(".js") {
895-
eprintln!("A non-js file was given: `{}`", path.display());
896-
panic!("Cannot run rustdoc-js-std tests");
897-
}
898-
command.arg("--test-file").arg(path);
879+
let nodejs =
880+
builder.config.nodejs.as_ref().expect("need nodejs to run rustdoc-js-std tests");
881+
let mut command = Command::new(nodejs);
882+
command
883+
.arg(builder.src.join("src/tools/rustdoc-js/tester.js"))
884+
.arg("--crate-name")
885+
.arg("std")
886+
.arg("--resource-suffix")
887+
.arg(&builder.version)
888+
.arg("--doc-folder")
889+
.arg(builder.doc_out(self.target))
890+
.arg("--test-folder")
891+
.arg(builder.src.join("tests/rustdoc-js-std"));
892+
for path in &builder.paths {
893+
if let Some(p) = util::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder) {
894+
if !p.ends_with(".js") {
895+
eprintln!("A non-js file was given: `{}`", path.display());
896+
panic!("Cannot run rustdoc-js-std tests");
899897
}
898+
command.arg("--test-file").arg(path);
900899
}
901-
builder.ensure(crate::doc::Std::new(
902-
builder.top_stage,
903-
self.target,
904-
DocumentationFormat::HTML,
905-
));
906-
builder.run(&mut command);
907-
} else {
908-
builder.info("No nodejs found, skipping \"tests/rustdoc-js-std\" tests");
909900
}
901+
builder.ensure(crate::doc::Std::new(
902+
builder.top_stage,
903+
self.target,
904+
DocumentationFormat::HTML,
905+
));
906+
builder.run(&mut command);
910907
}
911908
}
912909

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

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

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

933931
fn run(self, builder: &Builder<'_>) {
934-
if builder.config.nodejs.is_some() {
935-
builder.ensure(Compiletest {
936-
compiler: self.compiler,
937-
target: self.target,
938-
mode: "js-doc-test",
939-
suite: "rustdoc-js",
940-
path: "tests/rustdoc-js",
941-
compare_mode: None,
942-
});
943-
} else {
944-
builder.info("No nodejs found, skipping \"tests/rustdoc-js\" tests");
945-
}
932+
builder.ensure(Compiletest {
933+
compiler: self.compiler,
934+
target: self.target,
935+
mode: "js-doc-test",
936+
suite: "rustdoc-js",
937+
path: "tests/rustdoc-js",
938+
compare_mode: None,
939+
});
946940
}
947941
}
948942

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

15691563
if let Some(ref nodejs) = builder.config.nodejs {
15701564
cmd.arg("--nodejs").arg(nodejs);
1565+
} else if mode == "js-doc-test" {
1566+
panic!("need nodejs to run js-doc-test suite");
15711567
}
15721568
if let Some(ref npm) = builder.config.npm {
15731569
cmd.arg("--npm").arg(npm);

0 commit comments

Comments
 (0)