From de66328c70b239e2d51016db769667040871a3f2 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Fri, 12 Apr 2024 13:23:42 -0400 Subject: [PATCH] =?UTF-8?q?tests:=20=F0=9F=A4=9D=20hoist=20test=20subscrib?= =?UTF-8?q?er=20into=20a=20standalone=20library=20(#4200)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit in #4193, @zbuc is working on some changes to the dex component. in the `penumbra-app` crate, we have a helpful utility to define a tracing subscriber that correctly interacts with `libtest`'s output capturing, and can be configured via `RUST_LOG` when running tests. this is a tremendously helpful aid in debugging issues during feature development, and is not specific to the `penumbra-app` crate. this commit hoists that test utility into a standalone library in `crates/test/`, so that other components like the dex can also create a guard to capture traces in tests. now we can share this, by doing this: ```rust #[tokio::test] async fn example_test_case() -> anyhow::Result<()> { // Install a test logger... let guard = set_tracing_subscriber(); // Test logic here... drop(guard); Ok(()) } ``` #### checklist before requesting a review - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > only changes test code --- Cargo.lock | 9 +++++++++ Cargo.toml | 2 ++ crates/core/app/Cargo.toml | 17 +++++++++-------- crates/core/app/tests/common/mod.rs | 15 ++++++--------- crates/test/tracing-subscriber/Cargo.toml | 13 +++++++++++++ .../tracing-subscriber/src/lib.rs} | 5 +++++ 6 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 crates/test/tracing-subscriber/Cargo.toml rename crates/{core/app/tests/common/tracing_subscriber.rs => test/tracing-subscriber/src/lib.rs} (85%) diff --git a/Cargo.lock b/Cargo.lock index 748816d82e..7f1e79d1ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4652,6 +4652,7 @@ dependencies = [ "penumbra-stake", "penumbra-tct", "penumbra-tendermint-proxy", + "penumbra-test-subscriber", "penumbra-tower-trace", "penumbra-transaction", "penumbra-txhash", @@ -5577,6 +5578,14 @@ dependencies = [ "url", ] +[[package]] +name = "penumbra-test-subscriber" +version = "0.71.0" +dependencies = [ + "tracing", + "tracing-subscriber 0.3.18", +] + [[package]] name = "penumbra-tower-trace" version = "0.71.0" diff --git a/Cargo.toml b/Cargo.toml index a7dc78e149..2c84b5b206 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ members = [ "crates/test/mock-client", "crates/test/mock-consensus", "crates/test/tct-property-test", + "crates/test/tracing-subscriber", "crates/util/auto-https", "crates/util/tendermint-proxy", "crates/util/tower-trace", @@ -179,6 +180,7 @@ penumbra-sct = { default-features = false, path = "crates/co penumbra-shielded-pool = { default-features = false, path = "crates/core/component/shielded-pool" } penumbra-stake = { default-features = false, path = "crates/core/component/stake" } penumbra-tct = { default-features = false, path = "crates/crypto/tct" } +penumbra-test-subscriber = { path = "crates/test/tracing-subscriber" } penumbra-transaction = { default-features = false, path = "crates/core/transaction" } penumbra-txhash = { default-features = false, path = "crates/core/txhash" } penumbra-view = { path = "crates/view" } diff --git a/crates/core/app/Cargo.toml b/crates/core/app/Cargo.toml index 8c04f1d87f..3210cde095 100644 --- a/crates/core/app/Cargo.toml +++ b/crates/core/app/Cargo.toml @@ -81,14 +81,15 @@ tracing = { workspace = true } url = { workspace = true } [dev-dependencies] -ed25519-consensus = { workspace = true } -penumbra-mock-consensus = { workspace = true } -penumbra-mock-client = { workspace = true } -rand = { workspace = true } -rand_core = { workspace = true } -rand_chacha = { workspace = true } -tap = { workspace = true } -tracing-subscriber = { workspace = true } +ed25519-consensus = { workspace = true } +penumbra-mock-consensus = { workspace = true } +penumbra-mock-client = { workspace = true } +penumbra-test-subscriber = { workspace = true } +rand = { workspace = true } +rand_core = { workspace = true } +rand_chacha = { workspace = true } +tap = { workspace = true } +tracing-subscriber = { workspace = true } # Enable the feature flags to get proving keys when running tests. [dev-dependencies.penumbra-proof-params] diff --git a/crates/core/app/tests/common/mod.rs b/crates/core/app/tests/common/mod.rs index b1b00df1a8..1a6e788508 100644 --- a/crates/core/app/tests/common/mod.rs +++ b/crates/core/app/tests/common/mod.rs @@ -2,9 +2,12 @@ // NB: these reëxports are shared and consumed by files in `tests/`. #[allow(unused_imports)] -pub use self::{ - temp_storage_ext::TempStorageExt, test_node_builder_ext::BuilderExt, - test_node_ext::TestNodeExt, tracing_subscriber::set_tracing_subscriber, +pub use { + self::{ + temp_storage_ext::TempStorageExt, test_node_builder_ext::BuilderExt, + test_node_ext::TestNodeExt, + }, + penumbra_test_subscriber::set_tracing_subscriber, }; /// Penumbra-specific extensions to the mock consensus builder. @@ -19,9 +22,3 @@ mod temp_storage_ext; /// /// See [`TestNodeExt`]. mod test_node_ext; - -/// A pretty [`tracing`] subscriber for use in test cases. -/// -/// NB: this subscriber makes use of a test writer, that is compatible with `cargo test`'s output -/// capturing. -mod tracing_subscriber; diff --git a/crates/test/tracing-subscriber/Cargo.toml b/crates/test/tracing-subscriber/Cargo.toml new file mode 100644 index 0000000000..f4be0e6d37 --- /dev/null +++ b/crates/test/tracing-subscriber/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "penumbra-test-subscriber" +authors.workspace = true +edition.workspace = true +version.workspace = true +repository.workspace = true +homepage.workspace = true +license.workspace = true +publish = false + +[dependencies] +tracing = { workspace = true } +tracing-subscriber = { workspace = true } diff --git a/crates/core/app/tests/common/tracing_subscriber.rs b/crates/test/tracing-subscriber/src/lib.rs similarity index 85% rename from crates/core/app/tests/common/tracing_subscriber.rs rename to crates/test/tracing-subscriber/src/lib.rs index 52597956d7..3e48b80c0a 100644 --- a/crates/core/app/tests/common/tracing_subscriber.rs +++ b/crates/test/tracing-subscriber/src/lib.rs @@ -1,3 +1,8 @@ +//! A pretty [`tracing`] subscriber for use in test cases. +//! +//! NB: this subscriber makes use of a test writer, that is compatible with `cargo test`'s output +//! capturing. + use { tracing::subscriber::{set_default, DefaultGuard}, tracing_subscriber::{filter::EnvFilter, fmt},