From 4835699f31bb99f55761f0f4aca2c2afccc557b1 Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sat, 21 Dec 2024 15:43:20 +0800 Subject: [PATCH 1/9] Refactor `nvim_oxi::tests` into a directory structure --- crates/macros/src/test.rs | 8 +- src/lib.rs | 4 - src/tests/mod.rs | 7 ++ src/tests/terminator.rs | 50 ++++++++ src/{tests.rs => tests/test_macro.rs} | 166 ++++++++++---------------- 5 files changed, 123 insertions(+), 112 deletions(-) create mode 100644 src/tests/mod.rs create mode 100644 src/tests/terminator.rs rename src/{tests.rs => tests/test_macro.rs} (77%) diff --git a/crates/macros/src/test.rs b/crates/macros/src/test.rs index 533a4018..ba853a36 100644 --- a/crates/macros/src/test.rs +++ b/crates/macros/src/test.rs @@ -45,13 +45,13 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { fn __test_fn(#terminator) #ret { #block } - #nvim_oxi::tests::plugin_body_with_terminator(__test_fn) + #nvim_oxi::tests::test_macro::plugin_body_with_terminator(__test_fn) }, None => quote! { fn __test_fn() #ret { #block } - #nvim_oxi::tests::plugin_body(__test_fn) + #nvim_oxi::tests::test_macro::plugin_body(__test_fn) }, }; @@ -60,14 +60,14 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { fn __test_fn() #ret { #block } - #nvim_oxi::tests::plugin_body(__test_fn) + #nvim_oxi::tests::test_macro::plugin_body(__test_fn) }; quote! { #[test] #test_attrs fn #test_name() -> ::core::result::Result<(), ::std::string::String> { - #nvim_oxi::tests::test_body( + #nvim_oxi::tests::test_macro::test_body( env!("CARGO_CRATE_NAME"), env!("CARGO_MANIFEST_DIR"), stringify!(#plugin_name), diff --git a/src/lib.rs b/src/lib.rs index 9d65b488..0d657caf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -90,10 +90,6 @@ pub use macros::plugin; pub use macros::test; pub use types::*; #[cfg(feature = "test")] -#[doc(hidden)] pub mod tests; -#[cfg(feature = "test-terminator")] -#[cfg_attr(docsrs, doc(cfg(feature = "test-terminator")))] -pub use tests::{TestFailure, TestTerminator}; pub use toplevel::*; pub use types::string; diff --git a/src/tests/mod.rs b/src/tests/mod.rs new file mode 100644 index 00000000..cdf78fdd --- /dev/null +++ b/src/tests/mod.rs @@ -0,0 +1,7 @@ +#[cfg(feature = "test-terminator")] +mod terminator; +#[doc(hidden)] +pub mod r#test_macro; + +#[cfg(feature = "test-terminator")] +pub use terminator::{TestFailure, TestTerminator}; diff --git a/src/tests/terminator.rs b/src/tests/terminator.rs new file mode 100644 index 00000000..76b3818c --- /dev/null +++ b/src/tests/terminator.rs @@ -0,0 +1,50 @@ +use core::fmt::Display; +use std::panic::PanicHookInfo; +use std::sync::{Arc, OnceLock}; + +/// The error type given to [`TestTerminator::terminate`]. +/// +/// The two variants of this enum represent the two ways a test can fail: +/// either by returning an error or by panicking. +#[cfg_attr(docsrs, doc(cfg(feature = "test-terminator")))] +pub enum TestFailure<'a, E> { + /// This is used to indicate that the test failed due to an error being + /// returned from the test function. + Error(E), + + /// This is used to indicate that the test failed due to a panic. The + /// [`PanicHookInfo`](std::panic::PanicHookInfo) contains information about + /// the panic and can be obtained by calling + /// [`set_hook`](std::panic::set_hook). + Panic(&'a PanicHookInfo<'a>), +} + +/// A handle used to terminate a test annotated by [`test`](crate::test). +/// +/// The `test` macro works by turning the annotated function into its own +/// plugin, which is then loaded by Neovim and evalutated by `require`ing it +/// when the test is run, before immediately quitting. +/// +/// When testing asynchronous code this can be problematic, as the test may +/// need to continue running after the generated plugin has been `require`d. +/// +/// To allow for this, the test function can take a `TestTerminator` as its +/// only argument. This allows the test to be terminated asynchronously by +/// calling [`terminate`](Self::terminate). +/// +/// Note that if the `TestTerminator` is dropped without first calling +/// `terminate`, the test will run forever. +#[cfg_attr(docsrs, doc(cfg(feature = "test-terminator")))] +pub struct TestTerminator { + pub(super) lock: Arc>>, + pub(super) handle: crate::libuv::AsyncHandle, +} + +impl TestTerminator { + /// Terminates the test and consumes the `TestTerminator`. + pub fn terminate(self, res: Result<(), TestFailure<'_, E>>) { + let res = res.map_err(Into::into); + let Ok(()) = self.lock.set(res) else { unreachable!() }; + self.handle.send().unwrap(); + } +} diff --git a/src/tests.rs b/src/tests/test_macro.rs similarity index 77% rename from src/tests.rs rename to src/tests/test_macro.rs index 2a25c7e7..288b103f 100644 --- a/src/tests.rs +++ b/src/tests/test_macro.rs @@ -1,3 +1,5 @@ +//! Functions called by the code generated by `#[nvim_oxi::test].` + use std::any::Any; use std::env; use std::fmt::{Debug, Display}; @@ -12,32 +14,8 @@ use miniserde::json; use crate::IntoResult; -/// Returns the `target` directory in which cargo will place the compiled -/// artifacts for the crate whose manifest is located at `manifest_dir`. -pub fn target_dir(manifest_dir: &Path) -> PathBuf { - let output = Command::new( - env::var("CARGO").ok().unwrap_or_else(|| "cargo".to_owned()), - ) - .arg("metadata") - .arg("--format-version=1") - .arg("--no-deps") - .current_dir(manifest_dir) - .output() - .unwrap(); - - let object: json::Object = - json::from_str(&String::from_utf8(output.stdout).unwrap()).unwrap(); - - let target_dir = match object.get("target_directory").unwrap() { - json::Value::String(s) => s, - _ => panic!("must be string value"), - }; - - target_dir.into() -} - -/// This function is used as the body of the `#[nvim_oxi::plugin]` generated by -/// the `#[nvim_oxi::test]` macro. +/// The body of the `#[nvim_oxi::plugin]` generated by the `#[nvim_oxi::test]` +/// macro. pub fn plugin_body(test_body: F) where F: FnOnce() -> R + UnwindSafe, @@ -63,13 +41,13 @@ where exit(result); } -/// This function is used as the body of the `#[nvim_oxi::plugin]` generated by -/// the `#[nvim_oxi::test]` macro when the `test-terminator` feature is enabled -/// and the test function takes a `TestTerminator` argument. +/// The body of the `#[nvim_oxi::plugin]` generated by the `#[nvim_oxi::test]` +/// macro when the `test-terminator` feature is enabled and the test function +/// takes a `TestTerminator` argument. #[cfg(feature = "test-terminator")] pub fn plugin_body_with_terminator(test_body: F) where - F: FnOnce(TestTerminator), + F: FnOnce(super::terminator::TestTerminator), { let lock = Arc::new(OnceLock::>::new()); @@ -84,75 +62,10 @@ where } .unwrap(); - test_body(TestTerminator { lock, handle }); -} - -/// A handle used to terminate a test annotated by [`test`](crate::test). -/// -/// The `test` macro works by turning the annotated function into its own -/// plugin, which is then loaded by Neovim and evalutated by `require`ing it -/// when the test is run, before immediately quitting. -/// -/// When testing asynchronous code this can be problematic, as the test may -/// need to continue running after the generated plugin has been `require`d. -/// -/// To allow for this, the test function can take a `TestTerminator` as its -/// only argument. This allows the test to be terminated asynchronously by -/// calling [`terminate`](Self::terminate). -/// -/// Note that if the `TestTerminator` is dropped without first calling -/// `terminate`, the test will run forever. -#[cfg(feature = "test-terminator")] -pub struct TestTerminator { - lock: Arc>>, - handle: crate::libuv::AsyncHandle, -} - -#[cfg(feature = "test-terminator")] -impl TestTerminator { - /// Terminates the test and consumes the `TestTerminator`. - pub fn terminate(self, res: Result<(), TestFailure<'_, E>>) { - let res = res.map_err(Into::into); - let Ok(()) = self.lock.set(res) else { unreachable!() }; - self.handle.send().unwrap(); - } -} - -/// The error type given to [`TestTerminator::terminate`]. -/// -/// The two variants of this enum represent the two ways a test can fail: -/// either by returning an error or by panicking. -pub enum TestFailure<'a, E> { - /// This is used to indicate that the test failed due to an error being - /// returned from the test function. - Error(E), - - /// This is used to indicate that the test failed due to a panic. The - /// [`PanicHookInfo`](std::panic::PanicHookInfo) contains information about - /// the panic and can be obtained by calling - /// [`set_hook`](std::panic::set_hook). - Panic(&'a std::panic::PanicHookInfo<'a>), -} - -fn exit(result: Result<(), Failure>) { - #[cfg(all(feature = "neovim-0-9", not(feature = "neovim-0-10")))] - let exec = |cmd: &str| crate::api::exec(cmd, false).unwrap(); - - #[cfg(feature = "neovim-0-10")] - let exec = |cmd: &str| { - let opts = crate::api::opts::ExecOpts::builder().output(false).build(); - crate::api::exec2(cmd, &opts).unwrap(); - }; - - if let Err(failure) = result { - eprintln!("{failure}"); - exec("cquit 1"); - } else { - exec("qall!"); - } + test_body(super::terminator::TestTerminator { lock, handle }); } -/// TODO: docs +/// The body of the `#[test]` generated by the `#[nvim_oxi::test]` macro. pub fn test_body( crate_name: &str, manifest_dir: &str, @@ -219,7 +132,48 @@ pub fn test_body( } } -/// TODO: docs +/// Returns the `target` directory in which cargo will place the compiled +/// artifacts for the crate whose manifest is located at `manifest_dir`. +fn target_dir(manifest_dir: &Path) -> PathBuf { + let output = Command::new( + env::var("CARGO").ok().unwrap_or_else(|| "cargo".to_owned()), + ) + .arg("metadata") + .arg("--format-version=1") + .arg("--no-deps") + .current_dir(manifest_dir) + .output() + .unwrap(); + + let object: json::Object = + json::from_str(&String::from_utf8(output.stdout).unwrap()).unwrap(); + + let target_dir = match object.get("target_directory").unwrap() { + json::Value::String(s) => s, + _ => panic!("must be string value"), + }; + + target_dir.into() +} + +fn exit(result: Result<(), Failure>) { + #[cfg(all(feature = "neovim-0-9", not(feature = "neovim-0-10")))] + let exec = |cmd: &str| crate::api::exec(cmd, false).unwrap(); + + #[cfg(feature = "neovim-0-10")] + let exec = |cmd: &str| { + let opts = crate::api::opts::ExecOpts::builder().output(false).build(); + crate::api::exec2(cmd, &opts).unwrap(); + }; + + if let Err(failure) = result { + eprintln!("{failure}"); + exec("cquit 1"); + } else { + exec("qall!"); + } +} + fn run_nvim_command( crate_name: &str, manifest_dir: &str, @@ -267,7 +221,7 @@ fn run_nvim_command( } #[derive(Clone)] -struct PanicInfo { +pub(super) struct PanicInfo { msg: String, thread: String, file: Option, @@ -376,7 +330,7 @@ impl From<&panic::PanicHookInfo<'_>> for PanicInfo { } #[derive(Clone)] -enum Failure { +pub(super) enum Failure { Error(String), Panic(PanicInfo), } @@ -402,11 +356,15 @@ impl FromStr for Failure { } #[cfg(feature = "test-terminator")] -impl From> for Failure { - fn from(err: TestFailure<'_, E>) -> Self { +impl From> for Failure { + fn from(err: super::terminator::TestFailure<'_, E>) -> Self { match err { - TestFailure::Error(err) => Self::Error(err.to_string()), - TestFailure::Panic(info) => Self::Panic(info.into()), + super::terminator::TestFailure::Error(err) => { + Self::Error(err.to_string()) + }, + super::terminator::TestFailure::Panic(info) => { + Self::Panic(info.into()) + }, } } } From 5a13b2bbc585ca24b3c3ecbab92a7c3a940482f9 Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sat, 21 Dec 2024 18:29:25 +0800 Subject: [PATCH 2/9] Implement `nvim_oxi::tests::build()` and use it in `/tests` --- Cargo.toml | 14 +- crates/macros/src/test.rs | 2 +- src/tests/build.rs | 283 ++++++++++++++++++++++++++++++++++++++ src/tests/mod.rs | 2 + src/tests/test_macro.rs | 50 ++----- tests/Cargo.toml | 3 + tests/build.rs | 3 + 7 files changed, 315 insertions(+), 42 deletions(-) create mode 100644 src/tests/build.rs create mode 100644 tests/build.rs diff --git a/Cargo.toml b/Cargo.toml index 496564e6..c9c2bc12 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,9 +20,10 @@ api = { path = "./crates/api", package = "nvim-oxi-api" } libuv = { path = "./crates/libuv", package = "nvim-oxi-libuv" } luajit = { path = "./crates/luajit", package = "nvim-oxi-luajit" } macros = { path = "./crates/macros", package = "nvim-oxi-macros" } -thiserror = "1.0" types = { path = "./crates/types", package = "nvim-oxi-types" } +thiserror = "1.0" + [workspace.lints.clippy] mixed_attributes_style = "allow" @@ -48,19 +49,20 @@ neovim-nightly = ["api/neovim-nightly"] libuv = ["dep:libuv"] mlua = ["dep:mlua"] -test = ["macros/test", "miniserde"] +test = ["macros/test", "dep:cargo_metadata"] test-terminator = ["test", "libuv", "macros/test-terminator"] __vendored_luajit = ["mlua/vendored"] [dependencies] api = { workspace = true } -libuv = { workspace = true, optional = true } luajit = { workspace = true } macros = { workspace = true, features = ["plugin"] } -miniserde = { version = "0.1", optional = true } -mlua = { version = "0.10", features = ["luajit"], optional = true } -thiserror = { workspace = true } types = { workspace = true, features = ["serde"] } +libuv = { workspace = true, optional = true } + +thiserror = { workspace = true } +cargo_metadata = { version = "0.19", optional = true } +mlua = { version = "0.10", features = ["luajit"], optional = true } [dev-dependencies] mlua = { version = "0.10", features = ["luajit", "module"] } diff --git a/crates/macros/src/test.rs b/crates/macros/src/test.rs index ba853a36..3d3bd378 100644 --- a/crates/macros/src/test.rs +++ b/crates/macros/src/test.rs @@ -69,7 +69,7 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { fn #test_name() -> ::core::result::Result<(), ::std::string::String> { #nvim_oxi::tests::test_macro::test_body( env!("CARGO_CRATE_NAME"), - env!("CARGO_MANIFEST_DIR"), + env!("CARGO_MANIFEST_PATH"), stringify!(#plugin_name), #library_path, #extra_cmd, diff --git a/src/tests/build.rs b/src/tests/build.rs new file mode 100644 index 00000000..2ef450d5 --- /dev/null +++ b/src/tests/build.rs @@ -0,0 +1,283 @@ +use core::error::Error; +use std::ffi::OsStr; +use std::path::Path; +use std::process::Command; +use std::{env, io}; + +/// TODO: docs. +pub fn build() -> Result<(), BuildError> { + let Some(_g) = BuildGuard::::new()? else { return Ok(()) }; + let compilation_opts = CompilationOpts::from_env()?; + let manifest_path = EnvVar::get("CARGO_MANIFEST_PATH")?; + let manifest = CargoManifest::from_path(manifest_path.as_str())?; + let features = EnabledFeatures::from_env(&manifest)?; + BuildCommand::new(&manifest, &compilation_opts, &features).exec()?; + println!( + "cargo:rustc-env={}={}", + manifest.profile_env(), + compilation_opts.profile.as_str() + ); + Ok(()) +} + +/// TODO: docs. +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct BuildError { + #[from] + kind: BuildErrorKind, +} + +pub(super) struct CargoManifest { + metadata: cargo_metadata::Metadata, +} + +struct BuildGuard { + guard: Option, +} + +struct EnvVarGuard; + +struct CompilationOpts { + profile: Profile, +} + +enum Profile { + Debug, + Release, + Other(EnvVar), +} + +struct EnabledFeatures { + features: Vec, +} + +struct BuildCommand { + command: Command, +} + +impl EnvVarGuard { + const NAME: &'static str = "NVIM_OXI_BUILDING_TESTS"; +} + +struct EnvVar(String); + +#[derive(Debug, thiserror::Error)] +enum BuildErrorKind { + #[error("couldn't build tests: {0}")] + Build(io::Error), + + #[error("couldn't acquire guard: {0}")] + CouldntAcquireGuard(Box), + + #[error("couldn't read manifest: {0}")] + CouldntReadManifest(cargo_metadata::Error), + + #[error( + "nvim_oxi::tests::build() can only be used inside a build script" + )] + NotInBuildScript, + + #[error("couldn't get the root package")] + NoRootPackage, +} + +impl BuildGuard { + fn new() -> Result, BuildError> { + match G::acquire() { + Ok(guard) => Ok(Some(Self { guard: Some(guard) })), + Err(Ok(_busy)) => Ok(None), + Err(Err(acquire_err)) => { + Err(BuildErrorKind::CouldntAcquireGuard(Box::new(acquire_err)) + .into()) + }, + } + } +} + +impl CompilationOpts { + fn from_env() -> Result { + Ok(Self { profile: Profile::from_env_var(EnvVar::get("PROFILE")?) }) + } +} + +impl Profile { + fn as_args(&self) -> Vec + '_> { + enum Arg<'a> { + Str(&'a str), + EnvVar(&'a EnvVar), + } + + impl AsRef for Arg<'_> { + fn as_ref(&self) -> &OsStr { + match self { + Arg::Str(s) => s.as_ref(), + Arg::EnvVar(s) => s.as_str().as_ref(), + } + } + } + + match self { + Profile::Debug => vec![], + Profile::Release => vec![Arg::Str("--release")], + Profile::Other(other) => { + vec![Arg::Str("--profile"), Arg::EnvVar(other)] + }, + } + } + + fn as_str(&self) -> &str { + match self { + Profile::Debug => "debug", + Profile::Release => "release", + Profile::Other(other) => other.as_str(), + } + } + + fn from_env_var(profile: EnvVar) -> Self { + match profile.as_str() { + "debug" => Self::Debug, + "release" => Self::Release, + _ => Self::Other(profile), + } + } +} + +impl CargoManifest { + pub(super) fn from_path( + path: impl AsRef, + ) -> Result { + let metadata = cargo_metadata::MetadataCommand::new() + .manifest_path(path.as_ref()) + .exec() + .map_err(BuildErrorKind::CouldntReadManifest)?; + + if metadata.root_package().is_none() { + return Err(BuildErrorKind::NoRootPackage.into()); + } + + Ok(Self { metadata }) + } + + /// The name of the environment variable representing the profile the test + /// crate was compiled for. + pub(super) fn profile_env(&self) -> String { + format!( + "NVIM_OXI_TEST_BUILD_PROFILE_{}", + self.package().name.to_ascii_uppercase().replace('-', "_") + ) + } + + /// The path to the target directory containing the compiled test library + /// for the crate represented by this [`CargoManifest`]. + pub(super) fn target_dir(&self) -> impl Into { + self.metadata + .target_directory + // We have to use a different target directory to avoid a deadlock + // caused by invoking `cargo build` in a build script. + // + // See https://github.com/rust-lang/cargo/issues/6412 for more. + .join("nvim_oxi_tests") + // Namespace by the package name to allow for multiple test crates + // in the same workspace. + .join(&self.package().name) + } + + fn package(&self) -> &cargo_metadata::Package { + self.metadata.root_package().expect("checked when creating the ") + } +} + +impl EnabledFeatures { + fn from_env(manifest: &CargoManifest) -> Result { + let mut features = Vec::new(); + + for feature in manifest.package().features.keys() { + let env = format!( + "CARGO_FEATURE_{}", + feature.to_ascii_uppercase().replace('-', "_") + ); + if EnvVar::get(&env).is_ok() { + features.push(feature.clone()); + } + } + + Ok(Self { features }) + } +} + +impl BuildCommand { + fn exec(mut self) -> Result<(), BuildError> { + self.command + .status() + .map(|_| ()) + .map_err(|io_err| BuildErrorKind::Build(io_err).into()) + } + + fn new( + manifest: &CargoManifest, + compilation_opts: &CompilationOpts, + enabled_features: &EnabledFeatures, + ) -> Self { + let mut command = Command::new("cargo"); + command + .arg("build") + .args(compilation_opts.profile.as_args()) + .args(["--target-dir", manifest.target_dir().into().as_str()]) + .arg("--no-default-features") + .arg("--features") + .arg(enabled_features.features.join(",")); + Self { command } + } +} + +impl EnvVar { + fn as_str(&self) -> &str { + &self.0 + } + + fn get(env: &str) -> Result { + match env::var(env) { + Ok(value) => Ok(Self(value)), + Err(_) => Err(BuildErrorKind::NotInBuildScript.into()), + } + } +} + +impl Guard for EnvVarGuard { + type Error = env::VarError; + + fn acquire() -> Result> { + match env::var(Self::NAME) { + Ok(_) => Err(Ok(GuardBusy)), + Err(env::VarError::NotPresent) => unsafe { + env::set_var(Self::NAME, "1"); + Ok(Self) + }, + Err(var_error) => Err(Err(var_error)), + } + } + + fn release(self) -> Result<(), Self::Error> { + // Env variables are process-local. + Ok(()) + } +} + +impl Drop for BuildGuard { + fn drop(&mut self) { + if let Err(err) = self.guard.take().unwrap().release() { + panic!("couldn't release guard: {err}"); + } + } +} + +trait Guard: Sized { + type Error: Error + 'static; + fn acquire() -> Result>; + fn release(self) -> Result<(), Self::Error>; +} + +/// A sentinel value returned by [`Guard::acquire()`] indicating that the guard +/// has already been acquired by another build process. +struct GuardBusy; diff --git a/src/tests/mod.rs b/src/tests/mod.rs index cdf78fdd..cf9a20be 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,7 +1,9 @@ +mod build; #[cfg(feature = "test-terminator")] mod terminator; #[doc(hidden)] pub mod r#test_macro; +pub use build::{build, BuildError}; #[cfg(feature = "test-terminator")] pub use terminator::{TestFailure, TestTerminator}; diff --git a/src/tests/test_macro.rs b/src/tests/test_macro.rs index 288b103f..9585d471 100644 --- a/src/tests/test_macro.rs +++ b/src/tests/test_macro.rs @@ -10,8 +10,6 @@ use std::str::FromStr; use std::sync::{Arc, OnceLock}; use std::thread; -use miniserde::json; - use crate::IntoResult; /// The body of the `#[nvim_oxi::plugin]` generated by the `#[nvim_oxi::test]` @@ -68,7 +66,7 @@ where /// The body of the `#[test]` generated by the `#[nvim_oxi::test]` macro. pub fn test_body( crate_name: &str, - manifest_dir: &str, + manifest_path: &str, plugin_name: &str, library_path: Option>, extra_cmd: Option<&str>, @@ -91,7 +89,7 @@ pub fn test_body( let output = run_nvim_command( crate_name, - manifest_dir, + manifest_path, plugin_name, library_path, extra_cmd, @@ -127,35 +125,11 @@ pub fn test_body( }; match failure { - Failure::Error(err) => return Err(err), + Failure::Error(err) => Err(err), Failure::Panic(info) => panic::panic_any(info), } } -/// Returns the `target` directory in which cargo will place the compiled -/// artifacts for the crate whose manifest is located at `manifest_dir`. -fn target_dir(manifest_dir: &Path) -> PathBuf { - let output = Command::new( - env::var("CARGO").ok().unwrap_or_else(|| "cargo".to_owned()), - ) - .arg("metadata") - .arg("--format-version=1") - .arg("--no-deps") - .current_dir(manifest_dir) - .output() - .unwrap(); - - let object: json::Object = - json::from_str(&String::from_utf8(output.stdout).unwrap()).unwrap(); - - let target_dir = match object.get("target_directory").unwrap() { - json::Value::String(s) => s, - _ => panic!("must be string value"), - }; - - target_dir.into() -} - fn exit(result: Result<(), Failure>) { #[cfg(all(feature = "neovim-0-9", not(feature = "neovim-0-10")))] let exec = |cmd: &str| crate::api::exec(cmd, false).unwrap(); @@ -176,11 +150,19 @@ fn exit(result: Result<(), Failure>) { fn run_nvim_command( crate_name: &str, - manifest_dir: &str, + manifest_path: &str, plugin_name: &str, library_path: Option>, extra_cmd: Option<&str>, ) -> Result { + let manifest = super::build::CargoManifest::from_path(manifest_path) + .map_err(|err| err.to_string())?; + + let target_dir: PathBuf = manifest.target_dir().into().into(); + + let profile = + env::var(manifest.profile_env()).map_err(|err| err.to_string())?; + let library_path = library_path .map(|path| path.as_ref().to_owned()) .unwrap_or_else(|| { @@ -189,15 +171,13 @@ fn run_nvim_command( prefix = env::consts::DLL_PREFIX, suffix = env::consts::DLL_SUFFIX, ); - target_dir(Path::new(manifest_dir)) - .join("debug") - .join(library_name) + target_dir.join(profile).join(library_name) }); if !library_path.exists() { return Err(format!( - "Compiled library not found in '{}'. Please run `cargo build` \ - before running the tests.", + "couldn't find library at '{}'. Did you forget to use the build \ + script?", library_path.display() )); } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 03525420..cb0d965f 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -20,3 +20,6 @@ nvim-oxi = { path = "..", features = ["libuv", "test"] } [target.'cfg(any(target_os = "windows", target_env = "msvc"))'.dependencies] all_asserts = "2.3" nvim-oxi = { path = "..", features = ["test"] } + +[build-dependencies] +nvim-oxi = { path = "..", features = ["test"] } diff --git a/tests/build.rs b/tests/build.rs new file mode 100644 index 00000000..7f530fb5 --- /dev/null +++ b/tests/build.rs @@ -0,0 +1,3 @@ +fn main() -> Result<(), nvim_oxi::tests::BuildError> { + nvim_oxi::tests::build() +} From 51e001ed8b5b4c052c49c1d829c05ea968a237da Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:09:11 +0800 Subject: [PATCH 3/9] Add the `tests` crate to the workspace members --- Cargo.toml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c9c2bc12..87f99570 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,5 @@ [workspace] -members = ["crates/*", "examples/*"] -exclude = ["tests"] +members = ["crates/*", "examples/*", "tests"] resolver = "2" [workspace.package] @@ -44,8 +43,8 @@ rustdoc-args = ["--cfg", "docsrs"] [features] neovim-0-9 = ["api/neovim-0-9"] -neovim-0-10 = ["api/neovim-0-10"] -neovim-nightly = ["api/neovim-nightly"] +neovim-0-10 = ["neovim-0-9", "api/neovim-0-10"] +neovim-nightly = ["neovim-0-10", "api/neovim-nightly"] libuv = ["dep:libuv"] mlua = ["dep:mlua"] From f4bf0da0b3353aebe19db252bf52ae5d85c3149c Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:09:26 +0800 Subject: [PATCH 4/9] Fix unused import in `libuv`'s example --- examples/libuv.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/libuv.rs b/examples/libuv.rs index 1d31bf09..a8d22c4e 100644 --- a/examples/libuv.rs +++ b/examples/libuv.rs @@ -2,7 +2,7 @@ use std::thread; use std::time::Duration; use nvim_oxi::libuv::{AsyncHandle, TimerHandle}; -use nvim_oxi::{print, schedule, Error, Result}; +use nvim_oxi::{print, schedule, Result}; use tokio::sync::mpsc::{self, UnboundedSender}; use tokio::time; From 50b12a09d0420eca622f7c573054b2fe90aaeb38 Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:16:01 +0800 Subject: [PATCH 5/9] ci: stop building integration tests before running them --- .github/workflows/ci.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d76479ac..d5e2cb3f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,15 +36,9 @@ jobs: version: ${{ matrix.neovim }} - name: Install latest stable `rustc` uses: dtolnay/rust-toolchain@stable - - name: Run unit tests - run: cargo test ${{ matrix.features }} + - name: Run tests + run: cargo test --workspace ${{ matrix.features }} working-directory: . - - name: Build the `tests` crate - run: cargo build ${{ matrix.features }} - working-directory: ./tests - - name: Run integration tests - run: cargo test ${{ matrix.features }} - working-directory: ./tests clippy: name: clippy From 18e1cd0730522f68010b44dcc5f4cb6e2a7c39ac Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:21:37 +0800 Subject: [PATCH 6/9] Update `thiserror` to `2.0` --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 87f99570..250539d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ luajit = { path = "./crates/luajit", package = "nvim-oxi-luajit" } macros = { path = "./crates/macros", package = "nvim-oxi-macros" } types = { path = "./crates/types", package = "nvim-oxi-types" } -thiserror = "1.0" +thiserror = "2.0" [workspace.lints.clippy] mixed_attributes_style = "allow" From 1e15a4973eb62addb73d808b275414590abe00ee Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:27:04 +0800 Subject: [PATCH 7/9] macros: remove the `#[nvim_oxi::test(library_path = ..)]` attribute I added it to implement re-compiling the tests with on `cargo t` in `nvimx`, but that's now implemented directly in `nvim_oxi` so we don't need it anymore. --- crates/macros/src/lib.rs | 22 -------------------- crates/macros/src/test.rs | 44 --------------------------------------- src/tests/test_macro.rs | 32 ++++++++++------------------ 3 files changed, 11 insertions(+), 87 deletions(-) diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index 3c97241b..24e02897 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -141,28 +141,6 @@ pub fn plugin(attr: TokenStream, item: TokenStream) -> TokenStream { /// /// If the given string spans multiple lines, it will be joined into a single /// line using `;` as the separator. -/// -/// ## `library_path` -/// -/// The `library_path` attribute is used to specify the path to the compiled -/// shared library containing the code to test. This can be useful if you're -/// building your own macro on top of `test`, but should otherwise be left -/// unspecified. -/// -/// It can be set to any expression that evaluates to a type implementing -/// `AsRef`. -/// -/// ```ignore -/// # use nvim_oxi::api; -/// # use std::path::PathBuf; -/// #[nvim_oxi::test(library_path = my_custom_library_path())] -/// fn print_42() -> Result<(), api::Error> { -/// api::command("lua print(42)") -/// } -/// -/// fn my_custom_library_path() -> PathBuf { -/// todo!(); -/// } /// ``` #[cfg(feature = "test")] #[proc_macro_attribute] diff --git a/crates/macros/src/test.rs b/crates/macros/src/test.rs index 3d3bd378..c333d75d 100644 --- a/crates/macros/src/test.rs +++ b/crates/macros/src/test.rs @@ -27,13 +27,6 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { let nvim_oxi = &attrs.nvim_oxi; - let library_path = match &attrs.library_path { - Some(LibraryPath { path, .. }) => { - quote! { ::core::option::Option::Some(#path) } - }, - None => quote! { ::core::option::Option::<&str>::None }, - }; - let extra_cmd = match &attrs.cmd { Some(Cmd { cmd, .. }) => quote! { ::core::option::Option::Some(#cmd) }, None => quote! { ::core::option::Option::None }, @@ -71,7 +64,6 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { env!("CARGO_CRATE_NAME"), env!("CARGO_MANIFEST_PATH"), stringify!(#plugin_name), - #library_path, #extra_cmd, ) } @@ -87,7 +79,6 @@ pub fn test(attrs: TokenStream, item: TokenStream) -> TokenStream { #[derive(Default)] struct Attributes { cmd: Option, - library_path: Option, nvim_oxi: NvimOxi, } @@ -106,12 +97,6 @@ impl Parse for Attributes { } this.cmd = Some(cmd); }, - Attribute::LibraryPath(library_path) => { - if this.library_path.is_some() { - return Err(DuplicateError(library_path).into()); - } - this.library_path = Some(library_path); - }, Attribute::NvimOxi(nvim_oxi) => { if has_parsed_nvim_oxi { return Err(DuplicateError(nvim_oxi).into()); @@ -132,7 +117,6 @@ impl Parse for Attributes { enum Attribute { Cmd(Cmd), - LibraryPath(LibraryPath), NvimOxi(NvimOxi), } @@ -142,7 +126,6 @@ impl Parse for Attribute { input .parse::() .map(Self::Cmd) - .or_else(|_| input.parse::().map(Self::LibraryPath)) .or_else(|_| input.parse::().map(Self::NvimOxi)) } } @@ -182,30 +165,3 @@ impl ToTokens for Cmd { lit.to_tokens(tokens); } } - -/// The path to the compiled test library. -struct LibraryPath { - key_span: Span, - path: Expr, -} - -impl KeyedAttribute for LibraryPath { - const KEY: &'static str = "library_path"; - - type Value = Expr; - - #[inline] - fn key_span(&self) -> Span { - self.key_span - } -} - -impl Parse for LibraryPath { - #[inline] - fn parse(input: ParseStream) -> syn::Result { - Ok(Self { - key_span: Span::call_site(), - path: input.parse::>()?.value, - }) - } -} diff --git a/src/tests/test_macro.rs b/src/tests/test_macro.rs index 9585d471..c429f0ff 100644 --- a/src/tests/test_macro.rs +++ b/src/tests/test_macro.rs @@ -68,7 +68,6 @@ pub fn test_body( crate_name: &str, manifest_path: &str, plugin_name: &str, - library_path: Option>, extra_cmd: Option<&str>, ) -> Result<(), String> { panic::set_hook(Box::new(move |info| { @@ -87,15 +86,10 @@ pub fn test_body( eprintln!("{}", info); })); - let output = run_nvim_command( - crate_name, - manifest_path, - plugin_name, - library_path, - extra_cmd, - )? - .output() - .map_err(|err| err.to_string())?; + let output = + run_nvim_command(crate_name, manifest_path, plugin_name, extra_cmd)? + .output() + .map_err(|err| err.to_string())?; let stdout = String::from_utf8_lossy(&output.stdout); let stdout = stdout.trim(); @@ -152,7 +146,6 @@ fn run_nvim_command( crate_name: &str, manifest_path: &str, plugin_name: &str, - library_path: Option>, extra_cmd: Option<&str>, ) -> Result { let manifest = super::build::CargoManifest::from_path(manifest_path) @@ -163,16 +156,13 @@ fn run_nvim_command( let profile = env::var(manifest.profile_env()).map_err(|err| err.to_string())?; - let library_path = library_path - .map(|path| path.as_ref().to_owned()) - .unwrap_or_else(|| { - let library_name = format!( - "{prefix}{crate_name}{suffix}", - prefix = env::consts::DLL_PREFIX, - suffix = env::consts::DLL_SUFFIX, - ); - target_dir.join(profile).join(library_name) - }); + let library_name = format!( + "{prefix}{crate_name}{suffix}", + prefix = env::consts::DLL_PREFIX, + suffix = env::consts::DLL_SUFFIX, + ); + + let library_path = target_dir.join(profile).join(library_name); if !library_path.exists() { return Err(format!( From 9d144407faa7ed0833b6872b162c3e1c26295584 Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 02:56:16 +0800 Subject: [PATCH 8/9] Add docs to `build()` and `BuildError`. --- src/tests/build.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/tests/build.rs b/src/tests/build.rs index 2ef450d5..68f8a566 100644 --- a/src/tests/build.rs +++ b/src/tests/build.rs @@ -4,7 +4,36 @@ use std::path::Path; use std::process::Command; use std::{env, io}; -/// TODO: docs. +/// Builds the library required to run integration tests within Neovim. +/// +/// This function is designed to be used in the build script (`build.rs`) of a +/// crate containing integration tests for Neovim plugins. It is tightly +/// coupled with the [`test`](crate::test) macro, which is used to annotate the +/// functions to test. +/// +/// Together, they enable a workflow where the build script compiles the test +/// crate into a dynamic library, and the macro generates functions that load +/// this library into Neovim and execute the tests within it. +/// +/// # Usage +/// +/// Add the following to your test crate's `build.rs`: +/// +/// ```ignore +/// fn main() -> Result<(), nvim_oxi::tests::BuildError> { +/// nvim_oxi::tests::build() +/// } +/// ``` +/// +/// # Notes +/// +/// Like the plugin crate, the test crate must also be configured to be built +/// as a dynamic library by including the following in its Cargo.toml: +/// +/// ```toml +/// [lib] +/// crate-type = ["cdylib"] +/// ``` pub fn build() -> Result<(), BuildError> { let Some(_g) = BuildGuard::::new()? else { return Ok(()) }; let compilation_opts = CompilationOpts::from_env()?; @@ -20,7 +49,7 @@ pub fn build() -> Result<(), BuildError> { Ok(()) } -/// TODO: docs. +/// An opaque error returned when [`build`]ing a test crate fails. #[derive(Debug, thiserror::Error)] #[error(transparent)] pub struct BuildError { From bb77c73864ae266484238072ca19f617ffe6f6ea Mon Sep 17 00:00:00 2001 From: Riccardo Mazzarini Date: Sun, 22 Dec 2024 03:26:52 +0800 Subject: [PATCH 9/9] Update the `README` and `CHANGELOG` --- CHANGELOG.md | 4 ++++ README.md | 31 +++++++++++++++++++------------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae2825db..f83e9cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ - a `StringBuilder` struct that can be used to incrementally build `nvim_oxi::String`s; +- a `nvim_oxi::tests::build()` function to be used in the build script of + a crate containing integration tests annotated with `#[nvim_oxi::test]` + ([#201](https://github.com/noib3/nvim-oxi/pull/201)); + ### Changed - `nvim_oxi::api::echo` is now generic over the highlight group type instead of diff --git a/README.md b/README.md index 0b6c3d38..ca687a5a 100644 --- a/README.md +++ b/README.md @@ -60,16 +60,16 @@ might add a new example documenting your use case (if it can be done). ## Testing -The `test` feature flag enables the `#[nvim_oxi::test]` proc macro. This macro -replaces the regular `#[test]` annotations and can be used to test a piece of -code from within a Neovim instance using Rust's excellent testing framework. +Turning on the `test` feature enables `#[nvim_oxi::test]`, which replaces the +regular `#[test]` macro and allows you to test a piece of code from within a +Neovim instance using Rust's testing framework. For example: ```rust -use nvim_oxi::{self as oxi, api}; +use nvim_oxi::api; -#[oxi::test] +#[nvim_oxi::test] fn set_get_del_var() { api::set_var("foo", 42).unwrap(); assert_eq!(Ok(42), api::get_var("foo")); @@ -77,14 +77,11 @@ fn set_get_del_var() { } ``` -Then `cargo test` will spawn a new Neovim process with an empty config, run -that code and exit. There are a couple of gotchas: +When `cargo test` is executed, the generated code will spawn a new Neovim +process with the `nvim` binary in your `$PATH`, test your code, and exit. -- after changing a piece of code, `cargo build` has to be run before you can - test that with `cargo test`; - -- you cannot have two test functions with the same name, even if they belong to - different modules. For example this won't work: +There's a gotcha: you can't have two tests with the same name in the same +crate, even if they belong to different modules. For example, this won't work: ```rust mod a { @@ -97,3 +94,13 @@ mod b { fn foo() {} } ``` + +Note that all integration tests must live inside a separate `cdylib` crate with +the following build script: + +```rust +// build.rs +fn main() -> Result<(), nvim_oxi::tests::BuildError> { + nvim_oxi::tests::build() +} +```