Skip to content

Commit

Permalink
integration-tests: fix some expensive tests marking (near#6031)
Browse files Browse the repository at this point in the history
Firstly, the proper way to denote a test as expensive is to use
`#[cfg]` directive before its definition rather than adding a return
inside of the function.  The latter will cause the test to show up in
list of tests and always pass giving people false sense that running
the test without `expensive_tests` feature actually does something.

After fixing `sync_state_nodes_multishard` also add it to nightly run.

Secondly, the `check_nightly.py` script is rather crude and the order
in which `#[cfg]` and `#[test]` attributes are applied do matter.
With `#[test]` being first, the script failed to recognise
`test_catchup` as an expensive test.

Thirdly, the `check_nightly.py` script also doesn’t understand
`#[cfg]` matching multiple features.  Because of that it does not
detect `test_highload` as an expensive test either.  Since we’re not
using `regression_tests` feature for anything simply remove it.

Issue: near#4490
  • Loading branch information
mina86 authored Jan 10, 2022
1 parent 419b816 commit c7d5b1e
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 6 deletions.
1 change: 0 additions & 1 deletion integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ portpicker = "0.1.1"

[features]
performance_stats = ["nearcore/performance_stats", "near-network/performance_stats"]
regression_tests = []
expensive_tests = []
test_features = ["nearcore/test_features"]
protocol_feature_alt_bn128 = [
Expand Down
4 changes: 1 addition & 3 deletions integration-tests/src/tests/nearcore/sync_state_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,9 @@ fn sync_state_nodes() {
}

/// One client is in front, another must sync to it using state (fast) sync.
#[cfg(feature = "expensive_tests")]
#[test]
fn sync_state_nodes_multishard() {
if !cfg!(feature = "expensive_tests") {
return;
}
heavy_test(|| {
init_integration_logger();

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/tests/test_catchup.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Checks that late validator can catch-up and start validating.
#[test]
#[cfg(feature = "expensive_tests")]
#[test]
fn test_catchup() {
use std::time::Duration;

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/tests/test_tps_regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! and verifies that the output tps is not much different from the input tps (makes sure there is
//! no choking on transactions). The input tps -- is how fast the nodes can be accepting
//! transactions. The output tps -- is how fast the nodes propagate transactions into the blocks.
#[cfg(any(feature = "expensive_tests", feature = "regression_tests"))]
#[cfg(feature = "expensive_tests")]
#[cfg(test)]
mod test {
use std::io::stdout;
Expand Down
3 changes: 3 additions & 0 deletions nightly/expensive.txt
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,6 @@ expensive integration-tests integration_tests tests::test_simple::test::test_4_1
expensive integration-tests integration_tests tests::test_simple::test::test_4_10_multiple_nodes --features nightly_protocol,nightly_protocol_features
expensive integration-tests integration_tests tests::test_simple::test::test_7_10_multiple_nodes
expensive integration-tests integration_tests tests::test_simple::test::test_7_10_multiple_nodes --features nightly_protocol,nightly_protocol_features

expensive integration-tests integration_tests tests::nearcore::sync_state_nodes::sync_state_nodes_multishard
expensive integration-tests integration_tests tests::nearcore::sync_state_nodes::sync_state_nodes_multishard --features nightly_protocol,nightly_protocol_features

0 comments on commit c7d5b1e

Please sign in to comment.