Add rustdoc JS non-std tests #58330

GuillaumeGomez · 2019-02-09T19:29:06Z

@QuietMisdreavus: You asked it, here it is!

QuietMisdreavus

I'm hesitant to do the doc compilation in the JS script, because compiletest already has code that we can reuse to build the docs.

I also kinda want to rename the existing rustdoc-js tests to rustdoc-js-std, and name this new category as plain rustdoc-js. (Eventually i'd like to ditch all the rustdoc-js tests that depend on std, but that's a different matter.)

QuietMisdreavus · 2019-02-18T19:20:32Z

src/tools/rustdoc-js-not-std/tester.js

+}
+
+function remove_docs(out_dir) {
+    spawnSync('rm', ['-rf', out_dir]);


Will this work on Windows? The "delete files" command isn't the same.

That's a good point!

I'll just remove this function actually. It's pretty much useless in fact...

QuietMisdreavus · 2019-02-18T19:22:36Z

src/tools/rustdoc-js-not-std/tester.js

+            console.error(error_text.join("\n"));
+        } else {
+            // In this case, we remove the docs, no need to keep them around.
+            remove_docs(out_folder);


Removing the built docs every time means we have to rebuild them every time, even if the test crate never changed.

This is one of the reasons i wanted to move the logic into compiletest, because it can do that timestamp checking to build the docs only when the test (or rustdoc) has changed.

rust-highfive · 2019-02-24T00:10:06Z

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:091c19d0:start=1550966806933647647,finish=1550966809206830415,duration=2273182768
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:02:30]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:02:32] error[E0308]: mismatched types
[00:02:32]    --> src/bootstrap/test.rs:653:53
[00:02:32]     |
[00:02:32] 653 |                         if path.extension() == Some("rs") {
[00:02:32]     |                                                     ^^^^ expected struct `std::ffi::OsStr`, found str
[00:02:32]     = note: expected type `&std::ffi::OsStr`
[00:02:32]                found type `&'static str`
[00:02:32] 
[00:02:32] 
[00:02:32] error[E0277]: the trait bound `std::option::Option<&std::ffi::OsStr>: std::convert::AsRef<std::path::Path>` is not satisfied
[00:02:32]    --> src/bootstrap/test.rs:654:43
[00:02:32]     |
[00:02:32] 654 |                             let out = out.join(path.file_stem());
[00:02:32]     |                                           ^^^^ the trait `std::convert::AsRef<std::path::Path>` is not implemented for `std::option::Option<&std::ffi::OsStr>`
[00:02:32] 
[00:02:32] error[E0599]: no method named `to_os_string` found for type `std::option::Option<&std::ffi::OsStr>` in the current scope
[00:02:32]    --> src/bootstrap/test.rs:659:63
[00:02:32]     |
[00:02:32] 659 |                             tests_to_run.push(out.file_stem().to_os_string());
[00:02:32] 
[00:02:34] error: aborting due to 3 previous errors
[00:02:34] 
[00:02:34] Some errors occurred: E0277, E0308, E0599.
---
[00:02:36]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:02:38] error[E0308]: mismatched types
[00:02:38]    --> src/bootstrap/test.rs:653:53
[00:02:38]     |
[00:02:38] 653 |                         if path.extension() == Some("rs") {
[00:02:38]     |                                                     ^^^^ expected struct `std::ffi::OsStr`, found str
[00:02:38]     = note: expected type `&std::ffi::OsStr`
[00:02:38]                found type `&'static str`
[00:02:38] 
[00:02:38] 
[00:02:38] error[E0277]: the trait bound `std::option::Option<&std::ffi::OsStr>: std::convert::AsRef<std::path::Path>` is not satisfied
[00:02:38]    --> src/bootstrap/test.rs:654:43
[00:02:38]     |
[00:02:38] 654 |                             let out = out.join(path.file_stem());
[00:02:38]     |                                           ^^^^ the trait `std::convert::AsRef<std::path::Path>` is not implemented for `std::option::Option<&std::ffi::OsStr>`
[00:02:38] 
[00:02:38] error[E0599]: no method named `to_os_string` found for type `std::option::Option<&std::ffi::OsStr>` in the current scope
[00:02:38]    --> src/bootstrap/test.rs:659:63
[00:02:38]     |
[00:02:38] 659 |                             tests_to_run.push(out.file_stem().to_os_string());
[00:02:38] 
[00:02:39] error: aborting due to 3 previous errors
[00:02:39] 
[00:02:39] Some errors occurred: E0277, E0308, E0599.
---
[00:02:42]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:02:44] error[E0308]: mismatched types
[00:02:44]    --> src/bootstrap/test.rs:653:53
[00:02:44]     |
[00:02:44] 653 |                         if path.extension() == Some("rs") {
[00:02:44]     |                                                     ^^^^ expected struct `std::ffi::OsStr`, found str
[00:02:44]     = note: expected type `&std::ffi::OsStr`
[00:02:44]                found type `&'static str`
[00:02:44] 
[00:02:44] 
[00:02:44] error[E0277]: the trait bound `std::option::Option<&std::ffi::OsStr>: std::convert::AsRef<std::path::Path>` is not satisfied
[00:02:44]    --> src/bootstrap/test.rs:654:43
[00:02:44]     |
[00:02:44] 654 |                             let out = out.join(path.file_stem());
[00:02:44]     |                                           ^^^^ the trait `std::convert::AsRef<std::path::Path>` is not implemented for `std::option::Option<&std::ffi::OsStr>`
[00:02:44] 
[00:02:44] error[E0599]: no method named `to_os_string` found for type `std::option::Option<&std::ffi::OsStr>` in the current scope
[00:02:44]    --> src/bootstrap/test.rs:659:63
[00:02:44]     |
[00:02:44] 659 |                             tests_to_run.push(out.file_stem().to_os_string());
[00:02:44] 
[00:02:46] error: aborting due to 3 previous errors
[00:02:46] 
[00:02:46] Some errors occurred: E0277, E0308, E0599.
---
[00:02:49]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:02:51] error[E0308]: mismatched types
[00:02:51]    --> src/bootstrap/test.rs:653:53
[00:02:51]     |
[00:02:51] 653 |                         if path.extension() == Some("rs") {
[00:02:51]     |                                                     ^^^^ expected struct `std::ffi::OsStr`, found str
[00:02:51]     = note: expected type `&std::ffi::OsStr`
[00:02:51]                found type `&'static str`
[00:02:51] 
[00:02:51] 
[00:02:51] error[E0277]: the trait bound `std::option::Option<&std::ffi::OsStr>: std::convert::AsRef<std::path::Path>` is not satisfied
[00:02:51]    --> src/bootstrap/test.rs:654:43
[00:02:51]     |
[00:02:51] 654 |                             let out = out.join(path.file_stem());
[00:02:51]     |                                           ^^^^ the trait `std::convert::AsRef<std::path::Path>` is not implemented for `std::option::Option<&std::ffi::OsStr>`
[00:02:51] 
[00:02:51] error[E0599]: no method named `to_os_string` found for type `std::option::Option<&std::ffi::OsStr>` in the current scope
[00:02:51]    --> src/bootstrap/test.rs:659:63
[00:02:51]     |
[00:02:51] 659 |                             tests_to_run.push(out.file_stem().to_os_string());
[00:02:51] 
[00:02:53] error: aborting due to 3 previous errors
[00:02:53] 
[00:02:53] Some errors occurred: E0277, E0308, E0599.
---
[00:02:57]    Compiling bootstrap v0.0.0 (/checkout/src/bootstrap)
[00:03:00] error[E0308]: mismatched types
[00:03:00]    --> src/bootstrap/test.rs:653:53
[00:03:00]     |
[00:03:00] 653 |                         if path.extension() == Some("rs") {
[00:03:00]     |                                                     ^^^^ expected struct `std::ffi::OsStr`, found str
[00:03:00]     = note: expected type `&std::ffi::OsStr`
[00:03:00]                found type `&'static str`
[00:03:00] 
[00:03:00] 
[00:03:00] error[E0277]: the trait bound `std::option::Option<&std::ffi::OsStr>: std::convert::AsRef<std::path::Path>` is not satisfied
[00:03:00]    --> src/bootstrap/test.rs:654:43
[00:03:00]     |
[00:03:00] 654 |                             let out = out.join(path.file_stem());
[00:03:00]     |                                           ^^^^ the trait `std::convert::AsRef<std::path::Path>` is not implemented for `std::option::Option<&std::ffi::OsStr>`
[00:03:00] 
[00:03:00] error[E0599]: no method named `to_os_string` found for type `std::option::Option<&std::ffi::OsStr>` in the current scope
[00:03:00]    --> src/bootstrap/test.rs:659:63
[00:03:00]     |
[00:03:00] 659 |                             tests_to_run.push(out.file_stem().to_os_string());
[00:03:00] 
[00:03:01] error: aborting due to 3 previous errors
[00:03:01] 
[00:03:01] Some errors occurred: E0277, E0308, E0599.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

GuillaumeGomez · 2019-02-24T21:48:02Z

Is it good for you now @QuietMisdreavus ? :)

bors · 2019-02-25T14:55:27Z

☔ The latest upstream changes (presumably #58728) made this pull request unmergeable. Please resolve the merge conflicts.

QuietMisdreavus · 2019-02-26T15:29:13Z

You just moved the build logic from the JS into bootstrap itself. That still doesn't account for the fact that it's still going to rebuild every test crate every time, even if neither the test crate nor rustdoc changed between runs. (Unless i'm missing something in how bootstrap and/or rustdoc works?) As far as i know, the code that actually checks whether to rebuild a test crate for its output is in compiletest, not bootstrap. Can you look into moving the test-crate-building into there so we can reuse the conditional-rebuild logic?

GuillaumeGomez · 2019-02-28T17:09:35Z

No more wild rebuild. Now it's all handled by compiletest directly! Nice catch in here @QuietMisdreavus :)

QuietMisdreavus · 2019-03-05T15:35:45Z

src/tools/compiletest/src/runtest.rs

@@ -2710,6 +2712,25 @@ impl<'test> TestCx<'test> {
        fs::remove_dir(path)
    }

+    fn run_js_doc_test(&self) {
+        if let Some(nodejs) = &self.config.nodejs {


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

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

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

QuietMisdreavus · 2019-03-05T16:13:39Z

cc @Mark-Simulacrum for changes to compiletest (please add more people if you're not the right person!)

Mark-Simulacrum

I'm a little confused why we're doing this split (it seems somewhat arbitrary)...

Mark-Simulacrum · 2019-03-06T00:56:33Z

src/bootstrap/test.rs

@@ -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");


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.

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()); } }

Mark-Simulacrum · 2019-03-06T01:03:01Z

src/tools/compiletest/src/runtest.rs

@@ -290,7 +291,7 @@ impl<'test> TestCx<'test> {
    fn should_compile_successfully(&self) -> bool {
        match self.config.mode {
            CompileFail => self.props.compile_pass,
-            RunPass => true,
+            RunPass | JsDocTest => true,


Instead of the | could we combine this into separate arms?

Mark-Simulacrum · 2019-03-06T01:05:59Z

src/tools/compiletest/src/runtest.rs

@@ -2710,6 +2712,25 @@ impl<'test> TestCx<'test> {
        fs::remove_dir(path)
    }

+    fn run_js_doc_test(&self) {
+        if let Some(nodejs) = &self.config.nodejs {


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

Mark-Simulacrum · 2019-03-06T01:06:54Z

src/tools/compiletest/src/runtest.rs

@@ -2710,6 +2712,25 @@ impl<'test> TestCx<'test> {
        fs::remove_dir(path)
    }

+    fn run_js_doc_test(&self) {
+        if let Some(nodejs) = &self.config.nodejs {


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

Mark-Simulacrum · 2019-03-06T01:08:13Z

src/tools/compiletest/src/runtest.rs

+            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"))


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?

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

GuillaumeGomez · 2019-03-06T13:57:06Z

I'm a little confused why we're doing this split (it seems somewhat arbitrary)...

rustdoc-js-std is based on std docs, this one isn't and will allow us to avoid breaking changes due to API updates.

GuillaumeGomez · 2019-03-06T14:01:49Z

Updated.

Mark-Simulacrum · 2019-03-07T12:33:36Z

rustbuild and compiletest changes look good.

GuillaumeGomez · 2019-03-07T19:52:36Z

@bors: r=QuietMisdreavus,Mark-Simulacrum

bors · 2019-03-07T19:52:37Z

📌 Commit d6add90 has been approved by QuietMisdreavus,Mark-Simulacrum

bors · 2019-03-07T19:52:39Z

🌲 The tree is currently closed for pull requests below priority 500, this pull request will be tested once the tree is reopened

@QuietMisdreavus

…r=QuietMisdreavus,Mark-Simulacrum Add rustdoc JS non-std tests @QuietMisdreavus: You asked it, here it is! r? @QuietMisdreavus

Centril · 2019-03-10T21:02:15Z

Failed in #59069 (comment), @bors r-

GuillaumeGomez · 2019-03-10T21:04:40Z

How can I run an appveyor instance to test it?

Centril · 2019-03-10T21:06:41Z

cc @Mark-Simulacrum @pietroalbini ^

pietroalbini · 2019-03-10T21:12:03Z

We have no way to run try builds on AppVeyor.

GuillaumeGomez · 2019-03-10T21:14:26Z

Ah. That makes things complicated... :-/

GuillaumeGomez · 2019-03-10T22:11:10Z

Wrote a fix. Can we give it a try? :)

GuillaumeGomez · 2019-03-11T11:15:39Z

cc @Centril @pietroalbini

QuietMisdreavus · 2019-03-12T14:28:02Z

Seems reasonable to me.

@bors r=QuietMisdreavus,Mark-Simulacrum

bors · 2019-03-12T14:28:14Z

📌 Commit 37ab3dc has been approved by QuietMisdreavus,Mark-Simulacrum

@QuietMisdreavus

…reavus,Mark-Simulacrum Add rustdoc JS non-std tests @QuietMisdreavus: You asked it, here it is! r? @QuietMisdreavus

bors · 2019-03-12T15:34:07Z

⌛ Testing commit 37ab3dc with merge d06a020...

bors · 2019-03-12T18:58:14Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: QuietMisdreavus,Mark-Simulacrum
Pushing d06a020 to master...

@ghost

[beta] Rollup backports Cherry-picked: * #58021: Fix fallout from #57667 * #59599: Updated RELEASES.md for 1.34.0 * #59587: Remove #[doc(hidden)] from Error::type_id * #58994: Hide deprecation warnings inside derive expansions * #58015: Expand docs for `TryFrom` and `TryInto`. * #59770: ci: pin android emulator to 28.0.23 * #59704: ci: Update FreeBSD tarball downloads * #59257: Update CI configuration for building Redox libraries * #59724: Function arguments should never get promoted * #59499: Fix broken download link in the armhf-gnu image * #58330: Add rustdoc JS non-std tests * #58848: Prevent cache issues on version updates r? @ghost

rust-highfive assigned QuietMisdreavus Feb 9, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2019

This was referenced Feb 9, 2019

Update the future/task API #57992

Merged

Fix rustdoc-js unit-test for substring after removal of unstable API #58331

Closed

QuietMisdreavus reviewed Feb 18, 2019

View reviewed changes

QuietMisdreavus added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2019

GuillaumeGomez force-pushed the rustdoc-js-non-std branch from 191831e to 2f407db Compare February 24, 2019 00:05

GuillaumeGomez force-pushed the rustdoc-js-non-std branch from 2f407db to 48a3297 Compare February 24, 2019 16:19

GuillaumeGomez added 4 commits February 25, 2019 17:43

Add rustdoc JS non-std tests

834347d

Rename rustdoc js test suites

aa3ca32

Move documentation build into bootstrap

be23cd9

Update to last updates

240fad0

GuillaumeGomez force-pushed the rustdoc-js-non-std branch from 48a3297 to 240fad0 Compare February 25, 2019 16:47

Move rustdoc-js testing into compiletest

405d950

QuietMisdreavus reviewed Mar 5, 2019

View reviewed changes

Mark-Simulacrum reviewed Mar 6, 2019

View reviewed changes

Improve code

d6add90

Centril mentioned this pull request Mar 10, 2019

Rollup of 16 pull requests #59069

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2019

Make js tests work even with resource-suffix option

37ab3dc

This was referenced Mar 12, 2019

Rollup of 15 pull requests #59081

Closed

Prevent cache issues on version updates #58848

Merged

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 12, 2019

bors added a commit that referenced this pull request Mar 12, 2019

Auto merge of #58330 - GuillaumeGomez:rustdoc-js-non-std, r=QuietMisd…

d06a020

…reavus,Mark-Simulacrum Add rustdoc JS non-std tests @QuietMisdreavus: You asked it, here it is! r? @QuietMisdreavus

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 12, 2019

bors merged commit 37ab3dc into rust-lang:master Mar 12, 2019

bors mentioned this pull request Mar 12, 2019

Introduce assembly tests suite #58791

Merged

GuillaumeGomez deleted the rustdoc-js-non-std branch March 12, 2019 20:28

pietroalbini added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 8, 2019

pietroalbini mentioned this pull request Apr 8, 2019

[beta] Rollup backports #59737

Merged

Add rustdoc JS non-std tests #58330

Add rustdoc JS non-std tests #58330

Conversation

GuillaumeGomez commented Feb 9, 2019

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rust-highfive commented Feb 24, 2019

GuillaumeGomez commented Feb 24, 2019

bors commented Feb 25, 2019

QuietMisdreavus commented Feb 26, 2019

GuillaumeGomez commented Feb 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuietMisdreavus commented Mar 5, 2019

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez Mar 6, 2019 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez commented Mar 6, 2019

GuillaumeGomez commented Mar 6, 2019

Mark-Simulacrum commented Mar 7, 2019

GuillaumeGomez commented Mar 7, 2019

bors commented Mar 7, 2019

bors commented Mar 7, 2019

Centril commented Mar 10, 2019

GuillaumeGomez commented Mar 10, 2019

Centril commented Mar 10, 2019

pietroalbini commented Mar 10, 2019

GuillaumeGomez commented Mar 10, 2019

GuillaumeGomez commented Mar 10, 2019

GuillaumeGomez commented Mar 11, 2019

QuietMisdreavus commented Mar 12, 2019

bors commented Mar 12, 2019

bors commented Mar 12, 2019

bors commented Mar 12, 2019

GuillaumeGomez Mar 6, 2019 •

edited

Loading