Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to reference 3rd-party crates #706

Open
alexreg opened this issue Jun 12, 2018 · 15 comments · May be fixed by #2503
Open

Add ability to reference 3rd-party crates #706

alexreg opened this issue Jun 12, 2018 · 15 comments · May be fixed by #2503
Labels
A-Code-Blocks Area: Code blocks A-Tests Area: `mbdook test` related tests

Comments

@alexreg
Copy link

alexreg commented Jun 12, 2018

It would be nice to have the ability to reference 3rd-party (e.g. crates.io) crates, for the sake of things like mdbook test, which currently fail when doing something like extern crate foo;.

@dhardy
Copy link

dhardy commented Oct 22, 2018

There appears to be a -L option to help here, but it leaves a lot to be desired. E.g.:

git clone ../rand rand
cd rand
cargo build
cd ..
mdbook test -L rand/target/debug/deps

fails with:

error[E0464]: multiple matching crates for `rand`
 --> /tmp/mdbook-4MUEU6/overview.md:8:1
  |
2 | extern crate rand;
  | ^^^^^^^^^^^^^^^^^^
  |
  = note: candidates:
          crate `rand`: /home/dhardy/projects/rand/book/rand/target/debug/deps/librand-eab6e1d50c8bd424.rlib
          crate `rand`: /home/dhardy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librand-710e56bbaacb064e.rlib
          crate `rand`: /home/dhardy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librand-3f50c2bb97d1d7f5.rlib

Whoops, even with a fresh build I still have three versions of rand to choose from!


Perhaps the book.toml should contain a dependencies section (or dev-dependencies) like Cargo.toml does?

@alexreg
Copy link
Author

alexreg commented Oct 22, 2018

@dhardy Yeah, I like the suggestion about book.toml (shouldn't we name it MdBook.toml or something for consistency?)

@dhardy
Copy link

dhardy commented Oct 22, 2018

I'm making a suggestion in passing, nothing more.

@alexreg
Copy link
Author

alexreg commented Oct 22, 2018

I know, but it's worth pursuing probably.

@dhardy
Copy link

dhardy commented Oct 23, 2018

Too right, and it makes it impossible to test my book properly: rust-random/book#1

Apparently another copy of rand is already provided by rustup — but it's not the version I want to test against.

@Michael-F-Bryan
Copy link
Contributor

I've been thinking about rewriting mdbook test for a while now but haven't really had the time or known if the demand is there. At the moment we just shell out to rustdoc and effectively run rustdoc --test path/to/chapter.md for each chapter in the book, passing the -L argument straight through. It's clunky because we're directly calling rustdoc when cargo would normally set up all the dependency stuff for you.

If someone wants to champion this feature, mdbook already has pretty decent plugin support and lets you provide your own program for consuming a book and doing something with it. As an example of a backend which processes a book instead of rendering it, mdbook-linkcheck will validate all links in a book. So It's possible to create a backend which will:

  1. Create an empty crate under the output directory
  2. Dump the contents of each chapter into individual files in this crate
  3. Add the necessary dependencies to Cargo.toml
  4. Use rust-skeptic to generate the corresponding test code

Starting off as a plugin would let us iterate quickly outside the mdbook repository, possibly merging it into mdbook as a built-in renderer if that makes sense. I'd be happy to help out with reviewing and mentoring if needed 🙂

@GuillaumeGomez
Copy link
Member

Just a tiny note: currently when I need to test code examples from the book part, I use doc-comment. With the add of cfg(doctest), it only runs on test so it can be a dev-dependency and has the advantage to not force you to rebuild everytime (which is a down side of rust-skeptic).

@MingweiSamuel
Copy link

MingweiSamuel commented Mar 31, 2022

Inspired by doc-comment, this is what I am using, in lib.rs (or any other rust src file):

#[cfg(doctest)]
mod booktest {
    macro_rules! booktest {
        ($i:ident) => {
            #[doc = include_str!(concat!("../../book/", stringify!($i), ".md"))]
            mod $i {}
        }
    }
    booktest!(example_1);
    booktest!(example_2);
}

Note that because of $i:ident the book filenames have to be valid rust identifiers (no dashes or other special characters).

Shows up like this when running cargo test:

running 9 tests
test src/lib.rs - booktest::example_1 (line 105) ... ignored
test src/lib.rs - booktest::example_1 (line 112) ... ignored
test src/lib.rs - booktest::example_1 (line 127) ... ignored
test src/lib.rs - booktest::example_1 (line 69) ... ignored
test src/lib.rs - booktest::example_1 (line 79) ... ignored
test src/lib.rs - booktest::example_1 (line 91) ... ignored
test src/lib.rs - booktest::example_1 (line 97) ... ignored
test src/lib.rs - booktest::example_1 (line 30) ... ok
test src/lib.rs - booktest::example_2 (line 34) ... ok

@dhardy
Copy link

dhardy commented May 27, 2022

A derivative of the above, automating generation of test targets: https://github.com/rust-random/book/tree/master/tests

# generate.sh
mkdir -p src
cat << EOF > src/lib.rs
#![allow(non_snake_case)]
#[macro_use]
extern crate doc_comment;
EOF

for doc in ../src/*.md
do
    NAME=$(basename $doc .md)
    NAME=${NAME//./_}
    NAME=${NAME//-/_}
    echo -e "doctest\041(\"../$doc\");" > src/$NAME.rs
    echo "mod $NAME;" >> src/lib.rs
done

Test steps:

      - name: Generate harness
        working-directory: ./tests
        run: ./generate.sh
      - name: Test code samples
        working-directory: ./tests
        run: cargo test

@tfpk
Copy link

tfpk commented Jul 1, 2022

I'm just wondering if anyone has taken this on and made a mdBook extension as was suggested here? I'm working on a book for nom, and becoming frustrated with the way testing works currently.

I have three main things I think would significantly improve the experience of editing a book:

  1. A [dependencies] section in the book.toml; where you can configure extra packages without using the -L flag. That way, test wouldn't need the -L flag.
  2. Showing any compiler errors inline when building the book; so you can see the result of errors where they occur.
  3. As an extension, since we already would be compiling all the programs in the book, it'd be relatively easy to include the output of programs in the generated book. This gets into a rabbit-hole quickly (you'd need to deal with stdin, the fact builds could be non-deterministic, etc.) but it's something that'd be made much easier by steps 1 and 2.

If nobody is working on this, when some time frees up for me, I might have a go.

(Some further reading indicates rust-skeptic might already provide some of the above -- might integrating it into mdBook (or an extension, to start with) be a reasonable step?)

@tfpk
Copy link

tfpk commented Jul 8, 2022

After looking into this, I think what makes sense is to write a preprocessor, which:

  • Uses rust-skeptic to extract the test cases.
  • Runs the test cases for code that's changed since the last build.
  • Outputs to stderr where a test fails.
  • includes the output of a test below where it is called (possibly by use of a tag on the markdown, like include-result or something) .

Two relatively big design decisions seem important to mention though:

  1. Test functions would not be grouped by chapter; they'd be in individual files. Without this, it'd be difficult to do partial runs of the test cases.
  2. We would have to run each test individually (rather than letting cargo test run them all for us); so we can get the output for each of them. If we ran them all together; we'd need to parse out which test had which result; which is neither nice nor stable.

These would both be relatively easy to achieve using the functions exposed by rust-skeptic, so it doesn't seem like too much of an issue.

My remaining questions then:

  • @Michael-F-Bryan -- does your repo solve any of these issues? Is it something that you're still maintaining or using? Do you have any other words of wisdom?
  • Does anyone see a better way around these design decisions, or have any other comments or concerns before I set off to implement this?

@tfpk
Copy link

tfpk commented Oct 9, 2022

Good news!

v0.1.0 of my proposed crate has now been published, as mdbook-keeper. At the moment, it is an MVP. It:

  • Runs all tests, every time the project is built.
  • Tells you on stderr when a test fails
  • Allows you to configure a Cargo.toml and Cargo.lock as the list of packages to include for any new documentation.

As of yet, it does not:

  • Cache previous test results.
  • Display the results of tests inline.
  • Include test output.

I'd really appreciate feedback on this. Hopefully if enough projects use it, it's something we can bring into mdBook proper.

@mgeisler
Copy link
Contributor

Hey @tfpk, thanks for creating mdbook-keeper! I'm looking into using it for a Rust course I've written: google/comprehensive-rust#175.

@tfpk
Copy link

tfpk commented Jan 17, 2023

That's great to know! Hope it works out for you, and feel free to let me know if there are any issues :)

nikomatsakis added a commit to nikomatsakis/duchess-ndm that referenced this issue Jun 26, 2023
Currently all tests are ignored.
mdbook can't easily link to crates,
see rust-lang/mdBook#706 for details.
Narsil added a commit to huggingface/candle that referenced this issue Aug 1, 2023
- Loading with memmap
- Loading a sharded tensor
- Moved some snippets to `candle-examples/src/lib.rs` This is because
managing book specific dependencies is a pain rust-lang/mdBook#706
- This causes a non aligned inclusion  rust-lang/mdBook#1856 which we have
to ignore fmt to remove.

mdbook might need some more love :)
Narsil added a commit to huggingface/candle that referenced this issue Aug 1, 2023
- Loading with memmap
- Loading a sharded tensor
- Moved some snippets to `candle-examples/src/lib.rs` This is because
managing book specific dependencies is a pain rust-lang/mdBook#706
- This causes a non aligned inclusion  rust-lang/mdBook#1856 which we have
to ignore fmt to remove.

mdbook might need some more love :)
Narsil added a commit to huggingface/candle that referenced this issue Aug 2, 2023
- Loading with memmap
- Loading a sharded tensor
- Moved some snippets to `candle-examples/src/lib.rs` This is because
managing book specific dependencies is a pain rust-lang/mdBook#706
- This causes a non aligned inclusion  rust-lang/mdBook#1856 which we have
to ignore fmt to remove.

mdbook might need some more love :)
Narsil added a commit to huggingface/candle that referenced this issue Aug 2, 2023
- Loading with memmap
- Loading a sharded tensor
- Moved some snippets to `candle-examples/src/lib.rs` This is because
managing book specific dependencies is a pain rust-lang/mdBook#706
- This causes a non aligned inclusion  rust-lang/mdBook#1856 which we have
to ignore fmt to remove.

mdbook might need some more love :)
Narsil added a commit to huggingface/candle that referenced this issue Aug 2, 2023
- Loading with memmap
- Loading a sharded tensor
- Moved some snippets to `candle-examples/src/lib.rs` This is because
managing book specific dependencies is a pain rust-lang/mdBook#706
- This causes a non aligned inclusion  rust-lang/mdBook#1856 which we have
to ignore fmt to remove.

mdbook might need some more love :)
tcharding added a commit to rust-bitcoin/www.rust-bitcoin.org that referenced this issue Oct 6, 2023
cee4953 feat: cookbook (Einherjar)

Pull request description:

  - [x] Convert `build.sh` to a GitHub Actions and closes #6.
  - [x] Automated CI in GitHub Actions that test all the code in the cookbook: `.github/workflows/test.yml`
  - [x] Automated scheduled CI in GitHub Actions that check all markdown files for broken links (we are adding a LOT of links to the `rust-bitcoin` docs): `.github/workflows/check-links.yml`
  - [x] Dependabot to update (weekly) `.github/dependabot.yml`:
    - [x] GitHub Actions
    - [x] Test dependencies, this is good once we release `rust-bitcoin` new versions dependabot will open a PR here to update
  - [x] `justfile` to easy run the build and test with `just`
  - [x] ~~This is how you receive data over P2P~~ moved to #10.
  - [x] ~~This is how you parse blocks and transactions~~ moved to #11.
  - [x] This is how you construct and sign transactions
    - [x] [TABConf 2023 segwit signing exercise](https://github.com/tcharding/workshop/tree/master/sign-segwit-v0)
    - [x] [TABConf 2023 taproot signing exercise](https://github.com/tcharding/workshop/tree/master/sign-taproot)

  ## Details

  I want this cookbook to be tested automatically on CI, so the code will always run.
  The only solution that worked for me was [this workaround](rust-lang/mdBook#706 (comment)) that is used in [The Rust Rand Book](https://github.com/rust-random/book).

ACKs for top commit:
  sanket1729:
    utACK [cee4953](cee4953). Go ahead
  tcharding:
    ACK cee4953

Tree-SHA512: 94a2349380b8dab0c913ab5665d64ae714a8afa56bef82fa47d6d03184db49472ba2b3580b4ac781be3e2356c684041ac33a15ee26aa24c284688d8e71548b22
@avhz
Copy link

avhz commented Jul 31, 2024

Any updates on this ?

@bobhy bobhy linked a pull request Dec 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Code-Blocks Area: Code blocks A-Tests Area: `mbdook test` related tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants