[refactor] Break chain/client/src/test_utils.rs into module #9611
Conversation
| @@ -0,0 +1,88 @@ | |||
| use itertools::Itertools; | |||
There was a problem hiding this comment.
Will remove. This file has nothing
There was a problem hiding this comment.
Makes sense, thank you! Leaving couple nits.
Also general suggestion - please prepend largest files in test_utils with comments what logic is inside them. I never had complete understanding what are these 2.5k lines for, and you probably understand it better now :)
chain/client/src/test_utils/mod.rs
Outdated
| pub use test_env::*; | ||
| pub use test_env_builder::*; | ||
|
|
||
| const TEST_SEED: RngSeed = [3; 32]; |
There was a problem hiding this comment.
nit: probably can be moved to setup.rs
There was a problem hiding this comment.
It's used in test_env.rs and test_env_builder.rs as well.
There was a problem hiding this comment.
I understand, but they both depend on setup, right? And I'm assuming that the files don't have cyclic dependencies.
There was a problem hiding this comment.
Right, will shift there!
| pub use block_stats::*; | ||
| pub use client::*; | ||
| pub use peer_manager_mock::*; | ||
| pub use setup::*; | ||
| pub use test_env::*; | ||
| pub use test_env_builder::*; |
There was a problem hiding this comment.
It feels like we can use only one of pub mod or pub use. But I can imagine that it can lead to more chore work, so no strong opinion here.
There was a problem hiding this comment.
I had added in the use so that the current references to these functions doesn't change, otherwise we would need to write them as use near_client::test_utils::test_env_builder::TestEnvBuilder; instead of use near_client::test_utils::TestEnvBuilder;
runtime/near-vm-runner/src/tests.rs
Outdated
| @@ -8,7 +8,6 @@ pub(crate) mod test_builder; | |||
| mod ts_contract; | |||
| mod wasm_validation; | |||
|
|
|||
| #[cfg(all(feature = "near_vm", target_arch = "x86_64"))] | |||
There was a problem hiding this comment.
Oh, this, I can add back but I was getting build errors locally and rust-analyzer wasn't working.
There was a problem hiding this comment.
I think it's better to remove the feature compilation flag (keep the change) as I'm guessing this causes a problem for everyone using rust-analyzer, atleast in vscode. It doesn't cause too much of a difference in compiling.
There was a problem hiding this comment.
I'd make this a separate PR even if it's one line, because @near/contract-runtime should have much better understanding why it is there
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts. Additionally shifting location of `TestEnvNightshadeSetupExt` to nearcore so that it can be used in other modules, specifically `tools/state-viewer/src/state_dump.rs`. Note that we can't have `TestEnvNightshadeSetupExt` in `chain/client/src/test_utils` as it depends on NightshadeRuntime which is a part of `nearcore` module. `nearcore` module depends on `chain` and would create a cyclic dependency.
Breaking a large test_utils.rs file into multiple parts.
Additionally shifting location of
TestEnvNightshadeSetupExtto nearcore so that it can be used in other modules, specificallytools/state-viewer/src/state_dump.rs.Note that we can't have
TestEnvNightshadeSetupExtinchain/client/src/test_utilsas it depends on NightshadeRuntime which is a part ofnearcoremodule.nearcoremodule depends onchainand would create a cyclic dependency.