Skip to content

Commit

Permalink
Move mp4parse_capi's build_ffi_test to new test_ffi crate.
Browse files Browse the repository at this point in the history
The existing process invoked a Makefile from a Rust test, building test.cc
and a statically linkable mp4parse_capi, then running a series of C API
tests.  This wasn't very portable due the reliable on a Makefile, which also
made assumptions about the layout of cargo's build artifacts.

The new process introduces a new crate (test_ffi) and changes mp4parse_capi
to additionally build as a cdylib.  A build.rs in test_ffi builds
test.cc (which must now be written with a simple library API, rather than as
a binary with a main() due to cc-rs limitations) into a static libtest,
which is then linked to test_ffi's Rust binary in main.rs.

test.cc is now split into run_main for the binary dumping tool and test_main
for the API tests.  `cargo run -p test_ffi path/to/a.mp4` invokes run_main,
and test_ffi's ffi_test invokes test_main.

Fixes mozilla#197
  • Loading branch information
kinetiknz committed Mar 6, 2020
1 parent da2cb93 commit c83db2d
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 98 deletions.
8 changes: 3 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ env:
- RELEASE=--release

script:
- cargo test --all --verbose $RELEASE
# We cannot run `cargo test` with the mp4parse_infallible feature enabled
# We cannot run `cargo test` with the mp4parse_fallible feature enabled
# (see comment where the feature is defined in mp4parse_capi/Cargo.toml),
# but we can at least check for changes behind features that would break the
# build. Also note that we can't run `cargo check` before `cargo test` or
# else it breaks `build_ffi_test` for complicated reasons.
# See https://github.com/mozilla/mp4parse-rust/issues/197
# build.
- cargo check --all --verbose $RELEASE --tests --all-features
- cargo test --all --verbose $RELEASE
- cargo doc --package mp4parse_capi
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
members = [
"mp4parse",
"mp4parse_capi",
"test_ffi",
]

[profile.release]
Expand Down
13 changes: 5 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ API is intended to wrap the rust parser. As such, features should primarily
be implemented in the rust parser and exposed via the C API, rather than the C
API implementing features on its own.

`test_ffi` builds and links `test_ffi/src/test.cc` into a small Rust test
harness in `test_ffi/src/main.rs` that verifies the C API against the
generated `mp4parse.h`.

## Tests

Test coverage comes from several sources:
Expand All @@ -25,11 +29,4 @@ Test coverage comes from several sources:
`mp4parse_capi/tests`. These tests can be run via `cargo test`.
- Examples are included under `mp4parse_capi/examples`. These programs should
continue to build and run after changes are made. Note, these programs are not
typically run by `cargo test`, so manual verification is required. However,
`test.cc` is used to test the foreign function interface via
`build_ffi_test.rs` on non-Windows platforms and will be run by `cargo test`.
- Examples with `afl` related names are for use with the
[american fuzzy lop](http://lcamtuf.coredump.cx/afl/) fuzzer. These examples can
be run without `afl`, but they can be built specifically to receive input from
`afl` via by setting the `fuzz` feature when building. E.g. `cargo build
--features fuzz` would build the examples with fuzzing options.
typically run by `cargo test`, so manual verification is required.
3 changes: 3 additions & 0 deletions mp4parse_capi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ exclude = [
"*.mp4",
]

[lib]
crate-type = ["lib", "cdylib"]

[badges]
travis-ci = { repository = "https://github.com/mozilla/mp4parse-rust" }

Expand Down
3 changes: 1 addition & 2 deletions mp4parse_capi/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ extern crate cbindgen;
use cbindgen::{Config, RenameRule};

fn main() {
let crate_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();

println!("cargo:rerun-if-changed=src/lib.rs");

let config = {
Expand Down Expand Up @@ -33,6 +31,7 @@ extern "C" {
};

// Generate mp4parse.h.
let crate_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();
cbindgen::generate_with_config(&crate_dir, config)
.expect("Could not generate header")
.write_to_file("include/mp4parse.h");
Expand Down
47 changes: 0 additions & 47 deletions mp4parse_capi/examples/Makefile

This file was deleted.

20 changes: 0 additions & 20 deletions mp4parse_capi/tests/build_ffi_test.rs

This file was deleted.

12 changes: 12 additions & 0 deletions test_ffi/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "test_ffi"
version = "0.1.0"
authors = ["Matthew Gregan <[email protected]>"]
edition = "2018"

[dependencies]
mp4parse_capi = { path = "../mp4parse_capi" }

[build-dependencies]
mp4parse_capi = { path = "../mp4parse_capi" }
cc = "1.0"
22 changes: 22 additions & 0 deletions test_ffi/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
extern crate cc;

fn main() {
println!("cargo:rerun-if-changed=src/main.rs");
println!("cargo:rerun-if-changed=src/test.cc");

cc::Build::new()
.file("src/test.cc")
.cpp(true)
.flag_if_supported("-std=c++11")
.include("../mp4parse_capi/include")
.compile("libtest.a");

#[cfg(unix)]
let suffix = "";
#[cfg(windows)]
let suffix = ".dll";
println!("cargo:rustc-link-lib=dylib=mp4parse_capi{}", suffix);

let profile = std::env::var("PROFILE").unwrap();
println!("cargo:rustc-link-search=native=target/{}", profile);
}
48 changes: 48 additions & 0 deletions test_ffi/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use std::os::raw::c_char;
use std::ffi::CString;
use std::convert::TryInto as _;

fn main() {
use std::os::raw::c_int;

#[link(name="test", kind="static")]
extern { fn run_main(npaths: c_int, paths: *const *const c_char) -> c_int; }


let args: Vec<_> = std::env::args().map(|arg| CString::new(arg).unwrap()).collect();
let argv = {
let mut a: Vec<_> = args.iter().map(|arg| arg.as_ptr()).collect();
a.push(std::ptr::null());
a
};

let argc = argv.len() - 1;
unsafe { run_main(argc.try_into().unwrap(), argv.as_ptr()); }
}

#[test]
fn ffi_test() {
use std::os::raw::c_void;

extern { fn test_main(test_path: *const c_char) -> c_void; }

let path = {
let argv0 = std::env::args().next().unwrap();
let mut base = std::path::PathBuf::from(argv0);
base.pop();
base.push("../../../mp4parse/tests/minimal.mp4");
let path = base.canonicalize().unwrap();

#[cfg(unix)]
{
use std::os::unix::ffi::OsStrExt as _;
CString::new(path.as_os_str().as_bytes()).unwrap()
}
#[cfg(windows)]
{
CString::new(path.to_str().unwrap()).unwrap()
}
};

unsafe { test_main(path.as_ptr()); }
}
28 changes: 12 additions & 16 deletions mp4parse_capi/examples/test.cc → test_ffi/src/test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@

#include "mp4parse.h"

intptr_t abort_read(uint8_t *buffer, uintptr_t size, void *userdata)
intptr_t abort_read(uint8_t *, uintptr_t, void *)
{
// This shouldn't be called when allocating a parser.
abort();
}

intptr_t error_read(uint8_t *buffer, uintptr_t size, void *userdata)
intptr_t error_read(uint8_t *, uintptr_t, void *)
{
return -1;
}
Expand Down Expand Up @@ -108,9 +108,9 @@ void test_arg_validation_with_parser()
assert(dummy_value == 42);
}

void test_arg_validation_with_data(const std::string& filename)
void test_arg_validation_with_data(const char* filename)
{
FILE* f = fopen(filename.c_str(), "rb");
FILE* f = fopen(filename, "rb");
assert(f != nullptr);
Mp4parseIo io = { io_read, f };
Mp4parseParser *parser = mp4parse_new(&io);
Expand Down Expand Up @@ -229,24 +229,20 @@ int32_t read_file(const char* filename)
return MP4PARSE_STATUS_OK;
}

int main(int argc, char* argv[])
extern "C"
void test_main(const char* test_path)
{
// Parse command line options.
std::vector<std::string> args(argv + 1, argv + argc);

test_new_parser();
test_arg_validation();
test_arg_validation_with_parser();
test_arg_validation_with_data(test_path);
}

// Find our test file relative to our executable file path.
char* real = realpath(argv[0], NULL);
std::string path(real);
free(real);
auto split = path.rfind('/');
path.replace(split, path.length() - split, "/../../mp4parse/tests/minimal.mp4");
test_arg_validation_with_data(path);
extern "C"
int run_main(int argc, char* argv[])
{
std::vector<std::string> args(argv + 1, argv + argc);

// Run any other test files passed on the command line.
for (auto arg: args) {
read_file(arg.c_str());
}
Expand Down

0 comments on commit c83db2d

Please sign in to comment.