pvf: use test-utils feature to export test only #7538
Conversation
node/core/pvf/Cargo.toml
Outdated
|
|
||
| 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 = [] |
There was a problem hiding this comment.
One comment please on how to use it.
There was a problem hiding this comment.
Ok, I'm going to add. Thanks!
There was a problem hiding this comment.
@ggwpez Comment added. Is this what you had in mind?
There was a problem hiding this comment.
Thanks for adding comments. 🙌 But I think it is not entirely correct. The feature is not just for exporting code but also for building the puppet worker which is part of this crate.
node/core/pvf/Cargo.toml
Outdated
|
|
||
| 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 = [] |
There was a problem hiding this comment.
Thanks for adding comments. 🙌 But I think it is not entirely correct. The feature is not just for exporting code but also for building the puppet worker which is part of this crate.
…dd comments to test-utils
mrcnski
added
A0-please_review
|
Tests are passing 🙌 so this is just waiting on one more review. |
|
Thanks for the help with this one! |
|
Note that this PR requires cumulus companion. |
| pub mod testing; | ||
|
|
||
| // Used by `decl_puppet_worker_main!`. | ||
| #[doc(hidden)] | ||
| #[cfg(feature = "test-utils")] | ||
| pub use sp_tracing; |
There was a problem hiding this comment.
Does the dependency itself become optional too?
There was a problem hiding this comment.
Yes, in this case, the dependency will only be enabled if the test-utils feature is being used.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Sorry for the misunderstanding. Done.
mrcnski
added
A4-companion
|
@mrcnski I'm gonna do that. |
|
Ok, I'll remove it. It was cause I notice that the code for the workers was a dependency for polkadot, so I thought that I should remove it from there as well and put it as optional. I'm making the change now. |
|
Thanks @jpserrat! Yeah polkadot depends on the workers but they're not optional. I can see how the code structure is confusing, it should be documented, I'll write up an issue. |
|
Still missing companion for cumulus |
|
Doing that now. |
|
bot merge |
|
Error: Statuses failed for 6c46d31 |
|
bot merge |
* 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]>
Adding the test-utils feature for pvf. I've decided to leave the testing module with doc(hidden) and add the test-utils feature directly to the functions within the module. Made this decision because the testing module is exporting
spawn_with_program_path, which is used inside the integration tests. Please let me know if there is a better way to do this.Related
closes #7285
Cumulus companion: paritytech/cumulus#2989