[Extension] Extract and test Compliant and Non-compliant code blocks

[x0rw]

Extract code blocks from .rst files and create a global Rust test file (generated.rs) to check them all together using
rustc --test --edition=2021 exts/rust-code-runner/generated.rs under the hood.

Features:

• marker comments to hide specific code from being rendered in html:

      .. code-block:: rust

        // HIDDEN START
        use std::fs;
        use std::io::{self, Read};
        // HIDDEN END 

anything between // HIDDEN START and // HIDDEN END will not be rendered but will still be included for compilation in the generated test file.

• This extension is not tied to the other 'needs' extension so it can be used in other projects that require similar features.
• A set of functions that parse rustc --error-format=json output and extract minimal error information.

Tasks:

• Write unit tests -- this mostly uses regex(scary) for extracting code blocks.
• Comments
• Refactor
• Feature-gate running code, for CI integration.

How to test:

Run: ./make.py -c --offline it will notify you in case there is any issue in any code block,
also check exts/rust-code-runner/generated.rs after doing so.

(Not rebased yet)
Resolves #80

[netlify]

✅ Deploy Preview for scrc-coding-guidelines ready!

Name	Link
🔨 Latest commit	b882c1d
🔍 Latest deploy log	https://app.netlify.com/projects/scrc-coding-guidelines/deploys/68694de1027678000825062c
😎 Deploy Preview	https://deploy-preview-91--scrc-coding-guidelines.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[PLeVasseur]

Hey @x0rw -- thanks for starting on this!

Could I ask for your reason for choosing the 2021 edition instead of the newer 2024 edition?

[x0rw]

Could I ask for your reason for choosing the 2021 edition instead of the newer 2024 edition?

I just went with 2021 out of habit.

[PLeVasseur]

Could I ask for your reason for choosing the 2021 edition instead of the newer 2024 edition?

I just went with 2021 out of habit.

Could we update to the 2024 edition then?

[PLeVasseur]

Hey @x0rw -- wanted to check in to see if this is a work in progress or ready for review

[x0rw]

It’s still in progress. I need to write some tests for it (haven’t found the time yet) because it can easily fail and cause false alerts, I guess.

[PLeVasseur]

Okay, thanks for the update.

[PLeVasseur]

Hey @x0rw -- checking in on this PR. How's it going? Anything needed to help? ☺️

[x0rw]

Hey @PLeVasseur.
It's ready,
Now, it aggregates all the code blocks in a rust project structure and prevents hidden sections from appearing in other builds(html..), then it sanitises these codeb blocks, to remove stuffs like "#[macro_export]" and "#[tokio::main]" that could break the tests.

• the standard generated cargo.toml can be set to include external crates that are common in the tests like tokio, because i think when we reach Concurrency chapters, we will need external crates for some examples.
• running ./make.py collects test code blocks in build/rust-code-blocks/ and running ./make.py -c --offline --test-rust-blocks will test them and report the errors back,
  also try to intentionally write fallible code in a code block.
  and tell me if more flags or configurable options are necessary.

[felix91gr]

It's ready,

Hi @x0rw, sorry to bother you. Should I mark this as ready for review? Since it's currently a Draft PR :3

[x0rw]

Hi @felix91gr 👋🏻
This still needs some improvements and consensus. We need to consider all possible code cases, for example, code that could cause a segmentation fault or other critical issues that might break our tests.
A simple way to address this would be to allow certain code blocks to be marked as non-executable, so they won’t be run during testing.

@PLeVasseur what do you think?

[felix91gr]

code that could cause a segmentation fault or other critical issues that might break our tests

@x0rw you mean like code in a non-compliant example that triggers a panic, that sort of thing?

[x0rw]

Hi @felix91gr

Triggering a panic is fine because we can set the should_panic attribute(TODO) in the test, the real issues are seg faults and syntaxicaly incorrect code, they will basically crash or prevent our program from compiling(resp), a simple solution i could think of is having a flag like //DONT RUN at the top of the code block to signal that it should not run in our tests.

Sorry for the late reply 😄.

[PLeVasseur]

Hey @x0rw -- here's an idea: what if we have folks submit examples using the rustdoc syntax?

They've had to put some thought into attributes to allow for things like this.

For example, pulling from the above doc, we could have bits that are intended to not be compiled / checked be tagged like this:

/// ```ignore

/// fn foo() {

/// ```

And for bits of code that are known to panic like this:

/// ```should_panic

/// assert!(false);

/// ```

(sorry for the weird formatting, I struggled to get this to show up correctly with the "embedded" backticks)

We could consider then on whether to leave these annotations in the rendered version or remove it (using an additional part added to our Sphinx extension)

Thoughts?

tagging @felix91gr as well given he's been interested ☺️

[felix91gr]

here's an idea: what if we have folks submit examples using the rustdoc syntax?

I think this is brilliant. rustdoc has already more or less managed to solve this

@x0rw

the real issues are seg faults and syntactically incorrect code; they will basically crash or prevent our program from compiling(resp), a simple solution i could think of is having a flag like //DONT RUN at the top of the code block to signal that it should not run in our tests.

I'm not 100% sure about how rustdoc handles segfaulting code. If it doesn't handle it, I think we'd probably want to do something like sandbox the code being run. We might be able to read the program's return value if we do, and do something like assert_eq!(return_value, 1).

Regarding non-compiling code... well, if code doesn't compile, then it can't be either a compliant nor a non-compliant example, right? Since the point of both kinds of examples is that they would compile, and therefore the guidelines (and tooling associated with the guidelines) are needed to differentiate both of them.

Sorry for the late reply 😄.

Dw mate, it's all good :)
Always go at your own pace. This is volunteer work after all ^^

[senier]

As a side note: I did something similar with rustdoc lately (working from a markdown file). As this was a bare metal target, I passed a shell script using the --test-runtool (which would execute Qemu). In that setup, rustdoc would treat any non-zero exit status as a panic (i.e. if you had specified should_panic, this would be treated as a success). Not sure how segfaults are handled when executing on the host.

IMO the should_panic behavior of rustdoc is not ideal for us as any panic (even those unrelated to the point you're trying to make with your non-compliant example) will be accepted. It would be better if we could make sure that the expected panic has happened by matching the panic message. I'm thinking of something like the expected parameter of should_panic.

@x0rw Do you think such a matching feature would be easy to add?

[x0rw]

Hi @PLeVasseur

Hey @x0rw -- here's an idea: what if we have folks submit examples using the rustdoc syntax?

They've had to put some thought into attributes to allow for things like this.

This is a great idea, which will save us a lot of time and thinking, the only thing that should be thought of now is how we should handle these rustdoc blocks when Sphinx renders them into HTML, maybe just striping them out of the rustdoc is enough.

---

HI @senier 👋

IMO the should_panic behavior of rustdoc is not ideal for us as any panic (even those unrelated to the point you're trying to make with your non-compliant example) will be accepted. It would be better if we could make sure that the expected panic has happened by matching the panic message. I'm thinking of something like the expected parameter of should_panic.

Great idea, but there is another way with rustdoc, we can use catch_unwind with manual assertions so in the end we can do something like in the example in here: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html, in addition we can wrap a macro around it so in the end the authors can do

/// ```
/// should_panic!({
///     fn non_compl(){
///          -- this will panic with OOOPS --
///     }
/// }, "OOOPS");
/// ```

I still haven't looked at the limitations of this approach.
About the thing you mentioned:

I passed a shell script using the --test-runtool (which would execute Qemu). In that setup, rustdoc would treat any non-zero exit status as a panic (i.e. if you had specified should_panic, this would be treated as a success). Not sure how segfaults are handled when executing on the host.

I encountered a similar challenge when trying to create tests that intentionally segfaults when the program triggers a specific syscall (a Linux security mechanism enforces this). In this case, the output cannot be relied on for snapshot testing, as the OS terminates the process immediately and any buffered stdout/stderr may be partially written or lost. We can then only rely on exit codes and signals but sometimes if it is well planed we can rely on outputs but for non-compliant codes we don't want to add unrelated code just to make it relaible for the test.

Tagging @felix91gr on this last part. also, I agree that there shouldn't be any non-compiling code.

[x0rw]

TL;DR

• Using rustdoc syntax for examples is great.
• should_panic in rustdoc is too loose: it accepts any panic, even unrelated ones.
  • Use should_panic with expected.
  • Or use catch_unwind + assertions.
• Testing functions that segfault:
  • Output is unreliable, only exit-codes and signals are.
• Non-compiling code shouldn't exist in the examples.

[senier]

Hi @x0rw 👋

Great idea, but there is another way with rustdoc, we can use catch_unwind with manual assertions so in the end we can do something like in the example in here: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html, in addition we can wrap a macro around it so in the end the authors can do
[...]
I still haven't looked at the limitations of this approach.

TIL! I like that idea (won't help me with my other project as the code is no_std, though)!

[senier]

TL;DR

* Using rustdoc syntax for examples is great.

* `should_panic` in rustdoc is too loose: it accepts any panic, even unrelated ones.
  
  * Use should_panic with expected.
  * Or use catch_unwind + assertions.

* Testing functions that segfault:
  
  * Output is unreliable, only exit-codes and signals are.

* Non-**compiling** code shouldn't exist in the examples.

Thanks for the summary! Despite my comment, I think the match functionality (in whatever form) could come later.

Don't hesitate to let me know when you want me to review / try your code.