pvf: use test-utils feature to export test only (#7538)

* pvf: use test-utils feature to export test only * adding comment to test-utils feature * make prepare-worker and execute-worker as optional dependencies and add comments to test-utils * remove doc hidden from pvf testing * add prepare worker and execute worker entrypoints to test-utils feature * pvf: add sp_tracing as optional dependency of test-utils * add test-utils for polkadot and malus * add test-utils feature to prepare and execute workers script * remove required features from prepare and executing * Try to trigger CI again to fix broken jobs --------- Co-authored-by: Marcin S <[email protected]>
paritytech · Aug 14, 2023 · 9417f16 · 9417f16
1 parent 8f05479
commit 9417f16
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 21 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -39,7 +39,7 @@ 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-common = { path = "node/core/pvf/common", features = ["test-utils"] }
 polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker" }
 
 [dev-dependencies]
@@ -227,7 +227,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"]
-
 # 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/core/pvf/Cargo.toml b/node/core/pvf/Cargo.toml
@@ -9,6 +9,7 @@ license.workspace = true
 [[bin]]
 name = "puppet_worker"
 path = "bin/puppet_worker.rs"
+required-features = ["test-utils"]
 
 [dependencies]
 always-assert = "0.1"
@@ -27,26 +28,32 @@ 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" }
 
 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 }
 
 [build-dependencies]
 substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }
 
 [dev-dependencies]
 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" }
 halt = { package = "test-parachain-halt", path = "../../../parachain/test-parachains/halt" }
 
 [features]
 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", "sp-tracing"]
diff --git a/node/core/pvf/common/Cargo.toml b/node/core/pvf/common/Cargo.toml
@@ -25,11 +25,16 @@ 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"
 
 [dev-dependencies]
 assert_matches = "1.4.0"
 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 = ["sp-tracing"]
diff --git a/node/core/pvf/common/src/lib.rs b/node/core/pvf/common/src/lib.rs
@@ -26,15 +26,15 @@ 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";
 
 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
@@ -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
@@ -97,11 +97,11 @@ mod prepare;
 mod priority;
 mod worker_intf;
 
-#[doc(hidden)]
+#[cfg(feature = "test-utils")]
 pub mod testing;
 
 // Used by `decl_puppet_worker_main!`.
-#[doc(hidden)]
+#[cfg(feature = "test-utils")]
 pub use sp_tracing;
 
 pub use error::{InvalidCandidate, ValidationError};
@@ -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.

diff --git 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;

diff --git 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 <http://www.gnu.org/licenses/>.
 
-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/node/malus/Cargo.toml b/node/malus/Cargo.toml
@@ -48,7 +48,7 @@ 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-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" }
 

diff --git a/parachain/test-parachains/adder/collator/Cargo.toml b/parachain/test-parachains/adder/collator/Cargo.toml
@@ -13,6 +13,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,11 +32,10 @@ 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" }
+polkadot-node-core-pvf = { path = "../../../../node/core/pvf", features = ["test-utils"], optional = true }
 
 [dev-dependencies]
 polkadot-parachain = { path = "../../.." }
@@ -44,5 +44,12 @@ 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"] }
+
+[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
@@ -13,6 +13,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,18 +32,24 @@ 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" }
+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" }
+# 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" }
 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]
+# 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"]