Figure out how to run `cargo test` for our in-tree crates

NEW
Unassigned

Status

Testing
General
a year ago
2 months ago

People

(Reporter: ted, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a year ago
We're currently not running the Rust unit tests for our in-tree crates (like mp4parse). We should figure out how to make this happen. To clarify: this is for crates that are in mozilla-central and referenced as a path dependency, not crates that are vendored into `third_party/rust`.

Strawman proposal:
1) Add a `mach cargo-test` or `mach rust-test` or whatever command that just runs `cargo test` on all the in-tree crates, or on whatever crate path you pass, wire that up so `mach test path/to/crate` works as well.
2) For CI builds, run `cargo test --no-run` for each crate, package the resulting test binaries with the existing C++ unit tests so they get run there. (I think this ought to work without issue.)
Blocks: 1335518
Some of our crates call symbols only present in libxul. We'll need to find a way to link the cargo test binary with libxul for these symbols to be available.
(Reporter)

Comment 2

9 months ago
I don't know if that's actually possible with rust's test compilation model. We might need to rope in someone from the Rust team to ask them. AFAIK tests are generally compiled as a completely separate binary, so we wouldn't be able to link both the regular crate code (in gkrust-shared) with the test versions of the crates.

I guess we could ostensibly compile gtest/rust with --test, but I don't know that that's transitive? The other alternative would be to put all our Rust tests into separate crates, but that makes writing unit tests hard.
(Reporter)

Comment 3

9 months ago
We could maybe split the difference and allow two different types of tests:
1) Rust unit tests that don't link against gecko, we compile the crate with `cargo test` and run it as a standalone binary.
2) Rust tests that are written as a separate crate, linked into libxul-gtest, run as part of gtests.
Would it be difficult to build more binaries (possibly through 'cargo test') that link to libxul? Isn’t it more involved than println!("cargo:rustc-link-lib={}", path_to_libxul) in a Cargo build script? (See http://doc.crates.io/build-script.html#outputs-of-the-build-script )
(In reply to Simon Sapin (:SimonSapin) from comment #4)
> Would it be difficult to build more binaries (possibly through 'cargo test')
> that link to libxul? Isn’t it more involved than
> println!("cargo:rustc-link-lib={}", path_to_libxul) in a Cargo build script?
> (See http://doc.crates.io/build-script.html#outputs-of-the-build-script )

It's not difficult, per se, but given that many symbols in libxul are not externally accessible, it's an open question whether linking to libxul in the -lxul sense is really what you want.  Or maybe you're suggesting that the crate/binary link to a static libxul.a?
I don’t know what difference that would make, only that I got link-time missing symbol errors last time I tried to use 'cargo bench' with Stylo. I’d like to make that work *somehow*.
(Reporter)

Updated

6 months ago
See Also: → bug 1373878
(Reporter)

Comment 7

6 months ago
We've gone down this path with GTests for WebRTC stuff before. We had some tests that were built as standalone executables that ran GTests, and it was a mess to get the linkage right. libxul has *a lot* of code and most of it is not easily separable. It would be feasible to link against libxul as a static library, but that's going to be huge and result in huge final binaries.
Blocks: 1395933
You need to log in before you can comment on or make changes to this bug.