Run rustdoc tests

[JonathanWoollett-Light]

Running test_coverage.py does not take into account doc tests in the coverage, cargo test does not run them.

Using sudo ./tools/devtool shell -p and running cargo test also ignores doc tests.

Both

sudo ./tools/devtool test -- integration_tests/build/test_coverage.py

and

sudo ./tools/devtool shell -p
cargo test --all

exclude execution of rustdoc tests.

In addition this excludes rustdoc tests from code coverage (https://doc.rust-lang.org/rustc/instrument-coverage.html#including-doc-tests).

Blocked on: rust-lang/rust#56925

[dianpopa]

Indeed doc tests do not run when cross compiling (which is what happens when running the coverage): rust-lang/rust#64245 (tho it s good to also have an issue on our side).
The doc tests should get triggered when running unit tests with gnu.

[roypat]

Since we switched to grcov, we run both unittests and coverage with the *-gnu toolchain, so doc tests are included in both. cargo test inside of devtool shell does not run them because we have a .cargo/config file that specifies the x86_64-unknown-linux-musl target, meaning you're "cross-compiling" if you don't explicitly specify --target x86_64-unknown-linux-gnu. If you do that, doc tests will run.

I would vote for closing this issue, because there's nothing really to be done on our side, wdyt?

[JonathanWoollett-Light]

Rustdoc tests do not appear to run when under sudo ./tools/devtool test -- integration_tests/build/test_coverage.py. The coverage report from the features branch features/cpu-templates is attached (feature-branch-coverage-report.zip), this illustrates a lack of coverage on functions which have rustdoc tests that should cover them.

For example the function under the bit_mut_display macro is reported as not covered.

/// Macro for defining `impl Display for BitMut`.
macro_rules! bit_mut_display {
    ($x:ty) => {
        impl<const P: u8> std::fmt::Display for BitMut<'_, $x, P> {
            #[doc = concat!("
                ```
                use bit_fields::BitMut;
                let mut x = 5", stringify!($x), ";
                assert_eq!(BitMut::<_,0>(&mut x).to_string(),true.to_string());
                assert_eq!(BitMut::<_,1>(&mut x).to_string(),false.to_string());
                assert_eq!(BitMut::<_,2>(&mut x).to_string(),true.to_string());
                ```
            ")]
            #[inline]
            fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
                write!(f, "{}", bool::from(self))
            }
        }
    };
}

[dianpopa]

I think what we want is just to make sure the doc tests are run (i.e they compile) and not necessarily to include them in coverage (we write unit tests for that).

And as far as my investigation, this cargo test line here does seem to run them.

If you print the stderr of that line you will get:

 Running unittests src/lib.rs (build/cargo_target/aarch64-unknown-linux-gnu/debug/deps/api_server-bddb8ee0c18a3c30)
...
     Running unittests src/lib.rs (build/cargo_target/aarch64-unknown-linux-gnu/debug/deps/net_gen-b4c36af4f2f56542)
     Running unittests src/lib.rs (build/cargo_target/aarch64-unknown-linux-gnu/debug/deps/vmm-e4dd93bc3feaa65b)
   Doc-tests api_server
   Doc-tests arch
   Doc-tests arch_gen
   Doc-tests cpuid
   Doc-tests devices
   Doc-tests dumbo
   Doc-tests io_uring
   Doc-tests logger
   Doc-tests mmds
   Doc-tests net_gen
   Doc-tests rate_limiter
   Doc-tests seccompiler
   Doc-tests snapshot
   Doc-tests utils
   Doc-tests virtio_gen
   Doc-tests vm-memory
   Doc-tests vmm

[JonathanWoollett-Light]

I believe this is the issue relating to doc-tests coverage not working correctly rust-lang/rust#79417.

A specific case is with macros that generate functionality, the best approach to effectively test these is with doc-tests.

I think coverage from doc-tests is important, I would argue doc-tests are preferable to unit tests as they act as both unit tests and documentation.

[roypat]

that issue is about coverage of the actual doc test lines (e.g. the lines inside of a doc comment) - so not what we want. I have definitely seen coverage from doc tests being included in our coverage reports, so this seems to be an issue with our CI. Maybe some profraw files aren't picked up by the grcov invocation?

I would argue doc-tests are preferable to unit tests as they act as both unit tests and documentation.

there is a performance component to consider here: each doc test is its own compilation unit, see rust-lang/rust#75341 which seems to suggest an up to 30x speed degradation of running identical testing work load in doctests vs unit tests.