comprehensive-rust icon indicating copy to clipboard operation
comprehensive-rust copied to clipboard

Look at mdbook-keeper

Open mgeisler opened this issue 2 years ago • 7 comments

We currently have a number of code blocks marked with compile_fail: this is often because they depend on third-party crates which aren't available when running mdbook test.

We should look into using https://github.com/tfpk/mdbook-keeper/ to handle testing instead. It explicitly supports using third-party crates in code snippets.

mgeisler avatar Jan 17 '23 13:01 mgeisler

This is of course a bit of an open ended issue :slightly_smiling_face: What I hope for is that someone will look at what mdbook keeper can do for us:

  • can it run our current tests?
  • can we use it to run tests with more dependencies?

The goal would be remove the compile_fail from our code blocks.

If you can get this working locally, then we can work on a updating the GitHub action which runs the tests for us on every PR.

mgeisler avatar May 26 '23 15:05 mgeisler

I started looking into this. Some issues/notes so far:

  • Tests in sub-chapters aren't run (tfpk/mdbook-keeper#2)
  • It doesn't seem to understand {{#include some/file.rs}}
  • Output doesn't appear incrementally
  • The tests block progress on mdbook build and mdbook serve

On my machine, it takes ~3min to run the tests which is pretty rough when I just want to mdbook serve. I think there's hope, potentially using:

I intend to work on this some more. In case my work stalls out, or anyone's curious, my keeper branch has the WIP.

mauvealerts avatar Jun 02 '23 13:06 mauvealerts

Thanks @mauvealerts! That's already a very valuable analysis! For example, I had not realized that you run the tests all the time... so we would definitely only enable it on demand like we do for the mdbook-xgettext renderer.

mgeisler avatar Jun 02 '23 14:06 mgeisler

I have something that I'm fairly happy with, when combined with all of these changes to mdbook-keeper

  • tfpk/mdbook-keeper#2
  • tfpk/mdbook-keeper#3
  • tfpk/mdbook-keeper#4

I have things running comparable to mdbook test. IMO the steps necessary to make a mergable PR are:

  • [x] Decide where the skeleton project should go (currently mdbook-keeper-deps)
  • [x] Don't run tests by default for mdbook serve
  • [x] mdbook-keeper has released a version with the changes
  • [x] Prove out the usefulness by testing some snippets

mauvealerts avatar Jun 04 '23 09:06 mauvealerts

Thanks for pushing this!

  • Decide where the skeleton project should go (currently mdbook-keeper-deps)

I think this should be the root of the repository, or perhaps the src/ folder. We already have a Cargo workspace setup so that cargo check does something useful for the code in our exercises. I hope we can build on this for the book itself: perhaps we can move up the src/exercises/Cargo.toml file to src/ and then use its dependencies across the examples?

mgeisler avatar Jun 09 '23 11:06 mgeisler

I didn't think it was possible to have both a [workspace] and (e.g.) a lib in the same Cargo.toml, but apparently it is. I've updated my branch to just use Cargo.toml in the root directory, at least for now. I've also locked mdbook-keeper to the markdown output format, so it doesn't slow down mdbook serve

I started updating some examples, but ran into link errors (tfpk/mdbook-keeper#9). In particular, not being able to use tokio on Windows makes me hesitant to migrate the async examples (which was most of what I planned to do).

mauvealerts avatar Jun 12 '23 18:06 mauvealerts

I've also locked mdbook-keeper to the markdown output format, so it doesn't slow down mdbook serve

Ah, that's a clever work-around :smile: However, it almost suggests that mdbook-keeper should be a renderer instead of a preprocessor. @tfpk, have you considered that approach?

I started updating some examples, but ran into link errors (tfpk/mdbook-keeper#9). In particular, not being able to use tokio on Windows makes me hesitant to migrate the async examples (which was most of what I planned to do).

Oh, I see... I agree that it would be important to have test coverage for all the small code snippets involving Tokio and that was also my main reason to look at mdbook-keeper :slightly_smiling_face:

mgeisler avatar Jun 13 '23 10:06 mgeisler