From cb2cb25c66413764a9ef235b0ad916b49810cf4a Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 6 Dec 2024 11:54:28 -0500 Subject: [PATCH] Better bindings tests This comes from my experiences with #2333 and uniffi-bindgen-gecko-js. For those, I needed to run a lot of tests while (re)implementing a bindings generator. I love how the current tests cover basically all UniFFI features, but I think there a few things that can be improved. This commit adds a suite of tests specifically aimed at the bindings. The new tests target specific features rather than trying to mimic real-world examples. Before, I spent a lot of time trying to figure out what a test failure meant. I'm hoping that when one of these tests fail, it's obvious what's not working. It also means that the new test suites don't require writing so much foreign code. They also have a better external bindings story. I'm thinking we can direct external buindings authors to copy everything inside the `bindings-tests` directory and use that to test their bindings. They can use the test macro now, since it's doesn't hard code the mapping from file extension to test runner. Copying the code feels a bit weird, but I like it better than having to publish all these test fixture crates and I think it will work okay in practice. I still want to keep the current examples/fixtures. They're nice because they showcase real-world use-cases and test that those use-cases work like we expect them to. If this gets merged, we can rework those crates to focus on that. Maybe we rework them to only test one bindings language per example/fixture. --- Cargo.lock | 21 +++++ Cargo.toml | 3 + bindings-tests/Cargo.toml | 10 +++ bindings-tests/fixtures/fn-calls/Cargo.toml | 13 +++ bindings-tests/fixtures/fn-calls/src/lib.rs | 9 ++ .../fixtures/primitive-types/Cargo.toml | 13 +++ .../fixtures/primitive-types/src/lib.rs | 46 ++++++++++ .../python/uniffi-fixture-fn-calls.py | 12 +++ .../python/uniffi-fixture-primitive-types.py | 23 +++++ bindings-tests/src/lib.rs | 1 + bindings-tests/tests/tests.rs | 11 +++ uniffi/src/lib.rs | 2 + uniffi_bindgen/src/bindings/python/test.rs | 8 +- uniffi_macros/src/lib.rs | 8 ++ uniffi_macros/src/test.rs | 83 ++++++++++++++++++- uniffi_testing/src/lib.rs | 39 +++++---- 16 files changed, 283 insertions(+), 19 deletions(-) create mode 100644 bindings-tests/Cargo.toml create mode 100644 bindings-tests/fixtures/fn-calls/Cargo.toml create mode 100644 bindings-tests/fixtures/fn-calls/src/lib.rs create mode 100644 bindings-tests/fixtures/primitive-types/Cargo.toml create mode 100644 bindings-tests/fixtures/primitive-types/src/lib.rs create mode 100644 bindings-tests/python/uniffi-fixture-fn-calls.py create mode 100644 bindings-tests/python/uniffi-fixture-primitive-types.py create mode 100644 bindings-tests/src/lib.rs create mode 100644 bindings-tests/tests/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 7264c4877f..edeeb72e5d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,6 +275,13 @@ dependencies = [ "serde", ] +[[package]] +name = "bindings-tests" +version = "0.1.0" +dependencies = [ + "uniffi", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -1842,6 +1849,13 @@ dependencies = [ "uniffi-fixture-ext-types-lib-one", ] +[[package]] +name = "uniffi-fixture-fn-calls" +version = "0.1.0" +dependencies = [ + "uniffi", +] + [[package]] name = "uniffi-fixture-futures" version = "0.21.0" @@ -1896,6 +1910,13 @@ dependencies = [ "uniffi_meta", ] +[[package]] +name = "uniffi-fixture-primitive-types" +version = "0.1.0" +dependencies = [ + "uniffi", +] + [[package]] name = "uniffi-fixture-proc-macro" version = "0.22.0" diff --git a/Cargo.toml b/Cargo.toml index a6dc4a83e4..14532a2c32 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,9 @@ members = [ "examples/todolist", "examples/traits", + "bindings-tests", + "bindings-tests/fixtures/*", + "fixtures/benchmarks", "fixtures/coverall", "fixtures/callbacks", diff --git a/bindings-tests/Cargo.toml b/bindings-tests/Cargo.toml new file mode 100644 index 0000000000..a3ae2519be --- /dev/null +++ b/bindings-tests/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "bindings-tests" +version = "0.1.0" +edition = "2021" + +[dependencies] +uniffi = { path = "../uniffi", features = ["bindgen-tests"] } + +[features] +ffi-trace = ["uniffi/ffi-trace"] diff --git a/bindings-tests/fixtures/fn-calls/Cargo.toml b/bindings-tests/fixtures/fn-calls/Cargo.toml new file mode 100644 index 0000000000..fe5d5e67b0 --- /dev/null +++ b/bindings-tests/fixtures/fn-calls/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "uniffi-fixture-fn-calls" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +uniffi = { workspace = true } + +[features] +ffi-trace = ["uniffi/ffi-trace"] diff --git a/bindings-tests/fixtures/fn-calls/src/lib.rs b/bindings-tests/fixtures/fn-calls/src/lib.rs new file mode 100644 index 0000000000..f9b8417380 --- /dev/null +++ b/bindings-tests/fixtures/fn-calls/src/lib.rs @@ -0,0 +1,9 @@ +//! Extremely simple fixture to test making scaffolding calls without any arguments or return types +//! +//! The test is if the bindings can make a call to `test_func`. If in doubt, run the tests with +//! `--features=ffi-trace` to check that the function is actually called. + +#[uniffi::export] +pub fn test_func() {} + +uniffi::setup_scaffolding!("fn_calls"); diff --git a/bindings-tests/fixtures/primitive-types/Cargo.toml b/bindings-tests/fixtures/primitive-types/Cargo.toml new file mode 100644 index 0000000000..f855240e70 --- /dev/null +++ b/bindings-tests/fixtures/primitive-types/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "uniffi-fixture-primitive-types" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +uniffi = { workspace = true } + +[features] +ffi-trace = ["uniffi/ffi-trace"] diff --git a/bindings-tests/fixtures/primitive-types/src/lib.rs b/bindings-tests/fixtures/primitive-types/src/lib.rs new file mode 100644 index 0000000000..a69273ca26 --- /dev/null +++ b/bindings-tests/fixtures/primitive-types/src/lib.rs @@ -0,0 +1,46 @@ +//! Test lifting/lowering primitive types + +// Simple tests + +#[uniffi::export] +pub fn roundtrip(a: u32) -> u32 { + a +} + +#[uniffi::export] +pub fn roundtrip_bool(a: bool) -> bool { + a +} + +#[uniffi::export] +pub fn roundtrip_string(a: String) -> String { + a +} + +/// Complex test: input a bunch of different values and add them together +#[uniffi::export] +pub fn sum( + a: u8, + b: i8, + c: u16, + d: i16, + e: u32, + f: i32, + g: u64, + h: i64, + i: f32, + j: f64, + negate: bool, +) -> f64 { + let all_values = [ + a as f64, b as f64, c as f64, d as f64, e as f64, f as f64, g as f64, h as f64, i as f64, j, + ]; + let sum: f64 = all_values.into_iter().sum(); + if negate { + -sum + } else { + sum + } +} + +uniffi::setup_scaffolding!("primitive_types"); diff --git a/bindings-tests/python/uniffi-fixture-fn-calls.py b/bindings-tests/python/uniffi-fixture-fn-calls.py new file mode 100644 index 0000000000..cc8a4541db --- /dev/null +++ b/bindings-tests/python/uniffi-fixture-fn-calls.py @@ -0,0 +1,12 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import fn_calls +import unittest + +class FnCallsTest(unittest.TestCase): + def test_fn_call(self): + fn_calls.test_func() + +unittest.main() diff --git a/bindings-tests/python/uniffi-fixture-primitive-types.py b/bindings-tests/python/uniffi-fixture-primitive-types.py new file mode 100644 index 0000000000..7d1a4e957a --- /dev/null +++ b/bindings-tests/python/uniffi-fixture-primitive-types.py @@ -0,0 +1,23 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import primitive_types +import unittest + +class FnCallsTest(unittest.TestCase): + def test_roundtrip(self): + self.assertEqual(primitive_types.roundtrip(0), 0) + self.assertEqual(primitive_types.roundtrip(42), 42) + + def test_roundtrip_bool(self): + self.assertEqual(primitive_types.roundtrip_bool(True), True) + self.assertEqual(primitive_types.roundtrip_bool(False), False) + + def test_roundtrip_string(self): + self.assertEqual(primitive_types.roundtrip_string("Hello"), "Hello") + + def test_sum(self): + self.assertEqual(primitive_types.sum(1, -1, 2, -2, 3, -3, 4, -4, 0.5, 1.5, True), -2.0) + +unittest.main() diff --git a/bindings-tests/src/lib.rs b/bindings-tests/src/lib.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/bindings-tests/src/lib.rs @@ -0,0 +1 @@ + diff --git a/bindings-tests/tests/tests.rs b/bindings-tests/tests/tests.rs new file mode 100644 index 0000000000..341695658b --- /dev/null +++ b/bindings-tests/tests/tests.rs @@ -0,0 +1,11 @@ +use uniffi::deps::anyhow::Result; + +uniffi::bindings_tests!( + py: run_python_test, + // TODO Kotlin/Swift/Ruby +); + +pub fn run_python_test(tmp_dir: &str, fixture_name: &str) -> Result<()> { + let script_name = format!("python/{fixture_name}.py"); + uniffi::python_test::run_test(tmp_dir, fixture_name, &script_name) +} diff --git a/uniffi/src/lib.rs b/uniffi/src/lib.rs index 42b94c0438..4011ac17cf 100644 --- a/uniffi/src/lib.rs +++ b/uniffi/src/lib.rs @@ -25,6 +25,8 @@ pub use uniffi_bindgen::{ #[cfg(feature = "build")] pub use uniffi_build::{generate_scaffolding, generate_scaffolding_for_crate}; #[cfg(feature = "bindgen-tests")] +pub use uniffi_macros::bindings_tests; +#[cfg(feature = "bindgen-tests")] pub use uniffi_macros::build_foreign_language_testcases; #[cfg(feature = "cli")] diff --git a/uniffi_bindgen/src/bindings/python/test.rs b/uniffi_bindgen/src/bindings/python/test.rs index 39dbc9c136..a3cc9bee17 100644 --- a/uniffi_bindgen/src/bindings/python/test.rs +++ b/uniffi_bindgen/src/bindings/python/test.rs @@ -5,7 +5,7 @@ License, v. 2.0. If a copy of the MPL was not distributed with this use crate::bindings::RunScriptOptions; use crate::cargo_metadata::CrateConfigSupplier; use crate::library_mode::generate_bindings; -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use camino::Utf8Path; use std::env; use std::ffi::OsString; @@ -33,7 +33,11 @@ pub fn run_script( args: Vec, _options: &RunScriptOptions, ) -> Result<()> { - let script_path = Utf8Path::new(script_file).canonicalize_utf8()?; + let script_path = Utf8Path::new(script_file); + if !script_path.exists() { + bail!("{script_path} not found"); + } + let script_path = script_path.canonicalize_utf8()?; let test_helper = UniFFITestHelper::new(crate_name)?; let out_dir = test_helper.create_out_dir(tmp_dir, &script_path)?; let cdylib_path = test_helper.copy_cdylib_to_out_dir(&out_dir)?; diff --git a/uniffi_macros/src/lib.rs b/uniffi_macros/src/lib.rs index 7d5ff77772..5fdf29f4bf 100644 --- a/uniffi_macros/src/lib.rs +++ b/uniffi_macros/src/lib.rs @@ -32,6 +32,14 @@ use self::{ object::expand_object, record::expand_record, }; +/// A macro to build test cases for a bindings generator. +/// +/// See `bindings-tests/tests/tests.rs` for an example of how this is used. +#[proc_macro] +pub fn bindings_tests(tokens: TokenStream) -> TokenStream { + test::bindings_tests(tokens) +} + /// A macro to build testcases for a component's generated bindings. /// /// This macro provides some plumbing to write automated tests for the generated diff --git a/uniffi_macros/src/test.rs b/uniffi_macros/src/test.rs index e6ae142ce1..2138d45d43 100644 --- a/uniffi_macros/src/test.rs +++ b/uniffi_macros/src/test.rs @@ -6,7 +6,88 @@ use camino::{Utf8Path, Utf8PathBuf}; use proc_macro::TokenStream; use quote::{format_ident, quote}; use std::env; -use syn::{parse_macro_input, punctuated::Punctuated, LitStr, Token}; +use syn::{parse_macro_input, punctuated::Punctuated, Ident, LitStr, Token}; + +const TEST_FIXTURES: &[&'static str] = &[ + "uniffi-fixture-fn-calls", + "uniffi-fixture-primitive-types", + // TODO + // "uniffi-fixture-compound-types", + // "uniffi-fixture-extra-types", + // "uniffi-fixture-errors", + // "uniffi-fixture-records", + // "uniffi-fixture-enums", + // "uniffi-fixture-interfaces", + // "uniffi-fixture-interface-errors", + // "uniffi-fixture-callbacks", + // "uniffi-fixture-trait-interfaces", + // "uniffi-fixture-async", + // "uniffi-fixture-async-callbacks", + // "uniffi-fixture-custom-types", + // "uniffi-fixture-external-types", + // "uniffi-fixture-keywords", + // "uniffi-fixture-defaults", + // "uniffi-fixture-rust-trait-methods", +]; + +pub(crate) fn bindings_tests(tokens: TokenStream) -> TokenStream { + let input = parse_macro_input!(tokens as BindingsTestsInput); + + // For each test file found, generate a matching testcase. + let test_functions = input + .runners + .iter() + .flat_map(|runner| TEST_FIXTURES.iter().map(move |fixture| (fixture, runner))) + .map(|(fixture, runner)| { + let runner_name = &runner.name; + let runner_ident = &runner.runner; + let test_name = format_ident!( + "{}_{runner_name}", + fixture.replace(|c: char| !c.is_alphanumeric(), "_") + ); + quote! { + #[test] + fn #test_name () -> ::uniffi::deps::anyhow::Result<()> { + #runner_ident(std::env!("CARGO_TARGET_TMPDIR"), #fixture) + } + } + }) + .collect::>(); + let test_module = quote! { + #(#test_functions)* + }; + TokenStream::from(test_module) +} + +struct BindingsTestsInput { + runners: Vec, +} + +struct BindingsTestRunner { + name: Ident, + _sep: Token![:], + runner: Ident, +} + +impl syn::parse::Parse for BindingsTestsInput { + fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + let runners = Punctuated::::parse_terminated(input)? + .into_iter() + .collect(); + + Ok(Self { runners }) + } +} + +impl syn::parse::Parse for BindingsTestRunner { + fn parse(input: syn::parse::ParseStream<'_>) -> syn::Result { + Ok(Self { + name: input.parse()?, + _sep: input.parse()?, + runner: input.parse()?, + }) + } +} pub(crate) fn build_foreign_language_testcases(tokens: TokenStream) -> TokenStream { let input = parse_macro_input!(tokens as BuildForeignLanguageTestCaseInput); diff --git a/uniffi_testing/src/lib.rs b/uniffi_testing/src/lib.rs index 34584899b1..e4f6533a4a 100644 --- a/uniffi_testing/src/lib.rs +++ b/uniffi_testing/src/lib.rs @@ -8,11 +8,12 @@ use cargo_metadata::{Message, Metadata, MetadataCommand, Package, Target}; use fs_err as fs; use once_cell::sync::Lazy; use std::{ - collections::hash_map::DefaultHasher, + collections::{hash_map::DefaultHasher, HashMap}, env, env::consts::DLL_EXTENSION, hash::{Hash, Hasher}, process::{Command, Stdio}, + sync::Mutex, }; // A source to compile for a test @@ -25,7 +26,8 @@ pub struct CompileSource { // Store Cargo output to in a static to avoid calling it more than once. static CARGO_METADATA: Lazy = Lazy::new(get_cargo_metadata); -static CARGO_BUILD_MESSAGES: Lazy> = Lazy::new(get_cargo_build_messages); +static CARGO_BUILD_MESSAGES: Lazy>>> = + Lazy::new(|| Mutex::new(HashMap::new())); /// Struct for running fixture and example tests for bindings generators /// @@ -61,6 +63,12 @@ impl UniFFITestHelper { } fn find_name_and_cdylib_path(package: &Package) -> Result<(String, Utf8PathBuf)> { + let build_messages = CARGO_BUILD_MESSAGES + .lock() + .unwrap() + .entry(package.name.clone()) + .or_insert_with(|| get_cargo_build_messages_for_crate(&package.name)) + .clone(); let cdylib_targets: Vec<&Target> = package .targets .iter() @@ -72,18 +80,16 @@ impl UniFFITestHelper { }; let target_name = target.name.replace('-', "_"); - let artifacts = CARGO_BUILD_MESSAGES - .iter() - .filter_map(|message| match message { - Message::CompilerArtifact(artifact) => { - if artifact.target == *target { - Some(artifact.clone()) - } else { - None - } + let artifacts = build_messages.iter().filter_map(|message| match message { + Message::CompilerArtifact(artifact) => { + if artifact.target == *target { + Some(artifact.clone()) + } else { + None } - _ => None, - }); + } + _ => None, + }); let cdylib_files: Vec = artifacts .into_iter() .flat_map(|artifact| { @@ -161,7 +167,7 @@ fn get_cargo_metadata() -> Metadata { .expect("error running cargo metadata") } -fn get_cargo_build_messages() -> Vec { +fn get_cargo_build_messages_for_crate(crate_name: &str) -> Vec { #[cfg(feature = "ffi-trace")] let features_arg = "--features=ffi-trace"; @@ -169,9 +175,10 @@ fn get_cargo_build_messages() -> Vec { let features_arg = "--features="; let mut child = Command::new(env!("CARGO")) - .arg("test") + .arg("build") + .arg("-p") + .arg(crate_name) .arg(features_arg) - .arg("--no-run") .arg("--message-format=json") .stdout(Stdio::piped()) .spawn()