From cb9d3f250698646dd306805113865fa3bb7e6ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Sat, 22 Jul 2023 10:50:22 -0300 Subject: [PATCH 01/10] pvf: use test-utils feature to export test only --- node/core/pvf/Cargo.toml | 3 +++ node/core/pvf/common/Cargo.toml | 3 +++ node/core/pvf/common/src/lib.rs | 2 +- node/core/pvf/common/src/pvf.rs | 6 +++--- node/core/pvf/src/lib.rs | 2 +- node/core/pvf/src/testing.rs | 3 +++ node/core/pvf/tests/it/worker_common.rs | 6 ++++-- parachain/test-parachains/adder/collator/Cargo.toml | 13 ++++++++----- .../test-parachains/undying/collator/Cargo.toml | 13 ++++++++----- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index 658a26cff09d..c2b97ec658d9 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -7,6 +7,7 @@ edition.workspace = true [[bin]] name = "puppet_worker" path = "bin/puppet_worker.rs" +required-features = ["test-utils"] [dependencies] always-assert = "0.1" @@ -42,9 +43,11 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [dev-dependencies] assert_matches = "1.4.0" hex-literal = "0.3.4" +polkadot-node-core-pvf-common = { path = "common", features = ["test-utils"] } adder = { package = "test-parachain-adder", path = "../../../parachain/test-parachains/adder" } halt = { package = "test-parachain-halt", path = "../../../parachain/test-parachains/halt" } [features] ci-only-tests = [] +test-utils = [] diff --git a/node/core/pvf/common/Cargo.toml b/node/core/pvf/common/Cargo.toml index 52a0fb37c569..cba0bfae8635 100644 --- a/node/core/pvf/common/Cargo.toml +++ b/node/core/pvf/common/Cargo.toml @@ -34,3 +34,6 @@ tempfile = "3.3.0" [build-dependencies] substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } + +[features] +test-utils = [] \ No newline at end of file diff --git a/node/core/pvf/common/src/lib.rs b/node/core/pvf/common/src/lib.rs index 028fd9b17947..5da225b84849 100644 --- a/node/core/pvf/common/src/lib.rs +++ b/node/core/pvf/common/src/lib.rs @@ -30,7 +30,7 @@ const LOG_TARGET: &str = "parachain::pvf-common"; use std::mem; use tokio::io::{self, AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _}; -#[doc(hidden)] +#[cfg(feature = "test-utils")] pub mod tests { use std::time::Duration; diff --git a/node/core/pvf/common/src/pvf.rs b/node/core/pvf/common/src/pvf.rs index ab0007352d1d..e31264713a57 100644 --- a/node/core/pvf/common/src/pvf.rs +++ b/node/core/pvf/common/src/pvf.rs @@ -84,7 +84,7 @@ impl PvfPrepData { } /// Creates a structure for tests. - #[doc(hidden)] + #[cfg(feature = "test-utils")] pub fn from_discriminator_and_timeout(num: u32, timeout: Duration) -> Self { let descriminator_buf = num.to_le_bytes().to_vec(); Self::from_code( @@ -96,13 +96,13 @@ impl PvfPrepData { } /// Creates a structure for tests. - #[doc(hidden)] + #[cfg(feature = "test-utils")] pub fn from_discriminator(num: u32) -> Self { Self::from_discriminator_and_timeout(num, crate::tests::TEST_PREPARATION_TIMEOUT) } /// Creates a structure for tests. - #[doc(hidden)] + #[cfg(feature = "test-utils")] pub fn from_discriminator_precheck(num: u32) -> Self { let mut pvf = Self::from_discriminator_and_timeout(num, crate::tests::TEST_PREPARATION_TIMEOUT); diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index 8696119f801c..963019f2b4fa 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -101,7 +101,7 @@ mod worker_intf; pub mod testing; // Used by `decl_puppet_worker_main!`. -#[doc(hidden)] +#[cfg(feature = "test-utils")] pub use sp_tracing; pub use error::{InvalidCandidate, ValidationError}; diff --git a/node/core/pvf/src/testing.rs b/node/core/pvf/src/testing.rs index cc07d7aeef02..18907275b8c8 100644 --- a/node/core/pvf/src/testing.rs +++ b/node/core/pvf/src/testing.rs @@ -22,10 +22,12 @@ #[doc(hidden)] pub use crate::worker_intf::{spawn_with_program_path, SpawnErr}; +#[cfg(feature = "test-utils")] use polkadot_primitives::ExecutorParams; /// A function that emulates the stitches together behaviors of the preparation and the execution /// worker in a single synchronous function. +#[cfg(feature = "test-utils")] pub fn validate_candidate( code: &[u8], params: &[u8], @@ -52,6 +54,7 @@ pub fn validate_candidate( /// Use this macro to declare a `fn main() {}` that will check the arguments and dispatch them to /// the appropriate worker, making the executable that can be used for spawning workers. #[macro_export] +#[cfg(feature = "test-utils")] macro_rules! decl_puppet_worker_main { () => { fn main() { diff --git a/node/core/pvf/tests/it/worker_common.rs b/node/core/pvf/tests/it/worker_common.rs index 439ac8538c95..a3bf552e894a 100644 --- a/node/core/pvf/tests/it/worker_common.rs +++ b/node/core/pvf/tests/it/worker_common.rs @@ -14,10 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use crate::PUPPET_EXE; -use polkadot_node_core_pvf::testing::{spawn_with_program_path, SpawnErr}; use std::time::Duration; +use polkadot_node_core_pvf::testing::{spawn_with_program_path, SpawnErr}; + +use crate::PUPPET_EXE; + // Test spawning a program that immediately exits with a failure code. #[tokio::test] async fn spawn_immediate_exit() { diff --git a/parachain/test-parachains/adder/collator/Cargo.toml b/parachain/test-parachains/adder/collator/Cargo.toml index 29a10069e3e0..954242acfaf9 100644 --- a/parachain/test-parachains/adder/collator/Cargo.toml +++ b/parachain/test-parachains/adder/collator/Cargo.toml @@ -12,6 +12,7 @@ path = "src/main.rs" [[bin]] name = "adder_collator_puppet_worker" path = "bin/puppet_worker.rs" +required-features = ["test-utils"] [dependencies] parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] } @@ -31,17 +32,19 @@ sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } -# This one is tricky. Even though it is not used directly by the collator, we still need it for the -# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be -# a big problem since it is used transitively anyway. -polkadot-node-core-pvf = { path = "../../../../node/core/pvf" } - [dev-dependencies] polkadot-parachain = { path = "../../.." } polkadot-test-service = { path = "../../../../node/test/service" } +# This one is tricky. Even though it is not used directly by the collator, we still need it for the +# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be +# a big problem since it is used transitively anyway. +polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"] } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } tokio = { version = "1.24.2", features = ["macros"] } + +[features] +test-utils = ["polkadot-node-core-pvf/test-utils"] diff --git a/parachain/test-parachains/undying/collator/Cargo.toml b/parachain/test-parachains/undying/collator/Cargo.toml index f63757a20958..16525f6baafa 100644 --- a/parachain/test-parachains/undying/collator/Cargo.toml +++ b/parachain/test-parachains/undying/collator/Cargo.toml @@ -12,6 +12,7 @@ path = "src/main.rs" [[bin]] name = "undying_collator_puppet_worker" path = "bin/puppet_worker.rs" +required-features = ["test-utils"] [dependencies] parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] } @@ -31,17 +32,19 @@ sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } -# This one is tricky. Even though it is not used directly by the collator, we still need it for the -# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be -# a big problem since it is used transitively anyway. -polkadot-node-core-pvf = { path = "../../../../node/core/pvf" } - [dev-dependencies] polkadot-parachain = { path = "../../.." } polkadot-test-service = { path = "../../../../node/test/service" } +# This one is tricky. Even though it is not used directly by the collator, we still need it for the +# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be +# a big problem since it is used transitively anyway. +polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"] } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } tokio = { version = "1.24.2", features = ["macros"] } + +[features] +test-utils = ["polkadot-node-core-pvf/test-utils"] From b72c655f9323004d8da523f6f4690204172e9b83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Tue, 25 Jul 2023 15:50:42 -0300 Subject: [PATCH 02/10] adding comment to test-utils feature --- node/core/pvf/Cargo.toml | 1 + node/core/pvf/common/Cargo.toml | 1 + parachain/test-parachains/adder/collator/Cargo.toml | 2 ++ parachain/test-parachains/undying/collator/Cargo.toml | 2 ++ 4 files changed, 6 insertions(+) diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index c2b97ec658d9..7c43ae6160b4 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -50,4 +50,5 @@ halt = { package = "test-parachain-halt", path = "../../../parachain/test-parach [features] ci-only-tests = [] +# This feature is used to export test code to other crates without putting it in the production build. test-utils = [] diff --git a/node/core/pvf/common/Cargo.toml b/node/core/pvf/common/Cargo.toml index cba0bfae8635..5e43eeb81bbb 100644 --- a/node/core/pvf/common/Cargo.toml +++ b/node/core/pvf/common/Cargo.toml @@ -36,4 +36,5 @@ tempfile = "3.3.0" substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } [features] +# This feature is used to export test code to other crates without putting it in the production build. test-utils = [] \ No newline at end of file diff --git a/parachain/test-parachains/adder/collator/Cargo.toml b/parachain/test-parachains/adder/collator/Cargo.toml index 954242acfaf9..a158006375ae 100644 --- a/parachain/test-parachains/adder/collator/Cargo.toml +++ b/parachain/test-parachains/adder/collator/Cargo.toml @@ -47,4 +47,6 @@ sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master tokio = { version = "1.24.2", features = ["macros"] } [features] +# This feature is used to export test code to other crates without putting it in the production build. +# This is also used by the `puppet_worker` binary. test-utils = ["polkadot-node-core-pvf/test-utils"] diff --git a/parachain/test-parachains/undying/collator/Cargo.toml b/parachain/test-parachains/undying/collator/Cargo.toml index 16525f6baafa..6e27d81f4e03 100644 --- a/parachain/test-parachains/undying/collator/Cargo.toml +++ b/parachain/test-parachains/undying/collator/Cargo.toml @@ -47,4 +47,6 @@ sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master tokio = { version = "1.24.2", features = ["macros"] } [features] +# This feature is used to export test code to other crates without putting it in the production build. +# This is also used by the `puppet_worker` binary. test-utils = ["polkadot-node-core-pvf/test-utils"] From db467ff78579be636ef6d47780b9c8a8caf04628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 2 Aug 2023 08:24:40 -0300 Subject: [PATCH 03/10] make prepare-worker and execute-worker as optional dependencies and add comments to test-utils --- Cargo.lock | 3 +++ node/core/pvf/Cargo.toml | 8 +++++--- node/core/pvf/common/Cargo.toml | 1 + node/core/pvf/src/lib.rs | 2 +- node/core/pvf/src/testing.rs | 3 --- parachain/test-parachains/adder/collator/Cargo.toml | 9 +++++---- parachain/test-parachains/undying/collator/Cargo.toml | 9 +++++---- 7 files changed, 20 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc12858193d2..bc4be056bc58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7378,6 +7378,7 @@ dependencies = [ "parity-scale-codec", "pin-project", "polkadot-core-primitives", + "polkadot-node-core-pvf", "polkadot-node-core-pvf-common", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker", @@ -12459,6 +12460,7 @@ dependencies = [ "sp-keyring", "substrate-test-utils", "test-parachain-adder", + "test-parachain-adder-collator", "tokio", ] @@ -12507,6 +12509,7 @@ dependencies = [ "sp-keyring", "substrate-test-utils", "test-parachain-undying", + "test-parachain-undying-collator", "tokio", ] diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index 7c43ae6160b4..31d3e829781e 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -26,8 +26,6 @@ parity-scale-codec = { version = "3.6.1", default-features = false, features = [ polkadot-parachain = { path = "../../../parachain" } polkadot-core-primitives = { path = "../../../core-primitives" } polkadot-node-core-pvf-common = { path = "common" } -polkadot-node-core-pvf-execute-worker = { path = "execute-worker" } -polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker" } polkadot-node-metrics = { path = "../../metrics" } polkadot-node-primitives = { path = "../../primitives" } polkadot-primitives = { path = "../../../primitives" } @@ -36,6 +34,8 @@ sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-wasm-interface = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-maybe-compressed-blob = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" } +polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker", optional = true } +polkadot-node-core-pvf-execute-worker = { path = "execute-worker", optional = true } [build-dependencies] substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } @@ -44,6 +44,7 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" assert_matches = "1.4.0" hex-literal = "0.3.4" polkadot-node-core-pvf-common = { path = "common", features = ["test-utils"] } +polkadot-node-core-pvf = { path = ".", features = ["test-utils"] } adder = { package = "test-parachain-adder", path = "../../../parachain/test-parachains/adder" } halt = { package = "test-parachain-halt", path = "../../../parachain/test-parachains/halt" } @@ -51,4 +52,5 @@ halt = { package = "test-parachain-halt", path = "../../../parachain/test-parach [features] ci-only-tests = [] # This feature is used to export test code to other crates without putting it in the production build. -test-utils = [] +# This is also used by the `puppet_worker` binary. +test-utils = ["polkadot-node-core-pvf-prepare-worker", "polkadot-node-core-pvf-execute-worker"] diff --git a/node/core/pvf/common/Cargo.toml b/node/core/pvf/common/Cargo.toml index 5e43eeb81bbb..4b5251cb5487 100644 --- a/node/core/pvf/common/Cargo.toml +++ b/node/core/pvf/common/Cargo.toml @@ -37,4 +37,5 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [features] # This feature is used to export test code to other crates without putting it in the production build. +# Also used for building the puppet worker. test-utils = [] \ No newline at end of file diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index 963019f2b4fa..fa65abeba865 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -97,7 +97,7 @@ mod prepare; mod priority; mod worker_intf; -#[doc(hidden)] +#[cfg(feature = "test-utils")] pub mod testing; // Used by `decl_puppet_worker_main!`. diff --git a/node/core/pvf/src/testing.rs b/node/core/pvf/src/testing.rs index 18907275b8c8..cc07d7aeef02 100644 --- a/node/core/pvf/src/testing.rs +++ b/node/core/pvf/src/testing.rs @@ -22,12 +22,10 @@ #[doc(hidden)] pub use crate::worker_intf::{spawn_with_program_path, SpawnErr}; -#[cfg(feature = "test-utils")] use polkadot_primitives::ExecutorParams; /// A function that emulates the stitches together behaviors of the preparation and the execution /// worker in a single synchronous function. -#[cfg(feature = "test-utils")] pub fn validate_candidate( code: &[u8], params: &[u8], @@ -54,7 +52,6 @@ pub fn validate_candidate( /// Use this macro to declare a `fn main() {}` that will check the arguments and dispatch them to /// the appropriate worker, making the executable that can be used for spawning workers. #[macro_export] -#[cfg(feature = "test-utils")] macro_rules! decl_puppet_worker_main { () => { fn main() { diff --git a/parachain/test-parachains/adder/collator/Cargo.toml b/parachain/test-parachains/adder/collator/Cargo.toml index a158006375ae..c0b41fd79f57 100644 --- a/parachain/test-parachains/adder/collator/Cargo.toml +++ b/parachain/test-parachains/adder/collator/Cargo.toml @@ -31,18 +31,19 @@ polkadot-node-subsystem = { path = "../../../../node/subsystem" } sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } +# This one is tricky. Even though it is not used directly by the collator, we still need it for the +# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be +# a big problem since it is used transitively anyway. +polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"], optional = true } [dev-dependencies] polkadot-parachain = { path = "../../.." } polkadot-test-service = { path = "../../../../node/test/service" } -# This one is tricky. Even though it is not used directly by the collator, we still need it for the -# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be -# a big problem since it is used transitively anyway. -polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"] } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } +test-parachain-adder-collator = { path = ".", features = ["test-utils"] } tokio = { version = "1.24.2", features = ["macros"] } diff --git a/parachain/test-parachains/undying/collator/Cargo.toml b/parachain/test-parachains/undying/collator/Cargo.toml index 6e27d81f4e03..727bc3c193d3 100644 --- a/parachain/test-parachains/undying/collator/Cargo.toml +++ b/parachain/test-parachains/undying/collator/Cargo.toml @@ -31,14 +31,15 @@ polkadot-node-subsystem = { path = "../../../../node/subsystem" } sc-cli = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } +# This one is tricky. Even though it is not used directly by the collator, we still need it for the +# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be +# a big problem since it is used transitively anyway. +polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"], optional = true } [dev-dependencies] polkadot-parachain = { path = "../../.." } polkadot-test-service = { path = "../../../../node/test/service" } -# This one is tricky. Even though it is not used directly by the collator, we still need it for the -# `puppet_worker` binary, which is required for the integration test. However, this shouldn't be -# a big problem since it is used transitively anyway. -polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"] } +test-parachain-undying-collator = { path = ".", features = ["test-utils"] } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } From 05f8c9a64625f846227d53e76b6e592ff119bd93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 2 Aug 2023 10:41:39 -0300 Subject: [PATCH 04/10] remove doc hidden from pvf testing --- node/core/pvf/src/testing.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/node/core/pvf/src/testing.rs b/node/core/pvf/src/testing.rs index 3cd1ce304ab8..980a28c01566 100644 --- a/node/core/pvf/src/testing.rs +++ b/node/core/pvf/src/testing.rs @@ -19,7 +19,6 @@ //! N.B. This is not guarded with some feature flag. Overexposing items here may affect the final //! artifact even for production builds. -#[doc(hidden)] pub use crate::worker_intf::{spawn_with_program_path, SpawnErr}; use polkadot_primitives::ExecutorParams; From e6cca272f08824cfeb8760ad5b377277a3c459ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 2 Aug 2023 11:12:31 -0300 Subject: [PATCH 05/10] add prepare worker and execute worker entrypoints to test-utils feature --- node/core/pvf/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node/core/pvf/src/lib.rs b/node/core/pvf/src/lib.rs index 2a026c76e569..246d0cc6835a 100644 --- a/node/core/pvf/src/lib.rs +++ b/node/core/pvf/src/lib.rs @@ -118,7 +118,9 @@ pub use polkadot_node_core_pvf_common::{ }; // Re-export worker entrypoints. +#[cfg(feature = "test-utils")] pub use polkadot_node_core_pvf_execute_worker::worker_entrypoint as execute_worker_entrypoint; +#[cfg(feature = "test-utils")] pub use polkadot_node_core_pvf_prepare_worker::worker_entrypoint as prepare_worker_entrypoint; /// The log target for this crate. From f8e10ce91580d4cd13dfe480aaec019370d7626f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Tue, 8 Aug 2023 12:44:31 -0300 Subject: [PATCH 06/10] pvf: add sp_tracing as optional dependency of test-utils --- node/core/pvf/Cargo.toml | 4 ++-- node/core/pvf/common/Cargo.toml | 4 ++-- node/core/pvf/common/src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index 31d3e829781e..a3d4eb8fd907 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -33,7 +33,7 @@ polkadot-primitives = { path = "../../../primitives" } sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-wasm-interface = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-maybe-compressed-blob = { git = "https://github.com/paritytech/substrate", branch = "master" } -sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master", optional = true } polkadot-node-core-pvf-prepare-worker = { path = "prepare-worker", optional = true } polkadot-node-core-pvf-execute-worker = { path = "execute-worker", optional = true } @@ -53,4 +53,4 @@ halt = { package = "test-parachain-halt", path = "../../../parachain/test-parach ci-only-tests = [] # This feature is used to export test code to other crates without putting it in the production build. # This is also used by the `puppet_worker` binary. -test-utils = ["polkadot-node-core-pvf-prepare-worker", "polkadot-node-core-pvf-execute-worker"] +test-utils = ["polkadot-node-core-pvf-prepare-worker", "polkadot-node-core-pvf-execute-worker", "sp-tracing"] diff --git a/node/core/pvf/common/Cargo.toml b/node/core/pvf/common/Cargo.toml index 27c21245f166..c28aabe2fdbe 100644 --- a/node/core/pvf/common/Cargo.toml +++ b/node/core/pvf/common/Cargo.toml @@ -23,7 +23,7 @@ sc-executor-wasmtime = { git = "https://github.com/paritytech/substrate", branch sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-externalities = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" } -sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master", optional = true } [target.'cfg(target_os = "linux")'.dependencies] landlock = "0.2.0" @@ -35,4 +35,4 @@ tempfile = "3.3.0" [features] # This feature is used to export test code to other crates without putting it in the production build. # Also used for building the puppet worker. -test-utils = [] +test-utils = ["sp-tracing"] diff --git a/node/core/pvf/common/src/lib.rs b/node/core/pvf/common/src/lib.rs index babe36562983..ebcb13eae6a2 100644 --- a/node/core/pvf/common/src/lib.rs +++ b/node/core/pvf/common/src/lib.rs @@ -26,7 +26,7 @@ pub mod worker; pub use cpu_time::ProcessTime; // Used by `decl_worker_main!`. -#[doc(hidden)] +#[cfg(feature = "test-utils")] pub use sp_tracing; const LOG_TARGET: &str = "parachain::pvf-common"; From 735344a30cd7a1f75eed5669e011298ebcc89ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 9 Aug 2023 06:06:15 -0300 Subject: [PATCH 07/10] add test-utils for polkadot and malus --- Cargo.toml | 9 ++++++--- node/malus/Cargo.toml | 10 +++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 760c6ce39533..abc0efce61f2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,10 +5,12 @@ path = "src/main.rs" [[bin]] name = "polkadot-execute-worker" path = "src/bin/execute-worker.rs" +required-features = ["test-utils"] [[bin]] name = "polkadot-prepare-worker" path = "src/bin/prepare-worker.rs" +required-features = ["test-utils"] [package] name = "polkadot" @@ -38,8 +40,8 @@ polkadot-node-core-pvf-prepare-worker = { path = "node/core/pvf/prepare-worker" polkadot-overseer = { path = "node/overseer" } # Needed for worker binaries. -polkadot-node-core-pvf-common = { path = "node/core/pvf/common" } -polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker" } +polkadot-node-core-pvf-common = { path = "node/core/pvf/common", features = ["test-utils"], optional = true } +polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker", optional = true } [dev-dependencies] assert_cmd = "2.0.4" @@ -226,7 +228,8 @@ fast-runtime = [ "polkadot-cli/fast-runtime" ] runtime-metrics = [ "polkadot-cli/runtime-metrics" ] pyroscope = ["polkadot-cli/pyroscope"] jemalloc-allocator = ["polkadot-node-core-pvf-prepare-worker/jemalloc-allocator", "polkadot-overseer/jemalloc-allocator"] - +# Needed for worker binaries. +test-util = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker"] # Enables timeout-based tests supposed to be run only in CI environment as they may be flaky # when run locally depending on system load ci-only-tests = ["polkadot-node-core-pvf/ci-only-tests"] diff --git a/node/malus/Cargo.toml b/node/malus/Cargo.toml index 7e0bf0d8dd08..e3b3b82f603d 100644 --- a/node/malus/Cargo.toml +++ b/node/malus/Cargo.toml @@ -17,11 +17,13 @@ path = "src/malus.rs" [[bin]] name = "polkadot-execute-worker" path = "../../src/bin/execute-worker.rs" +required-features = ["test-utils"] # Prevent rustdoc error. Already documented from top-level Cargo.toml. doc = false [[bin]] name = "polkadot-prepare-worker" path = "../../src/bin/prepare-worker.rs" +required-features = ["test-utils"] # Prevent rustdoc error. Already documented from top-level Cargo.toml. doc = false @@ -48,9 +50,9 @@ erasure = { package = "polkadot-erasure-coding", path = "../../erasure-coding" } rand = "0.8.5" # Required for worker binaries to build. -polkadot-node-core-pvf-common = { path = "../core/pvf/common" } -polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker" } -polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker" } +polkadot-node-core-pvf-common = { path = "../core/pvf/common", features = ["test-utils"], optional = true } +polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker", optional = true } +polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker", optional = true } [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } @@ -63,3 +65,5 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [features] default = [] fast-runtime = ["polkadot-cli/fast-runtime"] +# Needed for worker binaries. +test-utils = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker"] From 41190efe28719ccb7b7cf576ba9e55b566587884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 9 Aug 2023 08:51:49 -0300 Subject: [PATCH 08/10] add test-utils feature to prepare and execute workers script --- Cargo.toml | 2 +- node/malus/Cargo.toml | 8 +++----- scripts/ci/gitlab/pipeline/test.yml | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index abc0efce61f2..d7cee11abdbc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -229,7 +229,7 @@ runtime-metrics = [ "polkadot-cli/runtime-metrics" ] pyroscope = ["polkadot-cli/pyroscope"] jemalloc-allocator = ["polkadot-node-core-pvf-prepare-worker/jemalloc-allocator", "polkadot-overseer/jemalloc-allocator"] # Needed for worker binaries. -test-util = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker"] +test-utils = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker"] # Enables timeout-based tests supposed to be run only in CI environment as they may be flaky # when run locally depending on system load ci-only-tests = ["polkadot-node-core-pvf/ci-only-tests"] diff --git a/node/malus/Cargo.toml b/node/malus/Cargo.toml index e3b3b82f603d..39c3eaf1be25 100644 --- a/node/malus/Cargo.toml +++ b/node/malus/Cargo.toml @@ -50,9 +50,9 @@ erasure = { package = "polkadot-erasure-coding", path = "../../erasure-coding" } rand = "0.8.5" # Required for worker binaries to build. -polkadot-node-core-pvf-common = { path = "../core/pvf/common", features = ["test-utils"], optional = true } -polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker", optional = true } -polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker", optional = true } +polkadot-node-core-pvf-common = { path = "../core/pvf/common", features = ["test-utils"] } +polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker" } +polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker" } [dev-dependencies] polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" } @@ -65,5 +65,3 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" [features] default = [] fast-runtime = ["polkadot-cli/fast-runtime"] -# Needed for worker binaries. -test-utils = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker", "polkadot-node-core-pvf-prepare-worker"] diff --git a/scripts/ci/gitlab/pipeline/test.yml b/scripts/ci/gitlab/pipeline/test.yml index b45c4c1be890..4053e7af1186 100644 --- a/scripts/ci/gitlab/pipeline/test.yml +++ b/scripts/ci/gitlab/pipeline/test.yml @@ -90,7 +90,7 @@ test-node-metrics: RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings" script: # Build the required workers. - - cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker --profile testnet --verbose --locked + - cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker --profile testnet --verbose --locked --features=test-utils # Run tests. - time cargo test --profile testnet --verbose --locked --features=runtime-metrics -p polkadot-node-metrics From 6c46d317ecc2ab8055481d5d1045a47b46c0fffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jo=C3=A3o=20Pedro=20Serrat?= Date: Wed, 9 Aug 2023 10:31:34 -0300 Subject: [PATCH 09/10] remove required features from prepare and executing --- Cargo.toml | 8 ++------ node/malus/Cargo.toml | 2 -- scripts/ci/gitlab/pipeline/test.yml | 2 +- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d7cee11abdbc..59de42666ffe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,12 +5,10 @@ path = "src/main.rs" [[bin]] name = "polkadot-execute-worker" path = "src/bin/execute-worker.rs" -required-features = ["test-utils"] [[bin]] name = "polkadot-prepare-worker" path = "src/bin/prepare-worker.rs" -required-features = ["test-utils"] [package] name = "polkadot" @@ -40,8 +38,8 @@ polkadot-node-core-pvf-prepare-worker = { path = "node/core/pvf/prepare-worker" polkadot-overseer = { path = "node/overseer" } # Needed for worker binaries. -polkadot-node-core-pvf-common = { path = "node/core/pvf/common", features = ["test-utils"], optional = true } -polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker", optional = true } +polkadot-node-core-pvf-common = { path = "node/core/pvf/common", features = ["test-utils"] } +polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker" } [dev-dependencies] assert_cmd = "2.0.4" @@ -228,8 +226,6 @@ fast-runtime = [ "polkadot-cli/fast-runtime" ] runtime-metrics = [ "polkadot-cli/runtime-metrics" ] pyroscope = ["polkadot-cli/pyroscope"] jemalloc-allocator = ["polkadot-node-core-pvf-prepare-worker/jemalloc-allocator", "polkadot-overseer/jemalloc-allocator"] -# Needed for worker binaries. -test-utils = ["polkadot-node-core-pvf-common/test-utils", "polkadot-node-core-pvf-execute-worker"] # Enables timeout-based tests supposed to be run only in CI environment as they may be flaky # when run locally depending on system load ci-only-tests = ["polkadot-node-core-pvf/ci-only-tests"] diff --git a/node/malus/Cargo.toml b/node/malus/Cargo.toml index 39c3eaf1be25..a03df97448bd 100644 --- a/node/malus/Cargo.toml +++ b/node/malus/Cargo.toml @@ -17,13 +17,11 @@ path = "src/malus.rs" [[bin]] name = "polkadot-execute-worker" path = "../../src/bin/execute-worker.rs" -required-features = ["test-utils"] # Prevent rustdoc error. Already documented from top-level Cargo.toml. doc = false [[bin]] name = "polkadot-prepare-worker" path = "../../src/bin/prepare-worker.rs" -required-features = ["test-utils"] # Prevent rustdoc error. Already documented from top-level Cargo.toml. doc = false diff --git a/scripts/ci/gitlab/pipeline/test.yml b/scripts/ci/gitlab/pipeline/test.yml index 4053e7af1186..b45c4c1be890 100644 --- a/scripts/ci/gitlab/pipeline/test.yml +++ b/scripts/ci/gitlab/pipeline/test.yml @@ -90,7 +90,7 @@ test-node-metrics: RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings" script: # Build the required workers. - - cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker --profile testnet --verbose --locked --features=test-utils + - cargo build --bin polkadot-execute-worker --bin polkadot-prepare-worker --profile testnet --verbose --locked # Run tests. - time cargo test --profile testnet --verbose --locked --features=runtime-metrics -p polkadot-node-metrics From ca94de72f9344c0dc92e37c5459bd673c17f4a2f Mon Sep 17 00:00:00 2001 From: Marcin S Date: Mon, 14 Aug 2023 10:55:32 +0200 Subject: [PATCH 10/10] Try to trigger CI again to fix broken jobs --- node/core/pvf/Cargo.toml | 1 + parachain/test-parachains/adder/collator/Cargo.toml | 1 + parachain/test-parachains/undying/collator/Cargo.toml | 1 + 3 files changed, 3 insertions(+) diff --git a/node/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml index a3d4eb8fd907..a7ab7616dd9f 100644 --- a/node/core/pvf/Cargo.toml +++ b/node/core/pvf/Cargo.toml @@ -44,6 +44,7 @@ substrate-build-script-utils = { git = "https://github.com/paritytech/substrate" assert_matches = "1.4.0" hex-literal = "0.3.4" polkadot-node-core-pvf-common = { path = "common", features = ["test-utils"] } +# For the puppet worker, depend on ourselves with the test-utils feature. polkadot-node-core-pvf = { path = ".", features = ["test-utils"] } adder = { package = "test-parachain-adder", path = "../../../parachain/test-parachains/adder" } diff --git a/parachain/test-parachains/adder/collator/Cargo.toml b/parachain/test-parachains/adder/collator/Cargo.toml index c0b41fd79f57..da88adab4285 100644 --- a/parachain/test-parachains/adder/collator/Cargo.toml +++ b/parachain/test-parachains/adder/collator/Cargo.toml @@ -43,6 +43,7 @@ polkadot-test-service = { path = "../../../../node/test/service" } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" } sc-service = { git = "https://github.com/paritytech/substrate", branch = "master" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } +# For the puppet worker, depend on ourselves with the test-utils feature. test-parachain-adder-collator = { path = ".", features = ["test-utils"] } tokio = { version = "1.24.2", features = ["macros"] } diff --git a/parachain/test-parachains/undying/collator/Cargo.toml b/parachain/test-parachains/undying/collator/Cargo.toml index 727bc3c193d3..6cea02bd9113 100644 --- a/parachain/test-parachains/undying/collator/Cargo.toml +++ b/parachain/test-parachains/undying/collator/Cargo.toml @@ -39,6 +39,7 @@ polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test [dev-dependencies] polkadot-parachain = { path = "../../.." } polkadot-test-service = { path = "../../../../node/test/service" } +# For the puppet worker, depend on ourselves with the test-utils feature. test-parachain-undying-collator = { path = ".", features = ["test-utils"] } substrate-test-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }