diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46bd3d00b..2de61aa68 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,7 +79,12 @@ jobs: RUST_BACKTRACE: "1" FEATURES: ${{needs.get-features.outputs.openssl-features}} run: | - cargo llvm-cov --lib --features "$FEATURES" --lcov --output-path lcov-openssl.info + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cargo llvm-cov -p c2pa --features "$FEATURES" --no-default-features --lib --lcov --output-path lcov-openssl-reader-builder-settings.info -- --test-threads=1 reader::tests builder::tests settings:: │ + # Run all other tests multi-threaded for better performance │ + cargo llvm-cov -p c2pa --features "$FEATURES" --no-default-features --lib --lcov --output-path lcov-openssl-other.info -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" │ + # Combine coverage reports │ + lcov --add-tracefile lcov-openssl-reader-builder-settings.info --add-tracefile lcov-openssl-other.info --output-file lcov-openssl.info │ # Tokens aren't available for PRs originating from forks, # so we don't attempt to upload code coverage in that case. @@ -128,7 +133,12 @@ jobs: RUST_BACKTRACE: "1" FEATURES: ${{needs.get-features.outputs.rust-native-features}} run: | - cargo llvm-cov -p c2pa --no-default-features --features "$FEATURES" --lcov --output-path lcov-rust_native_crypto.info + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cargo llvm-cov -p c2pa --no-default-features --features "$FEATURES" --lib --lcov --output-path lcov-rust_native_crypto-reader-builder-settings.info -- --test-threads=1 reader::tests builder::tests settings:: + # Run all other tests multi-threaded for better performance + cargo llvm-cov -p c2pa --no-default-features --features "$FEATURES" --lib --lcov --output-path lcov-rust_native_crypto-other.info -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" + # Combine coverage reports + lcov --add-tracefile lcov-rust_native_crypto-reader-builder-settings.info --add-tracefile lcov-rust_native_crypto-other.info --output-file lcov-rust_native_crypto.info # Tokens aren't available for PRs originating from forks, # so we don't attempt to upload code coverage in that case. diff --git a/.github/workflows/release_readiness.yml b/.github/workflows/release_readiness.yml index b7696b277..83a3b0989 100644 --- a/.github/workflows/release_readiness.yml +++ b/.github/workflows/release_readiness.yml @@ -69,7 +69,10 @@ jobs: RUST_BACKTRACE: "1" FEATURES: ${{needs.get-features.outputs.openssl-features}} run: | - cargo test --features "$FEATURES" + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cargo test -p c2pa --features "$FEATURES" --no-default-features --lib -- --test-threads=1 reader::tests builder::tests settings:: + # Run all other tests multi-threaded for better performance + cargo test -p c2pa --features "$FEATURES" --no-default-features --lib -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" tests-rust-native-crypto: name: Unit tests (with Rust native crypto installed) @@ -100,7 +103,10 @@ jobs: RUST_BACKTRACE: "1" FEATURES: ${{needs.get-features.outputs.rust_native_crypto-features}} run: | - cargo test --features "$FEATURES" + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cargo test -p c2pa --features "$FEATURES" --no-default-features --lib -- --test-threads=1 reader::tests builder::tests settings:: + # Run all other tests multi-threaded for better performance + cargo test -p c2pa --features "$FEATURES" --no-default-features --lib -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" tests-cross: name: Unit tests @@ -134,7 +140,10 @@ jobs: env: FEATURES: ${{needs.get-features.outputs.openssl-features}} run: | - cross test --all-targets --features "$FEATURES" --target ${{ matrix.target }} + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cross test --all-targets --features "$FEATURES" --target ${{ matrix.target }} -- --test-threads=1 reader::tests builder::tests settings:: + # Run all other tests multi-threaded for better performance + cross test --all-targets --features "$FEATURES" --target ${{ matrix.target }} -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" test-direct-minimal-versions: name: Unit tests with minimum versions of direct dependencies @@ -163,7 +172,10 @@ jobs: env: FEATURES: ${{needs.get-features.outputs.openssl-features}} run: | - cargo +nightly-2025-07-28 test -Z direct-minimal-versions --all-targets --features "$FEATURES" + # Run reader, builder, and settings tests single-threaded to avoid global state issues + cargo +nightly-2025-07-28 test -p c2pa -Z direct-minimal-versions --all-targets --features "$FEATURES" -- --test-threads=1 reader::tests builder::tests settings:: + # Run all other tests multi-threaded for better performance + cargo +nightly-2025-07-28 test -p c2pa -Z direct-minimal-versions --all-targets --features "$FEATURES" -- --skip "reader::tests" --skip "builder::tests" --skip "settings::" docs_rs: name: Preflight docs.rs build diff --git a/sdk/src/builder.rs b/sdk/src/builder.rs index 98d6176d6..e4523a53f 100644 --- a/sdk/src/builder.rs +++ b/sdk/src/builder.rs @@ -1955,7 +1955,6 @@ mod tests { // Source: https://github.com/contentauth/c2pa-rs/pull/1458 #[test] fn test_builder_one_placed_action_via_ingredient_id_ref() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -2027,7 +2026,6 @@ mod tests { #[test] fn test_builder_settings_auto_created() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -2065,7 +2063,6 @@ mod tests { #[test] fn test_builder_settings_auto_opened() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); let mut builder = Builder::new(); @@ -2121,7 +2118,6 @@ mod tests { #[test] fn test_builder_settings_auto_placed() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -2217,7 +2213,6 @@ mod tests { #[test] fn test_builder_settings_all_actions_included() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -2257,7 +2252,6 @@ mod tests { #[test] fn test_builder_settings_action_templates() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -2318,7 +2312,6 @@ mod tests { #[test] fn test_builder_settings_actions() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( diff --git a/sdk/src/reader.rs b/sdk/src/reader.rs index 16c4adf33..d1490a7da 100644 --- a/sdk/src/reader.rs +++ b/sdk/src/reader.rs @@ -1020,8 +1020,9 @@ pub mod tests { } #[test] - #[cfg(not(target_os = "wasi"))] // todo: enable when disable we find out wasi trust issues fn test_reader_trusted() -> Result<()> { + crate::settings::reset_default_settings()?; + let reader = Reader::from_stream("image/jpeg", std::io::Cursor::new(IMAGE_COMPLEX_MANIFEST))?; assert_eq!(reader.validation_state(), ValidationState::Trusted); diff --git a/sdk/src/settings/mod.rs b/sdk/src/settings/mod.rs index 8253ba8df..30fb501ed 100644 --- a/sdk/src/settings/mod.rs +++ b/sdk/src/settings/mod.rs @@ -19,11 +19,12 @@ pub mod signer; #[cfg(feature = "file_io")] use std::path::Path; use std::{ - cell::RefCell, io::{BufRead, BufReader, Cursor}, + sync::RwLock, }; use config::{Config, FileFormat}; +use lazy_static::lazy_static; use serde_derive::{Deserialize, Serialize}; use signer::SignerSettings; @@ -31,10 +32,10 @@ use crate::{crypto::base64, settings::builder::BuilderSettings, Error, Result, S const VERSION: u32 = 1; -thread_local!( - static SETTINGS: RefCell = - RefCell::new(Config::try_from(&Settings::default()).unwrap_or_default()); -); +lazy_static! { + static ref SETTINGS: RwLock = + RwLock::new(Config::try_from(&Settings::default()).unwrap_or_default()); +} // trait used to validate user input to make sure user supplied configurations are valid pub(crate) trait SettingsValidate { @@ -401,12 +402,15 @@ impl Settings { .build() .map_err(|_e| Error::BadParam("could not parse configuration file".into()))?; - let update_config = SETTINGS.with_borrow(|current_settings| { - Config::builder() - .add_source(current_settings.clone()) - .add_source(new_config) - .build() // merge overrides, allows for partial changes - }); + let update_config = match SETTINGS.read() { + Ok(current_settings) => { + Config::builder() + .add_source(current_settings.clone()) + .add_source(new_config) + .build() // merge overrides, allows for partial changes + } + Err(_) => return Err(Error::OtherError("could not read settings".into())), + }; match update_config { Ok(update_config) => { @@ -418,7 +422,10 @@ impl Settings { settings.validate()?; - SETTINGS.set(update_config.clone()); + match SETTINGS.write() { + Ok(mut settings) => *settings = update_config.clone(), + Err(_) => return Err(Error::OtherError("could not write settings".into())), + } Ok(settings) } @@ -451,7 +458,10 @@ impl Settings { /// deep based on the [Settings] definition. #[allow(unused)] pub(crate) fn set_value>(value_path: &str, value: T) -> Result<()> { - let c = SETTINGS.take(); + let c = match SETTINGS.read() { + Ok(config) => config.clone(), + Err(_) => return Err(Error::OtherError("could not read settings".into())), + }; let update_config = Config::builder() .add_source(c.clone()) @@ -468,11 +478,17 @@ impl Settings { .map_err(|e| Error::BadParam(e.to_string()))?; settings.validate()?; - SETTINGS.set(update_config); + match SETTINGS.write() { + Ok(mut settings) => *settings = update_config, + Err(_) => return Err(Error::OtherError("could not write settings".into())), + } Ok(()) } else { - SETTINGS.set(c); + match SETTINGS.write() { + Ok(mut settings) => *settings = c, + Err(_) => return Err(Error::OtherError("could not write settings".into())), + } Err(Error::OtherError("could not save settings".into())) } } @@ -484,23 +500,29 @@ impl Settings { /// deep based on the [Settings] definition. #[allow(unused)] fn get_value<'de, T: serde::de::Deserialize<'de>>(value_path: &str) -> Result { - SETTINGS.with_borrow(|current_settings| { - let update_config = Config::builder() - .add_source(current_settings.clone()) - .build() - .map_err(|_e| Error::OtherError("could not update configuration".into()))?; - - update_config - .get::(value_path) - .map_err(|_| Error::NotFound) - }) + match SETTINGS.read() { + Ok(current_settings) => { + let update_config = Config::builder() + .add_source(current_settings.clone()) + .build() + .map_err(|_e| Error::OtherError("could not update configuration".into()))?; + + update_config + .get::(value_path) + .map_err(|_| Error::NotFound) + } + Err(_) => Err(Error::OtherError("could not read settings".into())), + } } /// Set [Settings] back to the default values. #[allow(unused)] pub(crate) fn reset() -> Result<()> { if let Ok(default_settings) = Config::try_from(&Settings::default()) { - SETTINGS.set(default_settings); + match SETTINGS.write() { + Ok(mut settings) => *settings = default_settings, + Err(_) => return Err(Error::OtherError("could not write settings".into())), + } Ok(()) } else { Err(Error::OtherError("could not save settings".into())) @@ -568,7 +590,10 @@ impl SettingsValidate for Settings { // Get snapshot of the Settings objects, returns None if there is an error #[allow(unused)] pub(crate) fn get_settings() -> Option { - SETTINGS.with_borrow(|config| config.clone().try_deserialize::().ok()) + match SETTINGS.try_read() { + Ok(config) => config.clone().try_deserialize::().ok(), + Err(_) => None, + } } // Load settings from configuration file @@ -884,10 +909,7 @@ pub mod tests { #[test] fn test_load_settings_from_sample_toml() { - #[cfg(target_os = "wasi")] - Settings::reset().unwrap(); - - let toml = include_bytes!("../../examples/c2pa.toml"); - Settings::from_toml(std::str::from_utf8(toml).unwrap()).unwrap(); + let toml_bytes = include_bytes!("../../examples/c2pa.toml"); + Settings::from_toml(std::str::from_utf8(toml_bytes).unwrap()).unwrap(); } } diff --git a/sdk/src/settings/signer.rs b/sdk/src/settings/signer.rs index 4a83059bf..195d7b4f7 100644 --- a/sdk/src/settings/signer.rs +++ b/sdk/src/settings/signer.rs @@ -300,7 +300,6 @@ pub mod tests { #[test] fn test_make_local_signer() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); // Testing with a different alg than the default test signer. @@ -330,7 +329,6 @@ pub mod tests { use crate::create_signer; - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); let alg = SigningAlg::Ps384; diff --git a/sdk/src/utils/thumbnail.rs b/sdk/src/utils/thumbnail.rs index 7f19d1df0..c7fa22c33 100644 --- a/sdk/src/utils/thumbnail.rs +++ b/sdk/src/utils/thumbnail.rs @@ -407,7 +407,6 @@ pub mod tests { #[test] fn test_make_thumbnail_bytes_from_stream() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -437,7 +436,6 @@ pub mod tests { #[test] fn test_make_thumbnail_with_prefer_smallest_format() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml( @@ -504,7 +502,6 @@ pub mod tests { #[test] fn test_make_thumbnail_and_ignore_errors() { - #[cfg(target_os = "wasi")] Settings::reset().unwrap(); Settings::from_toml(