Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
7 changes: 6 additions & 1 deletion guide/src/cli/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mdbook test path/to/book
The `--library-path` (`-L`) option allows you to add directories to the library
search path used by `rustdoc` when it builds and tests the examples. Multiple
directories can be specified with multiple options (`-L foo -L bar`) or with a
comma-delimited list (`-L foo,bar`). The path should point to the Cargo
comma-delimited list (`-L foo,bar`). The path should point to the Cargo
[build cache](https://doc.rust-lang.org/cargo/guide/build-cache.html) `deps` directory that
contains the build output of your project. For example, if your Rust project's book is in a directory
named `my-book`, the following command would include the crate's dependencies when running `test`:
Expand All @@ -61,3 +61,8 @@ The `--dest-dir` (`-d`) option allows you to change the output directory for the
book. Relative paths are interpreted relative to the book's root directory. If
not specified it will default to the value of the `build.build-dir` key in
`book.toml`, or to `./book`.

#### --chapter

The `--chapter` (`-c`) option allows you to test a specific chapter of the
book using the chapter name or the relative path to the chapter.
29 changes: 27 additions & 2 deletions src/book/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ impl MDBook {

/// Run `rustdoc` tests on the book, linking against the provided libraries.
pub fn test(&mut self, library_paths: Vec<&str>) -> Result<()> {
self.test_chapter(library_paths, None)
}

/// Run `rustdoc` tests on a specific chapter of the book, linking against the provided libraries.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little unclear to me on what happens if chapter is None. Can it include a little more explanation?

Choose a reason for hiding this comment

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

Sure thing, I added clarifying comments // test_chapter with chapter:None will run all tests. and on the test_chapter function itself the doc comment: /// If chapterisNone, all tests will be run.

pub fn test_chapter(&mut self, library_paths: Vec<&str>, chapter: Option<&str>) -> Result<()> {
let library_args: Vec<&str> = (0..library_paths.len())
.map(|_| "-L")
.zip(library_paths.into_iter())
Expand All @@ -254,6 +259,13 @@ impl MDBook {

let temp_dir = TempFileBuilder::new().prefix("mdbook-").tempdir()?;

let chapter_name: &str = match chapter {
Some(p) => p,
None => "",
};

let mut chapter_found = false;

// FIXME: Is "test" the proper renderer name to use here?
let preprocess_context =
PreprocessorContext::new(self.root.clone(), self.config.clone(), "test".to_string());
Expand All @@ -270,8 +282,18 @@ impl MDBook {
_ => continue,
};

let path = self.source_dir().join(&chapter_path);
info!("Testing file: {:?}", path);
let cp = match chapter_path.to_str() {
Some(s) => s,
None => "",
};

if chapter_name != "" && ch.name != chapter_name && cp != chapter_name {
info!("Skipping chapter '{}'...", ch.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

This all seemed to have reverted back to using an empty string as a sentinel instead of an Option. Is there any particular reason for that?

Choose a reason for hiding this comment

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

sorry this might be my Rust ignorance, I couldn't figure out how to compare chapter : Option<&str> and ch.name: String. Rust complains: "can't compare std::string::String with std::option::Option<&str>". So, moving them both to &str solved the problem, but I guess I could instead convert ch.name to Option<&str> using:

let cn: Option<&str> = Some(&ch.name);

I pushed this change to see if this is what you prefer...

continue;
};

chapter_found = true;
info!("Testing chapter '{}': {:?}", ch.name, chapter_path);

// write preprocessed file to tempdir
let path = temp_dir.path().join(&chapter_path);
Expand Down Expand Up @@ -311,6 +333,9 @@ impl MDBook {
if failed {
bail!("One or more tests failed");
}
if chapter_name != "" && !chapter_found {
bail!(format!("Chapter not found: {}", chapter_name));
}
Ok(())
}

Expand Down
17 changes: 15 additions & 2 deletions src/cmd/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ pub fn make_subcommand<'help>() -> App<'help> {
Relative paths are interpreted relative to the book's root directory.{n}\
If omitted, mdBook uses build.build-dir from book.toml or defaults to `./book`.",
),
).arg(
Arg::new("chapter")
.short('c')
.long("chapter")
.value_name("chapter")
.help(
"Only test the specified chapter{n}\
Where the name of the chapter is defined in the SUMMARY.md file."
)
)
.arg(arg!([dir]
"Root directory for the book{n}\
Expand All @@ -41,14 +50,18 @@ pub fn execute(args: &ArgMatches) -> Result<()> {
.values_of("library-path")
.map(std::iter::Iterator::collect)
.unwrap_or_default();
let chapter: Option<&str> = args.value_of("chapter");

let book_dir = get_book_dir(args);
let mut book = MDBook::load(&book_dir)?;

if let Some(dest_dir) = args.value_of("dest-dir") {
book.config.build.build_dir = dest_dir.into();
}

book.test(library_paths)?;
match chapter {
Some(_) => book.test_chapter(library_paths, chapter),
None => book.test(library_paths),
}?;

Ok(())
}
4 changes: 2 additions & 2 deletions src/theme/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@
var theme = localStorage.getItem('mdbook-theme');
var sidebar = localStorage.getItem('mdbook-sidebar');

if (theme.startsWith('"') && theme.endsWith('"')) {
if (theme && theme.startsWith('"') && theme.endsWith('"')) {
localStorage.setItem('mdbook-theme', theme.slice(1, theme.length - 1));
}

if (sidebar.startsWith('"') && sidebar.endsWith('"')) {
if (sidebar && sidebar.startsWith('"') && sidebar.endsWith('"')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes seem unrelated to the PR?

Choose a reason for hiding this comment

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

they were null reference bugs I ran into while testing this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer unrelated changes to not be included.

Choose a reason for hiding this comment

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

this is a bit different as I couldn't get the tests to pass without this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which tests aren't passing? What kind of errors are you seeing? I don't believe there are any javascript tests, so it is not clear to me how this could have any impact on anything.

Choose a reason for hiding this comment

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

Hmmm, trying to repro it I can't find it any more and I see there's a try{ } catch (e) { } around this whole thing, so it's possible I caught the exception in the debugger and mistook it for a bug when it was expecting the try/catch block to catch it and continue. So I reverted these changes.

localStorage.setItem('mdbook-sidebar', sidebar.slice(1, sidebar.length - 1));
}
} catch (e) { }
Expand Down
20 changes: 10 additions & 10 deletions tests/cli/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ fn mdbook_cli_can_correctly_test_a_passing_book() {
let mut cmd = mdbook_cmd();
cmd.arg("test").current_dir(temp.path());
cmd.assert().success()
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]README.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]intro.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]first[\\/]index.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]first[\\/]nested.md""##).unwrap())
.stderr(predicates::str::is_match(r##"rustdoc returned an error:\n\n"##).unwrap().not())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "README.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "intro.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "first[\\/]index.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "first[\\/]nested.md""##).unwrap())
.stderr(predicates::str::is_match(r##"returned an error:\n\n"##).unwrap().not())
.stderr(predicates::str::is_match(r##"Nested_Chapter::Rustdoc_include_works_with_anchors_too \(line \d+\) ... FAILED"##).unwrap().not());
}

Expand All @@ -25,10 +25,10 @@ fn mdbook_cli_detects_book_with_failing_tests() {
let mut cmd = mdbook_cmd();
cmd.arg("test").current_dir(temp.path());
cmd.assert().failure()
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]README.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]intro.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]first[\\/]index.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing file: "([^"]+)[\\/]first[\\/]nested.md""##).unwrap())
.stderr(predicates::str::is_match(r##"rustdoc returned an error:\n\n"##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "README.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "intro.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "first[\\/]index.md""##).unwrap())
.stderr(predicates::str::is_match(r##"Testing chapter [^:]*: "first[\\/]nested.md""##).unwrap())
.stderr(predicates::str::is_match(r##"returned an error:\n\n"##).unwrap())
.stderr(predicates::str::is_match(r##"Nested_Chapter::Rustdoc_include_works_with_anchors_too \(line \d+\) ... FAILED"##).unwrap());
}
21 changes: 21 additions & 0 deletions tests/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,24 @@ fn mdbook_detects_book_with_failing_tests() {

assert!(md.test(vec![]).is_err());
}

#[test]
fn mdbook_test_chapter() {
let temp = DummyBook::new().with_passing_test(true).build().unwrap();
let mut md = MDBook::load(temp.path()).unwrap();

let result = md.test_chapter(vec![], Some("Introduction"));
assert!(
result.is_ok(),
"test_chapter failed with {}",
result.err().unwrap()
);
}

#[test]
fn mdbook_test_chapter_not_found() {
let temp = DummyBook::new().with_passing_test(true).build().unwrap();
let mut md = MDBook::load(temp.path()).unwrap();

assert!(md.test_chapter(vec![], Some("Bogus Chapter Name")).is_err());
}