From a634e051dd41bb53b2da5a2687d9628dd14481cc Mon Sep 17 00:00:00 2001 From: Bas van Dijk Date: Tue, 1 Sep 2026 22:15:32 +0000 Subject: [PATCH 1/2] fix: deflake //rs/tests/consensus/upgrade:upgrade_downgrade_unassigned_nodes_test The test performs two sequential GuestOS upgrades of the unassigned node, each polled with a 600s retry budget, but sets no driver timeouts, so the SystemTestGroup default per-test timeout of 10 minutes applies: a single exhausted poll alone eats the whole budget by design. The bazel test_timeout also defaulted to "long" (900s), leaving ~700s for setup + test + teardown. Give the test explicit driver timeouts covering its own worst case (25min per test / 30min overall) and set test_timeout = "eternal" on both targets sharing the binary, like the upgrade_downgrade_*_subnet_test siblings in this package. Co-Authored-By: Claude Fable 5 --- rs/tests/consensus/upgrade/BUILD.bazel | 2 ++ .../upgrade_downgrade_unassigned_nodes_test.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/rs/tests/consensus/upgrade/BUILD.bazel b/rs/tests/consensus/upgrade/BUILD.bazel index 98d818ae9085..f4d644f74b05 100644 --- a/rs/tests/consensus/upgrade/BUILD.bazel +++ b/rs/tests/consensus/upgrade/BUILD.bazel @@ -162,6 +162,7 @@ system_test_nns( "long_test", ], test_driver_target = ":upgrade_downgrade_unassigned_test_bin", + test_timeout = "eternal", ) system_test_nns( @@ -174,6 +175,7 @@ system_test_nns( "long_test", ], test_driver_target = ":upgrade_downgrade_unassigned_test_bin", + test_timeout = "eternal", ) system_test_nns( diff --git a/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs b/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs index cf627782b04b..92c095c1233f 100644 --- a/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs +++ b/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs @@ -22,6 +22,8 @@ Success:: end::catalog[] */ +use std::time::Duration; + use anyhow::Result; use anyhow::bail; use ic_consensus_system_test_upgrade_common::elect_target_version; @@ -40,6 +42,15 @@ use ic_types::ReplicaVersion; use slog::Logger; use slog::info; +// The test performs two sequential GuestOS upgrades of the unassigned node, +// each of which is polled with a 600s retry budget (see +// `upgrade_unassigned_nodes` below). The driver's default per-test timeout of +// 10 minutes cannot even cover a single exhausted poll, so give the test an +// explicit budget that covers its own worst case (2 x 600s polling + proposal +// handling + slack), like its siblings in this package do. +const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_secs(30 * 60); +const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_secs(25 * 60); + fn setup(env: TestEnv) { InternetComputer::new() .add_fast_single_node_subnet(SubnetType::System) @@ -96,6 +107,8 @@ fn upgrade_downgrade_unassigned_nodes(env: TestEnv) { fn main() -> Result<()> { SystemTestGroup::new() + .with_overall_timeout(UP_DOWNGRADE_OVERALL_TIMEOUT) + .with_timeout_per_test(UP_DOWNGRADE_PER_TEST_TIMEOUT) .with_setup(setup) .add_test(systest!(upgrade_downgrade_unassigned_nodes)) .execute_from_args()?; From 8761942152b0162780fbcd129cb5f59baedf13a9 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 4 Sep 2026 09:48:56 +0000 Subject: [PATCH 2/2] refactor: reuse constants --- rs/tests/consensus/upgrade/common.rs | 6 +++--- .../upgrade/upgrade_downgrade_app_subnet_test.rs | 14 +++++--------- .../upgrade/upgrade_downgrade_nns_subnet_test.rs | 13 +++++-------- .../upgrade_downgrade_unassigned_nodes_test.rs | 15 +++------------ 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/rs/tests/consensus/upgrade/common.rs b/rs/tests/consensus/upgrade/common.rs index 9beb3fa2917e..6c0bafc49a30 100644 --- a/rs/tests/consensus/upgrade/common.rs +++ b/rs/tests/consensus/upgrade/common.rs @@ -38,10 +38,10 @@ use slog::{Logger, info}; use std::collections::BTreeMap; use std::time::Duration; -const ALLOWED_FAILURES: usize = 1; +pub const ALLOWED_FAILURES: usize = 1; -pub const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_secs(25 * 60); -pub const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_secs(20 * 60); +pub const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_mins(35); +pub const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_mins(30); pub fn elect_target_version(env: &TestEnv, nns_node: &IcNodeSnapshot) -> ReplicaVersion { let logger = env.logger(); diff --git a/rs/tests/consensus/upgrade/upgrade_downgrade_app_subnet_test.rs b/rs/tests/consensus/upgrade/upgrade_downgrade_app_subnet_test.rs index 16cf2704d44b..c181ed0e091a 100644 --- a/rs/tests/consensus/upgrade/upgrade_downgrade_app_subnet_test.rs +++ b/rs/tests/consensus/upgrade/upgrade_downgrade_app_subnet_test.rs @@ -1,11 +1,7 @@ -use std::time::Duration; - use anyhow::Result; use futures::future::join_all; -use slog::Logger; -use tokio::runtime::{Builder, Runtime}; - use ic_consensus_system_test_upgrade_common::{ + ALLOWED_FAILURES, UP_DOWNGRADE_OVERALL_TIMEOUT, UP_DOWNGRADE_PER_TEST_TIMEOUT, elect_target_version, get_chain_key_canister_and_public_key, upgrade, }; use ic_consensus_system_test_utils::rw_message::{ @@ -32,14 +28,14 @@ use ic_system_test_driver::generic_workload_engine::metrics::{ use ic_system_test_driver::systest; use ic_system_test_driver::util::{MessageCanister, block_on, get_app_subnet_and_node}; use ic_types::Height; +use slog::Logger; use slog::info; +use std::time::Duration; +use tokio::runtime::{Builder, Runtime}; const SCHNORR_MSG_SIZE_BYTES: usize = 32; const DKG_INTERVAL: u64 = 29; -const ALLOWED_FAILURES: usize = 1; -const SUBNET_SIZE: usize = 3 * ALLOWED_FAILURES + 1; // 4 nodes -const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_secs(35 * 60); -const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_secs(30 * 60); +const SUBNET_SIZE: usize = 3 * ALLOWED_FAILURES + 1; const REQUESTS_DISPATCH_EXTRA_TIMEOUT: Duration = Duration::from_secs(1); fn setup(env: TestEnv) { diff --git a/rs/tests/consensus/upgrade/upgrade_downgrade_nns_subnet_test.rs b/rs/tests/consensus/upgrade/upgrade_downgrade_nns_subnet_test.rs index e8a9f3ae5141..7094be290387 100644 --- a/rs/tests/consensus/upgrade/upgrade_downgrade_nns_subnet_test.rs +++ b/rs/tests/consensus/upgrade/upgrade_downgrade_nns_subnet_test.rs @@ -1,8 +1,8 @@ -use std::time::Duration; - use anyhow::Result; - -use ic_consensus_system_test_upgrade_common::{elect_target_version, upgrade}; +use ic_consensus_system_test_upgrade_common::{ + ALLOWED_FAILURES, UP_DOWNGRADE_OVERALL_TIMEOUT, UP_DOWNGRADE_PER_TEST_TIMEOUT, + elect_target_version, upgrade, +}; use ic_consensus_system_test_utils::rw_message::{ can_read_msg_with_retries, install_nns_and_check_progress, }; @@ -19,10 +19,7 @@ use ic_types::Height; use slog::info; const DKG_INTERVAL: u64 = 9; -const ALLOWED_FAILURES: usize = 1; -const SUBNET_SIZE: usize = 3 * ALLOWED_FAILURES + 1; // 4 nodes -const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_secs(35 * 60); -const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_secs(30 * 60); +const SUBNET_SIZE: usize = 3 * ALLOWED_FAILURES + 1; fn setup(env: TestEnv) { let subnet_under_test = Subnet::new(SubnetType::System) diff --git a/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs b/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs index 92c095c1233f..2880837fadc1 100644 --- a/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs +++ b/rs/tests/consensus/upgrade/upgrade_downgrade_unassigned_nodes_test.rs @@ -22,11 +22,11 @@ Success:: end::catalog[] */ -use std::time::Duration; - use anyhow::Result; use anyhow::bail; -use ic_consensus_system_test_upgrade_common::elect_target_version; +use ic_consensus_system_test_upgrade_common::{ + UP_DOWNGRADE_OVERALL_TIMEOUT, UP_DOWNGRADE_PER_TEST_TIMEOUT, elect_target_version, +}; use ic_consensus_system_test_utils::{ rw_message::install_nns_and_check_progress, upgrade::{deploy_guestos_to_all_unassigned_nodes, fetch_unassigned_node_version}, @@ -42,15 +42,6 @@ use ic_types::ReplicaVersion; use slog::Logger; use slog::info; -// The test performs two sequential GuestOS upgrades of the unassigned node, -// each of which is polled with a 600s retry budget (see -// `upgrade_unassigned_nodes` below). The driver's default per-test timeout of -// 10 minutes cannot even cover a single exhausted poll, so give the test an -// explicit budget that covers its own worst case (2 x 600s polling + proposal -// handling + slack), like its siblings in this package do. -const UP_DOWNGRADE_OVERALL_TIMEOUT: Duration = Duration::from_secs(30 * 60); -const UP_DOWNGRADE_PER_TEST_TIMEOUT: Duration = Duration::from_secs(25 * 60); - fn setup(env: TestEnv) { InternetComputer::new() .add_fast_single_node_subnet(SubnetType::System)