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

Status: RESOLVED FIXED

(Testing :: General, defect, P3)

Description

[(not currently active) Ted Mielczarek]

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.)

Comment 1

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 2

[(not currently active) Ted Mielczarek]

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.

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Simon Sapin (:SimonSapin)]

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 )

Comment 5

[Nathan Froyd [:froydnj]]

(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?

Comment 6

[Simon Sapin (:SimonSapin)]

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*.

Comment 7

[(not currently active) Ted Mielczarek]

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.

Comment 8

[(not currently active) Ted Mielczarek]

I think bug 1373878 fixed this for all intents and purposes. We're not currently running rusttests on Android, but bug 1466580 covers that.