Seperate doc tests into separate stages #218

hjr3 · 2017-12-10T06:56:45Z

When generating documentation tests, the following stages will happen:

tests will be gathered from the docs as a list of strings
tests will be saved to target/doc/tests as individual files
tests will be compiled into a single binary called "rustdoc-test"
tests will be run by executing the "rustdoc-test" binary

Doc tests are now actual tests. This allows us to take advantage of the
existing test infrastructure supplied by rustc.

Fixes #32, #54

euclio · 2017-12-14T15:49:03Z

Nice, I really like the direction this PR is taking! A couple of things:

I don't actually see doctests being run as a part of cargo run --release -- --manifest-path=example/Cargo.toml test. Just "running 0 tests".
Could we hook this into the build --emit infrastructure? Like --emit=tests? I could see a world where we don't actually write out the test files unless explicitly requested.

euclio · 2017-12-14T15:50:14Z

src/test.rs

        let program = tokens.to_string();

        program
    } else {
        // If we couldn't parse the crate, then test compilation will fail anyways. Just wrap
-        // everything in a main function.
-        format!("fn main() {{\n{}\n}}", test)
+        // everything in a test function.


Is this backwards-compatible? This is a deviation from what the current rustdoc does.

I am trying to make it backwards compatible, but am running into some challenges. There were two things, single binary and actual tests, that I was trying to accomplish. I may have to land this PR and tackle turning them into tests later.

euclio · 2017-12-14T15:56:06Z

src/lib.rs

@@ -213,82 +211,11 @@ pub fn test(config: &Config) -> Result<()> {
    })?;
    let docs: Documentation = serde_json::from_reader(doc_json)?;

-    let krate = docs.data.as_ref().unwrap();


euclio · 2017-12-14T15:57:39Z

src/test.rs

+            // TODO make this a different function
+            // filter test names into valid identifiers that can be put into `mod #ident`
+            let name = name.replace("::", "_");
+            //


stray comment markers

hjr3 · 2017-12-19T01:47:39Z

Nice, I really like the direction this PR is taking!

Thank you for the review!

I don't actually see doctests being run as a part of cargo run --release -- --manifest-path=example/Cargo.toml test. Just "running 0 tests".

There seems to be a bug where the tests do not get parsed out into a file. I am working on tracking it down.

Could we hook this into the build --emit infrastructure? Like --emit=tests? I could see a world where we don't actually write out the test files unless explicitly requested.

Yes, great idea. I will do this.

steveklabnik · 2017-12-31T23:11:20Z

this is gonna need a rebase

hjr3 · 2018-01-05T14:47:12Z

@steveklabnik yup. i have also made some changes and am in the process of tracking down a bug where the tests do not get built every time. seems to be there prior to this patch too.

beyond that, are we good with changing the doc tests into actual tests? as @euclio mentioned, it is a change to the way existing rustdoc works.

steveklabnik · 2018-01-05T17:55:27Z

I'm happy with not worrying about compatibility right now. We can work on that later.

steveklabnik · 2018-01-05T17:56:26Z

Oh one more thing: it seems that master's tests are failing, dunno whats up with that. If you get CI in the same place, we can and fix those issues some other way.

hjr3 · 2018-01-06T17:24:53Z

I pushed some more commits. Give me another pass to make sure this change is sound and then we can merge and work on the edge cases, etc.

hjr3 · 2018-01-07T16:18:15Z

@steveklabnik This change is working pretty well. There are some outstanding issues listed below. If we land this PR, then I will make an issue for each and start working on them. Otherwise, I can try to address those issues in this PR.

Outstanding issues I will open issues for and fix:

Only the first line of the doc test is captured. This is due to how syn::parse_crate works. This is fixed in a newer version of syn, but the upgrade path will take some work. This is an existing issue in master.
Only one doc test is in the data.json. When testing against the mio crate, there are definitely more than one doc tests, but only one is in data.json. This is an existing issue in master.
A doc test that uses the #[macro_use] attribute will fail to compile. This happens because #[macro_use] cannot be specified inside a test. I have to write some code to strip it out and put it in the main.rs file.
Per the code review, I need to hook this up to the --emit infrastructure.

When generating documentation tests, the following stages will happen: - tests will be gathered from the docs as a list of strings - tests will be saved to target/doc/tests as individual files - tests will be compiled into a single binary called "rustdoc-test" - tests will be run by executing the "rustdoc-test" binary Doc tests are now actual tests. This allows us to take advantage of the existing test infrastructure supplied by rustc. Fixes steveklabnik#32, steveklabnik#54, steveklabnik#219

hjr3 · 2018-01-10T02:56:38Z

Only the first line of the doc test is captured. This is due to how syn::parse_crate works. This is fixed in a newer version of syn, but the upgrade path will take some work. This is an existing issue in master.

Now that https://github.com/dtolnay/syn is on 0.12, this upgrade makes sense.

euclio reviewed Dec 14, 2017

View reviewed changes

hjr3 mentioned this pull request Dec 19, 2017

Not all required crates are added to doctests #219

Open

hjr3 force-pushed the extract-tests branch from a14a19a to ca8ecd4 Compare January 6, 2018 17:24

hjr3 force-pushed the extract-tests branch from ca8ecd4 to 7623b6d Compare January 7, 2018 16:02

hjr3 force-pushed the extract-tests branch from 7623b6d to c86f5b0 Compare January 7, 2018 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Seperate doc tests into separate stages #218

Seperate doc tests into separate stages #218

hjr3 commented Dec 10, 2017

euclio commented Dec 14, 2017

euclio Dec 14, 2017

hjr3 Dec 19, 2017

euclio Dec 14, 2017

euclio Dec 14, 2017

hjr3 commented Dec 19, 2017

steveklabnik commented Dec 31, 2017

hjr3 commented Jan 5, 2018

steveklabnik commented Jan 5, 2018

steveklabnik commented Jan 5, 2018

hjr3 commented Jan 6, 2018

hjr3 commented Jan 7, 2018 •

edited

Loading

hjr3 commented Jan 10, 2018

Seperate doc tests into separate stages #218

Are you sure you want to change the base?

Seperate doc tests into separate stages #218

Conversation

hjr3 commented Dec 10, 2017

euclio commented Dec 14, 2017

euclio Dec 14, 2017

Choose a reason for hiding this comment

hjr3 Dec 19, 2017

Choose a reason for hiding this comment

euclio Dec 14, 2017

Choose a reason for hiding this comment

euclio Dec 14, 2017

Choose a reason for hiding this comment

hjr3 commented Dec 19, 2017

steveklabnik commented Dec 31, 2017

hjr3 commented Jan 5, 2018

steveklabnik commented Jan 5, 2018

steveklabnik commented Jan 5, 2018

hjr3 commented Jan 6, 2018

hjr3 commented Jan 7, 2018 • edited Loading

hjr3 commented Jan 10, 2018

hjr3 commented Jan 7, 2018 •

edited

Loading