Skip to content

Commit

Permalink
Introduce a #[nightly_test] attribute
Browse files Browse the repository at this point in the history
Introduce a `#[nightly_test]` attribute to be used to mark tests as
expensive (i.e. to be run nightly on NayDuck but omitted from casual
`cargo test` invocations or CI runs).

It replaces the use of `#[cfg(feature = "expensive_tests")]` directive
and under the hood marks tests as ignored rather than skipping their
compilation.

This means that while previously nightly tests would be built only if
`expensive_tests` feature is set now they are always compiled.  This
further means that changes that break such tests will be caught by CI
(previously they would break only once NayDuck decides to run them)
and there is no longer need to conditionally compile helper functions
which are used by such tests only.

Issue: #4490
  • Loading branch information
mina86 committed Jan 9, 2022
1 parent 56e3c1c commit faf1d75
Show file tree
Hide file tree
Showing 26 changed files with 180 additions and 191 deletions.
16 changes: 12 additions & 4 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ members = [
"integration-tests",
"utils/near-rate-limiter",
"utils/near-cache",
"utils/near-macros",
]

[workspace.metadata.workspaces]
Expand Down
1 change: 1 addition & 0 deletions chain/chain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ delay-detector = { path = "../../tools/delay_detector", optional = true}
near-chain-configs = { path = "../../core/chain-configs" }
near-chain-primitives = { path = "../chain-primitives" }
near-crypto = { path = "../../core/crypto" }
near-macros = { path = "../../utils/near-macros" }
near-metrics = { path = "../../core/metrics" }
near-pool = { path = "../pool" }
near-primitives = { path = "../../core/primitives" }
Expand Down
7 changes: 2 additions & 5 deletions chain/chain/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2944,8 +2944,8 @@ mod tests {
use strum::IntoEnumIterator;

use near_crypto::KeyType;
use near_macros::nightly_test;
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;
Expand All @@ -2954,7 +2954,6 @@ 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};
Expand Down Expand Up @@ -3366,8 +3365,7 @@ mod tests {
}

/// Test that `gc_blocks_limit` works properly
#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_clear_old_data_too_many_heights() {
for i in 1..5 {
println!("gc_blocks_limit == {:?}", i);
Expand All @@ -3378,7 +3376,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();
Expand Down
4 changes: 2 additions & 2 deletions chain/chain/src/tests/doomslug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::time::{Duration, Instant};

use crate::{Doomslug, DoomslugThresholdMode};
use near_crypto::{KeyType, SecretKey};
use near_macros::nightly_test;
use near_primitives::block::Approval;
use near_primitives::hash::{hash, CryptoHash};
use near_primitives::types::{ApprovalStake, BlockHeight};
Expand Down Expand Up @@ -301,8 +302,7 @@ fn one_iter(
(now - started, largest_produced_height)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
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)]
Expand Down
19 changes: 7 additions & 12 deletions chain/chain/src/tests/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::test_utils::KeyValueRuntime;
use crate::types::{ChainGenesis, Tip};
use crate::DoomslugThresholdMode;
use near_crypto::KeyType;
use near_macros::nightly_test;
use near_primitives::block::Block;
use near_primitives::merkle::PartialMerkleTree;
use near_primitives::shard_layout::ShardUId;
Expand Down Expand Up @@ -293,8 +294,7 @@ fn test_gc_remove_fork_small() {
test_gc_remove_fork_common(1)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_remove_fork_large() {
test_gc_remove_fork_common(20)
}
Expand Down Expand Up @@ -340,8 +340,7 @@ fn test_gc_not_remove_fork_small() {
test_gc_not_remove_fork_common(1)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_not_remove_fork_large() {
test_gc_not_remove_fork_common(20)
}
Expand Down Expand Up @@ -426,8 +425,7 @@ fn test_gc_boundaries_small() {
test_gc_boundaries_common(1)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_boundaries_large() {
test_gc_boundaries_common(20)
}
Expand Down Expand Up @@ -455,8 +453,7 @@ fn test_gc_random_small() {
test_gc_random_common(3);
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_random_large() {
test_gc_random_common(25);
}
Expand Down Expand Up @@ -500,8 +497,7 @@ fn test_gc_pine_small() {
gc_fork_common(chains, 1);
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_pine() {
for max_changes in 1..=20 {
let mut chains = vec![SimpleChain { from: 0, length: 101, is_removed: false }];
Expand Down Expand Up @@ -542,8 +538,7 @@ fn test_gc_star_small() {
test_gc_star_common(1)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_gc_star_large() {
test_gc_star_common(20)
}
1 change: 0 additions & 1 deletion chain/chain/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod challenges;
#[cfg(feature = "expensive_tests")]
mod doomslug;
mod gc;
mod simple_chain;
Expand Down
1 change: 1 addition & 0 deletions chain/chunks/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ near-network = { path = "../network" }
near-chain = { path = "../chain" }
near-pool = { path = "../pool" }
near-network-primitives = { path = "../network-primitives" }
near-macros = { path = "../../utils/near-macros" }

[dev-dependencies]
near-logger-utils = { path = "../../test-utils/logger" }
Expand Down
7 changes: 3 additions & 4 deletions chain/chunks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ impl ShardsManager {
if stored_chunk.len() < MAX_STORED_PARTIAL_CHUNK_SIZE {
stored_chunk.push(partial_encoded_chunk.clone());
} else {
warn!(target:"shards_manager", "Drop partial encoded chunk because stored partial encoded chunks already exceed limit, height: {} shard: {}",
warn!(target:"shards_manager", "Drop partial encoded chunk because stored partial encoded chunks already exceed limit, height: {} shard: {}",
height, shard_id);
}
}
Expand Down Expand Up @@ -1869,10 +1869,10 @@ mod test {
use std::sync::Arc;
use std::time::Duration;

use near_macros::nightly_test;
use near_network::types::NetworkRequests;
use near_primitives::block::Tip;
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,
Expand Down Expand Up @@ -1926,8 +1926,7 @@ mod test {
};
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_seal_removal() {
init_test_logger();
let runtime_adapter = Arc::new(KeyValueRuntime::new_with_validators(
Expand Down
1 change: 1 addition & 0 deletions chain/client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ near-metrics = { path = "../../core/metrics" }
near-chain-configs = { path = "../../core/chain-configs" }
near-chain = { path = "../chain" }
near-client-primitives = { path = "../client-primitives" }
near-macros = { path = "../../utils/near-macros" }
near-network = { path = "../network" }
near-pool = { path = "../pool" }
near-chunks = { path = "../chunks" }
Expand Down
43 changes: 15 additions & 28 deletions chain/client/src/tests/catching_up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use near_chain::test_utils::account_id_to_shard_id;
use near_chain_configs::TEST_STATE_SYNC_TIMEOUT;
use near_crypto::{InMemorySigner, KeyType};
use near_logger_utils::init_integration_logger;
use near_macros::nightly_test;
use near_network::types::{
NetworkClientMessages, NetworkRequests, NetworkResponses, PeerManagerMessageRequest,
};
Expand Down Expand Up @@ -96,8 +97,7 @@ pub struct StateRequestStruct {
}

/// Sanity checks that the incoming and outgoing receipts are properly sent and received
#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_catchup_receipts_sync_third_epoch() {
test_catchup_receipts_sync_common(13, 1, false)
}
Expand All @@ -109,20 +109,17 @@ 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]
#[nightly_test]
fn test_catchup_receipts_sync_hold() {
test_catchup_receipts_sync_common(13, 1, true)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_catchup_receipts_sync_last_block() {
test_catchup_receipts_sync_common(13, 5, false)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_catchup_receipts_sync_distant_epoch() {
test_catchup_receipts_sync_common(35, 1, false)
}
Expand Down Expand Up @@ -376,37 +373,32 @@ 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]
#[nightly_test]
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]
#[nightly_test]
fn test_catchup_random_single_part_sync_skip_15() {
test_catchup_random_single_part_sync_common(true, false, 13)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
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]
#[nightly_test]
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]
#[nightly_test]
fn test_catchup_random_single_part_sync_height_6() {
test_catchup_random_single_part_sync_common(false, false, 6)
}
Expand Down Expand Up @@ -618,8 +610,7 @@ 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]
#[nightly_test]
fn test_catchup_sanity_blocks_produced() {
let validator_groups = 2;
init_integration_logger();
Expand Down Expand Up @@ -693,8 +684,7 @@ enum ChunkGrievingPhases {
}

// TODO(#3180): seals are disabled in single shard setting
#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
#[ignore]
fn test_chunk_grieving() {
let validator_groups = 1;
Expand Down Expand Up @@ -847,20 +837,17 @@ fn test_chunk_grieving() {
});
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_all_chunks_accepted_1000() {
test_all_chunks_accepted_common(1000, 3000, 5)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_all_chunks_accepted_1000_slow() {
test_all_chunks_accepted_common(1000, 6000, 5)
}

#[cfg(feature = "expensive_tests")]
#[test]
#[nightly_test]
fn test_all_chunks_accepted_1000_rare_epoch_changing() {
test_all_chunks_accepted_common(1000, 1500, 100)
}
Expand Down
Loading

0 comments on commit faf1d75

Please sign in to comment.