[build] Remove testing feature and ensure correct builds

Libra used the 'testing' feature as well as overloaded dev-dependencies to enable testing features of dependencies at test time. Unfortunately due to Cargo's feature unification, this resulted in every build including test and fuzzing code, including code that allowed private keys to be cloned. This retains the testing and fuzzing support while removing the feature unification problems. This is accomplished by never overloading features in dev-dependencies, and instead passing wanted features on the command line at testing time. It also avoids other cases of feature unification by using default-members to control what gets built by default. Building all targets simultaneously will always result in feature unification turning on things as some of our targets are test only and cargo unifies features globally per invocation of cargo build. Because this increase the complexity of running unit tests, some of which need extra flags now, a new extension to Cargo is added called xtask. Use `cargo xtask test-unit` or `cargo xtask test-unit -p CRATE` to unit test all crates or an individual one. The test-unit command knows what flags each crate needs for testing. Closes: aptos-labs#1495 Approved by: bmwill
RuslanBahlai · Oct 24, 2019 · 1d1cb40 · 1d1cb40
1 parent cee5949
commit 1d1cb40
Show file tree

Hide file tree

Showing 114 changed files with 599 additions and 385 deletions.
diff --git a/.cargo/config b/.cargo/config
@@ -0,0 +1,2 @@
+[alias]
+xtask = "run --package xtask --bin xtask --"
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -71,19 +71,27 @@ jobs:
       - run:
           name: Build Release
           command: |
-            [[ $CIRCLE_NODE_INDEX =~ [023] ]] || RUST_BACKTRACE=1 cargo build -j 16 --all --release
+            [[ $CIRCLE_NODE_INDEX =~ [023] ]] || RUST_BACKTRACE=1 cargo build -j 16 --release
       - run:
           name: Build Dev
           command: |
-            [[ $CIRCLE_NODE_INDEX =~ [01] ]] || RUST_BACKTRACE=1 cargo build -j 16 --all
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p benchmark
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p libra-swarm
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p cluster-test
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p libra-fuzzer
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p language_benchmarks
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p bytecode-to-boogie
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p cost-synthesis
+            [[ $CIRCLE_NODE_INDEX =~ [012] ]] || RUST_BACKTRACE=1 cargo build -j 16 -p test-generation
       - run:
           name: Run All Unit Tests
           command: |
-            [[ $CIRCLE_NODE_INDEX =~ [013] ]] || RUST_BACKTRACE=1 cargo test -j 16 --all --exclude testsuite
+            [[ $CIRCLE_NODE_INDEX =~ [013] ]] || RUST_BACKTRACE=1 cargo xtask test-unit
       - run:
           name: Run Cryptography Unit Tests with the formally verified backend
           command: |
-            [[ $CIRCLE_NODE_INDEX =~ [013] ]] || RUST_BACKTRACE=1 cargo test -j 16 -p libra-crypto -p secret-service --features='std fiat_u64_backend' --no-default-features
+            [[ $CIRCLE_NODE_INDEX =~ [013] ]] || ( RUST_BACKTRACE=1 cd crypto/crypto && cargo test --features='std fiat_u64_backend fuzzing' --no-default-features )
       - run:
           name: Run All End to End Tests
           command: |

diff --git a/.gitignore b/.gitignore
@@ -38,8 +38,5 @@ Temporary Items
 # Generated VM config in vm-genesis
 language/vm/vm-genesis/genesis/vm_config.toml
 
-# local cargo config
-.cargo/config
-
 # Terraform
 .terraform/
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -7,17 +7,22 @@ members = [
     "client",
     "client/libra_wallet",
     "common/bounded-executor",
+    "common/channel",
     "common/crash-handler",
     "common/datatest-stable",
     "common/debug-interface",
     "common/executable-helpers",
     "common/failure-ext",
+    "common/failure-ext/failure-macros",
     "common/futures-semaphore",
+    "common/grpc-helpers",
     "common/lcs",
     "common/logger",
     "common/metrics",
+    "common/nibble",
     "common/proptest-helpers",
     "common/prost-ext",
+    "common/tools",
     "config",
     "config/config-builder",
     "config/generate-keypair",
@@ -44,6 +49,7 @@ members = [
     "language/stackless-bytecode/bytecode-to-boogie",
     "language/stackless-bytecode/generator",
     "language/stackless-bytecode/tree_heap",
+    "language/stdlib",
     "language/vm",
     "language/vm/serializer_tests",
     "language/vm/vm-runtime",
@@ -74,6 +80,19 @@ members = [
     "testsuite/libra-fuzzer",
     "types",
     "vm-validator",
+    "xtask",
+]
+
+# NOTE: These members should never include crates that require fuzzing
+# features or test features. These are the crates we want built with no extra
+# test-only code included.
+default-members = [
+    "client",
+    "crypto/secret-service",
+    "language/compiler",
+    "language/vm/vm-genesis",
+    "libra-node",
+    "storage/storage-service",
 ]
 
 [profile.release]

diff --git a/admission_control/admission-control-proto/Cargo.toml b/admission_control/admission-control-proto/Cargo.toml
@@ -20,9 +20,10 @@ libra-logger = { path = "../../common/logger", version = "0.1.0" }
 libra-mempool-shared-proto = { path = "../../mempool/mempool-shared-proto", version = "0.1.0" }
 libra-types = { path = "../../types", version = "0.1.0" }
 
-[dev-dependencies]
-libra-types = { path = "../../types", version = "0.1.0", features = ["testing"]}
-
 [build-dependencies]
 grpcio-compiler = { version = "0.5.0-alpha.2", default-features = false, features = ["prost-codec"] }
 prost-build = "0.5.0"
+
+[features]
+default = []
+fuzzing = ["libra-types/fuzzing"]
diff --git a/admission_control/admission-control-service/Cargo.toml b/admission_control/admission-control-service/Cargo.toml
@@ -37,17 +37,13 @@ vm-validator = { path = "../../vm-validator", version = "0.1.0" }
 libra-prost-ext = { path = "../../common/prost-ext", version = "0.1.0" }
 network = { path = "../../network", version = "0.1.0" }
 
-storage-service = { path = "../../storage/storage-service", version = "0.1.0", optional = true }
-libra-proptest-helpers = { path = "../../common/proptest-helpers", version = "0.1.0", optional = true }
 proptest = { version = "0.9.4", optional = true }
+libra-proptest-helpers = { path = "../../common/proptest-helpers", optional = true }
+storage-service = { path = "../../storage/storage-service" }
 
 [dev-dependencies]
 assert_matches = "1.3.0"
-storage-service = { path = "../../storage/storage-service", version = "0.1.0" }
-libra-types = { path = "../../types", version = "0.1.0", features = ["testing"] }
-libra-proptest-helpers = { path = "../../common/proptest-helpers", version = "0.1.0" }
-proptest = "0.9.4"
 
 [features]
 default = []
-fuzzing = ["storage-service", "libra-proptest-helpers", "proptest"]
+fuzzing = ["proptest", "libra-proptest-helpers", "libra-types/fuzzing", "storage-service/fuzzing"]
diff --git a/admission_control/admission-control-service/src/admission_control_service.rs b/admission_control/admission-control-service/src/admission_control_service.rs
@@ -43,7 +43,7 @@ use vm_validator::vm_validator::{get_account_state, TransactionValidation};
 #[path = "unit_tests/admission_control_service_test.rs"]
 mod admission_control_service_test;
 
-#[cfg(any(feature = "fuzzing", test))]
+#[cfg(feature = "fuzzing")]
 #[path = "admission_control_fuzzing.rs"]
 /// fuzzing module for admission control
 pub mod fuzzing;

diff --git a/admission_control/admission-control-service/src/lib.rs b/admission_control/admission-control-service/src/lib.rs
@@ -13,7 +13,7 @@
 
 /// AC gRPC service.
 pub mod admission_control_service;
-#[cfg(any(test, feature = "fuzzing"))]
+#[cfg(feature = "fuzzing")]
 /// Useful Mocks
 pub mod mocks;
 /// AC runtime to launch gRPC and network service

diff --git a/benchmark/Cargo.toml b/benchmark/Cargo.toml
@@ -29,15 +29,8 @@ libra-wallet = { path = "../client/libra_wallet", version = "0.1.0" }
 libra-swarm = { path = "../libra-swarm", version = "0.1.0" }
 libra-logger = { path = "../common/logger", version = "0.1.0" }
 libra-metrics = { path = "../common/metrics", version = "0.1.0" }
-libra-crypto = { path = "../crypto/crypto", version = "0.1.0" }
+libra-crypto = { path = "../crypto/crypto", version = "0.1.0", features = ["cloneable-private-keys"] }
 rusty-fork = "0.2.1"
 libra-tools = { path = "../common/tools", version = "0.1.0" }
-libra-types = { path = "../types", version = "0.1.0" }
+libra-types = { path = "../types", version = "0.1.0", features = ["fuzzing"] }
 transaction-builder = { path = "../language/transaction-builder", version = "0.1.0" }
-
-[dev-dependencies]
-libra-crypto = { path = "../crypto/crypto", version = "0.1.0", features = ["testing"] }
-
-[features]
-default = []
-testing = ["libra-crypto/testing", "libra-swarm/testing"]
diff --git a/client/Cargo.toml b/client/Cargo.toml
@@ -15,7 +15,7 @@ futures = "0.1.28"
 grpcio = { version = "=0.5.0-alpha.4", default-features = false, features = ["prost-codec"] }
 hex = "0.3.2"
 itertools = "0.8.0"
-proptest = "0.9.2"
+proptest = { version = "0.9.2", optional = true }
 rustyline = "5.0.3"
 rust_decimal = "1.0.2"
 num-traits = "0.2"
@@ -38,9 +38,8 @@ libra-tools = { path = "../common/tools/", version = "0.1.0" }
 transaction-builder = { path = "../language/transaction-builder", version = "0.1.0" }
 
 [dev-dependencies]
-libra-crypto = { path = "../crypto/crypto", version = "0.1.0", features = ["testing"] }
-libra-types = { path = "../types", version = "0.1.0", features = ["testing"]}
+proptest = "0.9.2"
 
 [features]
 default = []
-testing = ["libra-types/testing", "libra-crypto/testing"]
+fuzzing = ["proptest", "libra-crypto/fuzzing", "libra-types/fuzzing"]
diff --git a/client/libra_wallet/Cargo.toml b/client/libra_wallet/Cargo.toml
@@ -24,5 +24,8 @@ failure = { path = "../../common/failure-ext", version = "0.1.0", package = "lib
 libra-types = { path = "../../types", version = "0.1.0" }
 
 [dev-dependencies]
-libra-types = { path = "../../types", version = "0.1.0", features = ["testing"]}
-libra-tools = { path = "../../common/tools", version = "0.1.0"}
+libra-tools = { path = "../../common/tools", version = "0.1.0" }
+
+[features]
+default = []
+fuzzing = ["libra-types/fuzzing"]
diff --git a/client/src/client_proxy.rs b/client/src/client_proxy.rs
@@ -214,7 +214,7 @@ impl ClientProxy {
     }
 
     /// Clone all accounts held in the client.
-    #[cfg(any(test, feature = "testing"))]
+    #[cfg(any(test, feature = "fuzzing"))]
     pub fn copy_all_accounts(&self) -> Vec<AccountData> {
         self.accounts.clone()
     }

diff --git a/client/src/lib.rs b/client/src/lib.rs
@@ -27,7 +27,7 @@ pub(crate) mod transfer_commands;
 /// Struct used to store data for each created account.  We track the sequence number
 /// so we can create new transactions easily
 #[derive(Debug, Serialize, Deserialize, PartialEq)]
-#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[cfg_attr(any(test, feature = "fuzzing"), derive(Clone))]
 pub struct AccountData {
     /// Address of the account.
     pub address: AccountAddress,

diff --git a/common/executable-helpers/Cargo.toml b/common/executable-helpers/Cargo.toml
@@ -12,7 +12,11 @@ edition = "2018"
 [dependencies]
 slog-scope = "4.0"
 
-libra-config = { path = "../../config", version = "0.1.0"}
+libra-config = { path = "../../config", version = "0.1.0" }
 crash-handler = { path = "../crash-handler", version = "0.1.0" }
 libra-logger =  { path = "../logger", version = "0.1.0" }
 libra-metrics = { path = "../metrics", version = "0.1.0" }
+
+[features]
+default = []
+fuzzing = ["libra-config/fuzzing"]
diff --git a/common/nibble/Cargo.toml b/common/nibble/Cargo.toml
@@ -10,5 +10,9 @@ publish = false
 edition = "2018"
 
 [dependencies]
-proptest = "0.9.4"
+proptest = { version = "0.9.4", optional = true }
 serde = { version = "1.0.101", features = ["derive"] }
+
+[features]
+default = []
+fuzzing = ["proptest"]
diff --git a/common/nibble/src/lib.rs b/common/nibble/src/lib.rs
@@ -3,6 +3,7 @@
 
 //! `Nibble` represents a four-bit unsigned integer.
 
+#[cfg(feature = "fuzzing")]
 use proptest::prelude::*;
 use serde::{Deserialize, Serialize};
 use std::fmt;
@@ -29,6 +30,7 @@ impl fmt::LowerHex for Nibble {
     }
 }
 
+#[cfg(feature = "fuzzing")]
 impl Arbitrary for Nibble {
     type Parameters = ();
     type Strategy = BoxedStrategy<Self>;

diff --git a/config/Cargo.toml b/config/Cargo.toml
@@ -24,10 +24,6 @@ failure = { path = "../common/failure-ext", version = "0.1.0", package = "libra-
 libra-tools = { path = "../common/tools", version = "0.1.0" }
 libra-types = { path = "../types", version = "0.1.0" }
 
-[dev-dependencies]
-libra-types = { path = "../types", version = "0.1.0", features = ["testing"] }
-libra-crypto = { path = "../crypto/crypto", version = "0.1.0", features = ["testing"] }
-
 [features]
 default = []
-testing = ["libra-crypto/testing", "libra-types/testing"]
+fuzzing = ["libra-crypto/fuzzing", "libra-types/fuzzing"]
diff --git a/config/config-builder/Cargo.toml b/config/config-builder/Cargo.toml
@@ -24,5 +24,6 @@ libra-prost-ext = { path = "../../common/prost-ext", version = "0.1.0" }
 libra-types = { path = "../../types", version = "0.1.0" }
 vm-genesis = { path = "../../language/vm/vm-genesis", version = "0.1.0" }
 
-[dev-dependencies]
-libra-types = { path = "../../types", version = "0.1.0", features = ["testing"]}
+[features]
+default = []
+fuzzing = ["libra-config/fuzzing"]
diff --git a/config/src/config.rs b/config/src/config.rs
@@ -46,7 +46,7 @@ static CONFIG_TEMPLATE: &[u8] = include_bytes!("../data/configs/node.config.toml
 /// The config file is broken up into sections for each module
 /// so that only that module can be passed around
 #[derive(Debug, Deserialize, Serialize)]
-#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[cfg_attr(any(test, feature = "fuzzing"), derive(Clone))]
 pub struct NodeConfig {
     //TODO Add configuration for multiple chain's in a future diff
     #[serde(default)]
@@ -152,7 +152,7 @@ impl BaseConfig {
     }
 }
 
-#[cfg(any(test, feature = "testing"))]
+#[cfg(any(test, feature = "fuzzing"))]
 impl Clone for BaseConfig {
     fn clone(&self) -> Self {
         Self {
@@ -303,7 +303,7 @@ impl Default for StorageConfig {
     }
 }
 
-#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[cfg_attr(any(test, feature = "fuzzing"), derive(Clone))]
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(default)]
 pub struct NetworkConfig {
@@ -398,7 +398,7 @@ impl NetworkConfig {
     }
 }
 
-#[cfg_attr(any(test, feature = "testing"), derive(Clone))]
+#[cfg_attr(any(test, feature = "fuzzing"), derive(Clone))]
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(default)]
 pub struct ConsensusConfig {
@@ -800,7 +800,7 @@ impl VMConfig {
     /// Creates a new `VMConfig` where the whitelist is empty. This should only be used for testing.
     #[allow(non_snake_case)]
     #[doc(hidden)]
-    #[cfg(any(test, feature = "testing"))]
+    #[cfg(any(test, feature = "fuzzing"))]
     pub fn empty_whitelist_FOR_TESTING() -> Self {
         VMConfig {
             publishing_options: VMPublishingOption::Locked(HashSet::new()),
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		[alias]
		xtask = "run --package xtask --bin xtask --"