Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

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

Merged
merged 11 commits into from
Aug 14, 2023
Merged

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

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cb9d3f2
pvf: use test-utils feature to export test only
jpserrat Jul 22, 2023
b72c655
adding comment to test-utils feature
jpserrat Jul 25, 2023
db467ff
make prepare-worker and execute-worker as optional dependencies and a…
jpserrat Aug 2, 2023
49067f9
Merge branch 'master' into jpserrat-pvf-add-test-utils-and-remove-doc…
mrcnski Aug 2, 2023
05f8c9a
remove doc hidden from pvf testing
jpserrat Aug 2, 2023
e6cca27
add prepare worker and execute worker entrypoints to test-utils feature
jpserrat Aug 2, 2023
f8e10ce
pvf: add sp_tracing as optional dependency of test-utils
jpserrat Aug 8, 2023
735344a
add test-utils for polkadot and malus
jpserrat Aug 9, 2023
41190ef
add test-utils feature to prepare and execute workers script
jpserrat Aug 9, 2023
6c46d31
remove required features from prepare and executing
jpserrat Aug 9, 2023
ca94de7
Try to trigger CI again to fix broken jobs
mrcnski Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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]
Expand Down Expand Up @@ -226,7 +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"]

# 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"]
Expand Down
12 changes: 9 additions & 3 deletions node/core/pvf/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition.workspace = true
[[bin]]
name = "puppet_worker"
path = "bin/puppet_worker.rs"
required-features = ["test-utils"]
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
mrcnski marked this conversation as resolved.
Show resolved Hide resolved

[dependencies]
mrcnski marked this conversation as resolved.
Show resolved Hide resolved
always-assert = "0.1"
Expand All @@ -25,26 +26,31 @@ 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"] }
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"]
7 changes: 6 additions & 1 deletion node/core/pvf/common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,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"]
4 changes: 2 additions & 2 deletions node/core/pvf/common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions node/core/pvf/common/src/pvf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions node/core/pvf/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the dependency itself become optional too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in this case, the dependency will only be enabled if the test-utils feature is being used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be re-exported with the feature, however it's always there in toml file:

sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }

My question is could it be made optional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the misunderstanding. Done.


pub use error::{InvalidCandidate, ValidationError};
Expand All @@ -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.
Expand Down
1 change: 0 additions & 1 deletion node/core/pvf/src/testing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions node/core/pvf/tests/it/worker_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion node/malus/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
10 changes: 8 additions & 2 deletions parachain/test-parachains/adder/collator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ path = "src/main.rs"
[[bin]]
name = "adder_collator_puppet_worker"
path = "bin/puppet_worker.rs"
required-features = ["test-utils"]
mrcnski marked this conversation as resolved.
Show resolved Hide resolved

[dependencies]
parity-scale-codec = { version = "3.6.1", default-features = false, features = ["derive"] }
Expand All @@ -30,11 +31,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 = "../../.." }
Expand All @@ -43,5 +43,11 @@ 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" }
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"]
10 changes: 8 additions & 2 deletions parachain/test-parachains/undying/collator/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand All @@ -30,18 +31,23 @@ 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" }
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"]