Use cfg_attr to ignore expensive tests (near#6032)

Rather than using `cfg` to not compile expensive tests, use `cfg_attr`
to conditionally mark such tests as ignored.  In other words, instead
of writing:

    #[cfg(feature = "expensive_tests")]
    #[test]
    fn test_clear_old_data_too_many_heights() {
        // ...
    }

write:

    #[test]
    #[cfg_attr(not(feature = "expensive_tests"), ignore)]
    fn test_clear_old_data_too_many_heights() {
        // ...
    }

With this change, expensive tests will always be built which means
that i) any code changes breaking them will be caught by CI (rather
than having to wait for a nightly run) and ii) code used by expensive
tests only is no longer unused when `expensive_tests` feature is not
enabled (which means fewer `#[cfg(feature = "expensive_tests")]`
directives sprinkled throughout code).

Since we no longer mark whole modules as compiled only if the feature
is enabled, this change also means that each individual test needs to
be marked individually (rather than being able to mark whole module).
This makes it more obvious which tests are expensive and which aren’t
(since the marking is right at the test definition site) and
simplifies `check_nightly.py` script.

Issue: near#4490

chain/chain/src/store.rs

@@ -2943,9 +2943,9 @@ mod tests {
     use borsh::BorshSerialize;
     use strum::IntoEnumIterator;
 
+    use near_chain_configs::GenesisConfig;
     use near_crypto::KeyType;
     use near_primitives::block::{Block, Tip};
-    #[cfg(feature = "expensive_tests")]
     use near_primitives::epoch_manager::block_info::BlockInfo;
     use near_primitives::errors::InvalidTxError;
     use near_primitives::hash::hash;
@@ -2954,10 +2954,9 @@ mod tests {
     use near_primitives::validator_signer::InMemoryValidatorSigner;
     use near_store::test_utils::create_test_store;
     use near_store::DBCol;
-    #[cfg(feature = "expensive_tests")]
-    use {crate::store_validator::StoreValidator, near_chain_configs::GenesisConfig};
 
     use crate::store::{ChainStoreAccess, GCMode};
+    use crate::store_validator::StoreValidator;
     use crate::test_utils::KeyValueRuntime;
     use crate::{Chain, ChainGenesis, DoomslugThresholdMode};
 
@@ -3366,8 +3365,8 @@ mod tests {
     }
 
     /// Test that `gc_blocks_limit` works properly
-    #[cfg(feature = "expensive_tests")]
     #[test]
+    #[cfg_attr(not(feature = "expensive_tests"), ignore)]
     fn test_clear_old_data_too_many_heights() {
         for i in 1..5 {
             println!("gc_blocks_limit == {:?}", i);
@@ -3378,7 +3377,6 @@ mod tests {
         test_clear_old_data_too_many_heights_common(87);
     }
 
-    #[cfg(feature = "expensive_tests")]
     fn test_clear_old_data_too_many_heights_common(gc_blocks_limit: NumBlocks) {
         let mut chain = get_chain_with_epoch_length(1);
         let genesis = chain.get_block_by_height(0).unwrap().clone();

chain/chain/src/tests/doomslug.rs

@@ -301,8 +301,8 @@ fn one_iter(
     (now - started, largest_produced_height)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_fuzzy_doomslug_liveness_and_safety() {
     for (time_to_gst_millis, height_goal) in
         &[(0, 200), (1000, 200), (10000, 300), (100000, 400), (500000, 500)]

chain/chain/src/tests/gc.rs

@@ -293,8 +293,8 @@ fn test_gc_remove_fork_small() {
     test_gc_remove_fork_common(1)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_remove_fork_large() {
     test_gc_remove_fork_common(20)
 }
@@ -340,8 +340,8 @@ fn test_gc_not_remove_fork_small() {
     test_gc_not_remove_fork_common(1)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_not_remove_fork_large() {
     test_gc_not_remove_fork_common(20)
 }
@@ -426,8 +426,8 @@ fn test_gc_boundaries_small() {
     test_gc_boundaries_common(1)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_boundaries_large() {
     test_gc_boundaries_common(20)
 }
@@ -455,8 +455,8 @@ fn test_gc_random_small() {
     test_gc_random_common(3);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_random_large() {
     test_gc_random_common(25);
 }
@@ -500,8 +500,8 @@ fn test_gc_pine_small() {
     gc_fork_common(chains, 1);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_pine() {
     for max_changes in 1..=20 {
         let mut chains = vec![SimpleChain { from: 0, length: 101, is_removed: false }];
@@ -542,8 +542,8 @@ fn test_gc_star_small() {
     test_gc_star_common(1)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_gc_star_large() {
     test_gc_star_common(20)
 }

chain/chain/src/tests/mod.rs

@@ -1,5 +1,4 @@
 mod challenges;
-#[cfg(feature = "expensive_tests")]
 mod doomslug;
 mod gc;
 mod simple_chain;

chain/chunks/src/lib.rs

@@ -2039,26 +2039,27 @@ impl ShardsManager {
 
 #[cfg(test)]
 mod test {
-    use super::*;
-    use crate::test_utils::*;
-    use near_chain::test_utils::KeyValueRuntime;
-    use near_network::test_utils::MockPeerManagerAdapter;
-    use near_primitives::hash::{hash, CryptoHash};
-    use near_store::test_utils::create_test_store;
+
     use std::sync::Arc;
     use std::time::Duration;
 
+    use near_chain::test_utils::KeyValueRuntime;
+    use near_chain::{ChainStore, RuntimeAdapter};
+    use near_crypto::KeyType;
+    use near_logger_utils::init_test_logger;
+    use near_network::test_utils::MockPeerManagerAdapter;
     use near_network::types::NetworkRequests;
     use near_primitives::block::Tip;
+    use near_primitives::hash::{hash, CryptoHash};
+    use near_primitives::merkle::merklize;
+    use near_primitives::sharding::ReedSolomonWrapper;
     use near_primitives::types::EpochId;
-    #[cfg(feature = "expensive_tests")]
-    use {
-        crate::ACCEPTING_SEAL_PERIOD_MS, near_chain::ChainStore, near_chain::RuntimeAdapter,
-        near_crypto::KeyType, near_logger_utils::init_test_logger,
-        near_primitives::merkle::merklize, near_primitives::sharding::ReedSolomonWrapper,
-        near_primitives::validator_signer::InMemoryValidatorSigner,
-        near_primitives::version::PROTOCOL_VERSION,
-    };
+    use near_primitives::validator_signer::InMemoryValidatorSigner;
+    use near_primitives::version::PROTOCOL_VERSION;
+    use near_store::test_utils::create_test_store;
+
+    use super::*;
+    use crate::test_utils::*;
 
     const TEST_SEED: RngSeed = [3; 32];
 
@@ -2106,8 +2107,8 @@ mod test {
         };
     }
 
-    #[cfg(feature = "expensive_tests")]
     #[test]
+    #[cfg_attr(not(feature = "expensive_tests"), ignore)]
     fn test_seal_removal() {
         init_test_logger();
         let runtime_adapter = Arc::new(KeyValueRuntime::new_with_validators(
@@ -2181,7 +2182,7 @@ mod test {
             encoded_chunk.create_partial_encoded_chunk(vec![0, 1], vec![], &proof);
         let partial_encoded_chunk2 =
             encoded_chunk.create_partial_encoded_chunk(vec![2, 3, 4], vec![], &proof);
-        std::thread::sleep(Duration::from_millis(ACCEPTING_SEAL_PERIOD_MS as u64 + 100));
+        std::thread::sleep(Duration::from_millis(crate::ACCEPTING_SEAL_PERIOD_MS as u64 + 100));
         for partial_encoded_chunk in vec![partial_encoded_chunk1, partial_encoded_chunk2] {
             let pec_v2 = partial_encoded_chunk.into();
             shards_manager

chain/client/src/tests/catching_up.rs

@@ -96,8 +96,8 @@ pub struct StateRequestStruct {
 }
 
 /// Sanity checks that the incoming and outgoing receipts are properly sent and received
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_receipts_sync_third_epoch() {
     test_catchup_receipts_sync_common(13, 1, false)
 }
@@ -109,20 +109,20 @@ fn test_catchup_receipts_sync_third_epoch() {
 /// It will be executed 10 times faster.
 /// The reason of increasing block_prod_time in the test is to allow syncing complete.
 /// Otherwise epochs will be changing faster than state sync happen.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_receipts_sync_hold() {
     test_catchup_receipts_sync_common(13, 1, true)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_receipts_sync_last_block() {
     test_catchup_receipts_sync_common(13, 5, false)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_receipts_sync_distant_epoch() {
     test_catchup_receipts_sync_common(35, 1, false)
 }
@@ -376,37 +376,37 @@ enum RandomSinglePartPhases {
 /// If random one parts fetched during the epoch preceding the epoch a block producer is
 /// assigned to were to have incorrect receipts, the balances in the fourth epoch would have
 /// been incorrect due to wrong receipts applied during the third epoch.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_random_single_part_sync() {
     test_catchup_random_single_part_sync_common(false, false, 13)
 }
 
 // Same test as `test_catchup_random_single_part_sync`, but skips the chunks on height 14 and 15
 // It causes all the receipts to be applied only on height 16, which is the next epoch.
 // It tests that the incoming receipts are property synced through epochs
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_random_single_part_sync_skip_15() {
     test_catchup_random_single_part_sync_common(true, false, 13)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_random_single_part_sync_send_15() {
     test_catchup_random_single_part_sync_common(false, false, 15)
 }
 
 // Make sure that transactions are at least applied.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_random_single_part_sync_non_zero_amounts() {
     test_catchup_random_single_part_sync_common(false, true, 13)
 }
 
 // Use another height to send txs.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_random_single_part_sync_height_6() {
     test_catchup_random_single_part_sync_common(false, false, 6)
 }
@@ -618,8 +618,8 @@ fn test_catchup_random_single_part_sync_common(skip_15: bool, non_zero: bool, he
 /// to be skipped)
 /// This test would fail if at any point validators got stuck with state sync, or block
 /// production stalled for any other reason.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_catchup_sanity_blocks_produced() {
     let validator_groups = 2;
     init_integration_logger();
@@ -693,8 +693,8 @@ enum ChunkGrievingPhases {
 }
 
 // TODO(#3180): seals are disabled in single shard setting
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 #[ignore]
 fn test_chunk_grieving() {
     let validator_groups = 1;
@@ -847,20 +847,20 @@ fn test_chunk_grieving() {
     });
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_all_chunks_accepted_1000() {
     test_all_chunks_accepted_common(1000, 3000, 5)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_all_chunks_accepted_1000_slow() {
     test_all_chunks_accepted_common(1000, 6000, 5)
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_all_chunks_accepted_1000_rare_epoch_changing() {
     test_all_chunks_accepted_common(1000, 1500, 100)
 }

chain/client/src/tests/consensus.rs

@@ -21,8 +21,8 @@ use near_primitives::types::{AccountId, BlockHeight};
 /// the future, and delay the distribution of the block produced this way.
 /// Periodically verify finality is not violated.
 /// This test is designed to reproduce finality bugs on the epoch boundaries.
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_consensus_with_epoch_switches() {
     init_integration_logger();
 

chain/client/src/tests/cross_shard_tx.rs

@@ -96,7 +96,6 @@ fn test_keyvalue_runtime_balances() {
     });
 }
 
-#[cfg(feature = "expensive_tests")]
 fn send_tx(
     num_validators: usize,
     connectors: Arc<RwLock<Vec<(Addr<ClientActor>, Addr<ViewClientActor>)>>>,
@@ -155,7 +154,6 @@ fn send_tx(
     );
 }
 
-#[cfg(feature = "expensive_tests")]
 fn test_cross_shard_tx_callback(
     res: Result<Result<QueryResponse, crate::QueryError>, MailboxError>,
     account_id: AccountId,
@@ -373,7 +371,6 @@ fn test_cross_shard_tx_callback(
     }
 }
 
-#[cfg(feature = "expensive_tests")]
 fn test_cross_shard_tx_common(
     num_iters: usize,
     rotate_validators: bool,
@@ -510,44 +507,44 @@ fn test_cross_shard_tx_common(
     });
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx() {
     test_cross_shard_tx_common(64, false, false, false, 70, Some(2.3), None);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_doomslug() {
     test_cross_shard_tx_common(64, false, false, true, 200, None, Some(1.5));
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_drop_chunks() {
     test_cross_shard_tx_common(64, false, true, false, 250, None, None);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_8_iterations() {
     test_cross_shard_tx_common(8, false, false, false, 200, Some(2.4), None);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_8_iterations_drop_chunks() {
     test_cross_shard_tx_common(8, false, true, false, 200, Some(2.4), None);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_with_validator_rotation_1() {
     test_cross_shard_tx_common(8, true, false, false, 220, Some(2.4), None);
 }
 
-#[cfg(feature = "expensive_tests")]
 #[test]
+#[cfg_attr(not(feature = "expensive_tests"), ignore)]
 fn test_cross_shard_tx_with_validator_rotation_2() {
     test_cross_shard_tx_common(24, true, false, false, 400, Some(2.4), None);
 }

chain/client/src/tests/mod.rs

@@ -1,8 +1,6 @@
 mod bug_repros;
-#[cfg(feature = "expensive_tests")]
 mod catching_up;
 mod chunks_management;
-#[cfg(feature = "expensive_tests")]
 mod consensus;
 mod cross_shard_tx;
 mod query_client;