From 3ef56821897fb5ecc353d1e05178efd2badf7ec7 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 5 Oct 2026 16:11:20 +0200 Subject: [PATCH] fix(tests): avoid races and leaked state in tests - test_tx_chaining: lock the whole cluster instance. The test checks how many blocks the chained Txs take, so Txs of other tests must not take the block space, and the chained Txs fill the mempool for other tests. - Plutus script stake address tests: respin the cluster instance when a test leaves the script stake address registered, otherwise all the following tests using the script are skipped. - KES negative tests: take a pool marked as "in use" instead of a hardcoded one, and pass a copy of its cold counter file to `issue-op-cert`, as the command would increment the shared counter if it succeeded. - test_rotation_pair_keeps_voting: check the epoch length in the fixture, before the respin is requested. - test_permanent_fork: request the respin before splitting the cluster, so it is requested even when the split fails half-way. - set_network_id_env: restore the original `CARDANO_NODE_NETWORK_ID` value instead of deleting it. --- .../tests/test_chain_transactions.py | 10 ++++++- .../tests/test_env_network_id.py | 19 ++++++------- cardano_node_tests/tests/test_kes.py | 28 +++++++++++++------ cardano_node_tests/tests/test_rollback.py | 7 +++-- .../tests/tests_dijkstra/test_bls_rotation.py | 7 +++-- .../tests/tests_plutus/test_delegation.py | 15 ++++++++-- 6 files changed, 57 insertions(+), 29 deletions(-) diff --git a/cardano_node_tests/tests/test_chain_transactions.py b/cardano_node_tests/tests/test_chain_transactions.py index f0340f479..36806576b 100644 --- a/cardano_node_tests/tests/test_chain_transactions.py +++ b/cardano_node_tests/tests/test_chain_transactions.py @@ -11,6 +11,7 @@ from cardano_node_tests.cluster_management import cluster_management from cardano_node_tests.tests import addrs_common from cardano_node_tests.tests import common +from cardano_node_tests.tests import markers from cardano_node_tests.utils import configuration from cardano_node_tests.utils import dbsync_utils from cardano_node_tests.utils import helpers @@ -97,11 +98,18 @@ def _repeat_submit(cluster_obj: clusterlib.ClusterLib, tx_file: pl.Path) -> str: class TestTxChaining: @pytest.fixture def cluster(self, cluster_manager: cluster_management.ClusterManager) -> clusterlib.ClusterLib: + """Lock the whole cluster instance. + + The test checks how many blocks the chained Txs take, so Txs of other tests must not + take the block space. The Txs also fill the mempool, which would slow down the + submission of Txs of other tests. + """ return cluster_manager.get( - lock_resources=[cluster_management.Resources.PERF], + lock_resources=[cluster_management.Resources.CLUSTER], ) @allure.link(helpers.get_vcs_link()) + @pytest.mark.xdist_split(markers.XdSplits.heavy) @pytest.mark.dbsync def test_tx_chaining( self, diff --git a/cardano_node_tests/tests/test_env_network_id.py b/cardano_node_tests/tests/test_env_network_id.py index 4c80c4d00..003efa410 100644 --- a/cardano_node_tests/tests/test_env_network_id.py +++ b/cardano_node_tests/tests/test_env_network_id.py @@ -4,7 +4,6 @@ import os import re import time -import typing as tp import allure import pytest @@ -82,16 +81,14 @@ def skip_on_no_env( @pytest.fixture def set_network_id_env( cluster: clusterlib.ClusterLib, -) -> tp.Generator[None]: - """Set `CARDANO_NODE_NETWORK_ID` and prevent `cardano-cli` from using `--testnet-magic`.""" - magic_args = cluster.magic_args[:] - - os.environ["CARDANO_NODE_NETWORK_ID"] = str(cluster.network_magic) - cluster.magic_args = [] - yield - if os.environ.get("CARDANO_NODE_NETWORK_ID"): - del os.environ["CARDANO_NODE_NETWORK_ID"] - cluster.magic_args = magic_args + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Set `CARDANO_NODE_NETWORK_ID` and prevent `cardano-cli` from using `--testnet-magic`. + + The original values are restored after the test, also when the test changed them. + """ + monkeypatch.setenv("CARDANO_NODE_NETWORK_ID", str(cluster.network_magic)) + monkeypatch.setattr(cluster, "magic_args", []) @pytest.fixture diff --git a/cardano_node_tests/tests/test_kes.py b/cardano_node_tests/tests/test_kes.py index e16bcf22b..4fbcfd9d8 100644 --- a/cardano_node_tests/tests/test_kes.py +++ b/cardano_node_tests/tests/test_kes.py @@ -772,7 +772,7 @@ def test_update_valid_opcert( @allure.link(helpers.get_vcs_link()) def test_no_kes_period_arg( self, - cluster: clusterlib.ClusterLib, + cluster_use_pool: tuple[clusterlib.ClusterLib, str], cluster_manager: cluster_management.ClusterManager, ): """Try to generate new operational certificate without specifying the `--kes-period`. @@ -780,17 +780,22 @@ def test_no_kes_period_arg( Test that operational certificate generation fails when required `--kes-period` argument is not provided. Expect failure. - * Get pool KES verification key, cold signing key, and counter file from pool2 + * Get pool KES verification key, cold signing key, and a copy of counter file * Attempt to execute `cardano-cli node issue-op-cert` without --kes-period argument * Check that command fails with "Missing: --kes-period NATURAL" error * Verify operational certificate file was not created """ - pool_name = cluster_management.Resources.POOL2 + cluster, pool_name = cluster_use_pool pool_rec = cluster_manager.cache.addrs_data[pool_name] temp_template = common.get_test_id(cluster) out_file = pl.Path(f"{temp_template}_shouldnt_exist.opcert") + # The command would increment the counter if it succeeded, so don't pass it the shared + # counter file of the cluster pool + counter_file = pl.Path(f"{temp_template}_cold.counter") + shutil.copyfile(pool_rec["cold_key_pair"].counter_file, counter_file) + # Try to generate new operational certificate without specifying the `--kes-period` with pytest.raises(clusterlib.CLIError) as excinfo: cluster.cli( @@ -802,7 +807,7 @@ def test_no_kes_period_arg( "--cold-signing-key-file", str(pool_rec["cold_key_pair"].skey_file), "--operational-certificate-issue-counter", - str(pool_rec["cold_key_pair"].counter_file), + str(counter_file), "--out-file", str(out_file), ] @@ -817,7 +822,7 @@ def test_no_kes_period_arg( @pytest.mark.smoke def test_negative_kes_period_arg( self, - cluster: clusterlib.ClusterLib, + cluster_use_pool: tuple[clusterlib.ClusterLib, str], cluster_manager: cluster_management.ClusterManager, ): """Try to generate new operational certificate with a negative value for `--kes-period`. @@ -825,17 +830,22 @@ def test_negative_kes_period_arg( Test that operational certificate generation fails when --kes-period is set to a negative value. Expect failure. - * Get pool KES verification key, cold signing key, and counter file from pool2 + * Get pool KES verification key, cold signing key, and a copy of counter file * Attempt to generate operational certificate with --kes-period set to -100 * Check that generation fails with "KES_PERIOD must not be less than 0" error """ - common.get_test_id(cluster) + cluster, pool_name = cluster_use_pool + temp_template = common.get_test_id(cluster) - pool_name = cluster_management.Resources.POOL2 pool_rec = cluster_manager.cache.addrs_data[pool_name] node_name = pool_name.replace("node-", "") + # The command would increment the counter if it succeeded, so don't pass it the shared + # counter file of the cluster pool + counter_file = pl.Path(f"{temp_template}_cold.counter") + shutil.copyfile(pool_rec["cold_key_pair"].counter_file, counter_file) + # Generate new operational certificate with negative value for `--kes-period` invalid_kes_period = -100 @@ -844,7 +854,7 @@ def test_negative_kes_period_arg( node_name=f"{node_name}_invalid_opcert_file", kes_vkey_file=pool_rec["kes_key_pair"].vkey_file, cold_skey_file=pool_rec["cold_key_pair"].skey_file, - cold_counter_file=pool_rec["cold_key_pair"].counter_file, + cold_counter_file=counter_file, kes_period=invalid_kes_period, ) except clusterlib.CLIError as exc: diff --git a/cardano_node_tests/tests/test_rollback.py b/cardano_node_tests/tests/test_rollback.py index 6eb5926cc..f0fde6871 100644 --- a/cardano_node_tests/tests/test_rollback.py +++ b/cardano_node_tests/tests/test_rollback.py @@ -399,12 +399,13 @@ def test_permanent_fork( split_block = cluster.g_query.get_block_no() + 10 final_block = split_block + cluster.genesis["securityParam"] + # The cluster needs respin after this point. Request the respin before the split, so + # it is requested even when the split fails half-way. + cluster_manager.set_needs_respin() + # Split the cluster into two separate clusters self.split_cluster(split_topology_dir=split_topology_dir) - # The cluster needs respin after this point - cluster_manager.set_needs_respin() - # Submit a Tx number 2 on the first cluster tx_outputs.append( self.node_submit_tx( diff --git a/cardano_node_tests/tests/tests_dijkstra/test_bls_rotation.py b/cardano_node_tests/tests/tests_dijkstra/test_bls_rotation.py index 885b31ed1..d7f48ea08 100644 --- a/cardano_node_tests/tests/tests_dijkstra/test_bls_rotation.py +++ b/cardano_node_tests/tests/tests_dijkstra/test_bls_rotation.py @@ -188,6 +188,10 @@ def cluster_leios_singleton( f"the {epoch_tail_sec} sec search window" ) + # The test waits for up to 6.5 epochs: up to `leios.VOTING_START_EPOCH` epochs, then + # `bls.BLS_ACTIVATION_EPOCHS` epochs, and ~20 min of log searches and node restarts + common.skip_on_long_epochs(cluster_obj=cluster_obj, epochs=6.5) + cluster_manager.set_needs_respin() return cluster_obj @@ -1258,9 +1262,6 @@ def test_rotation_pair_keeps_voting( the rotation is seated - and check that the pool votes again """ cluster = cluster_leios_singleton - # Waits for up to 6.5 epochs: up to `leios.VOTING_START_EPOCH` epochs, then - # `bls.BLS_ACTIVATION_EPOCHS` epochs, and ~20 min of log searches and node restarts - common.skip_on_long_epochs(cluster_obj=cluster, epochs=6.5) temp_template = common.get_test_id(cluster) pool_name = cluster_management.Resources.POOL_FOR_OFFLINE diff --git a/cardano_node_tests/tests/tests_plutus/test_delegation.py b/cardano_node_tests/tests/tests_plutus/test_delegation.py index 590603986..6d75e3947 100644 --- a/cardano_node_tests/tests/tests_plutus/test_delegation.py +++ b/cardano_node_tests/tests/tests_plutus/test_delegation.py @@ -44,11 +44,15 @@ @pytest.fixture def cluster_lock_42stake( cluster_manager: cluster_management.ClusterManager, -) -> tuple[clusterlib.ClusterLib, str]: +) -> tp.Generator[tuple[clusterlib.ClusterLib, str]]: """Make sure just one staking Plutus test run at a time. Plutus script always has the same address. When one script is used in multiple tests that are running in parallel, the balances etc. don't add up. + + When a test fails before deregistering the Plutus script stake address, the address + stays registered and all the following tests using the script would be skipped. + Respin the cluster instance in such case. """ plutus_script = ( plutus_common.STAKE_PLUTUS_V2 @@ -72,7 +76,14 @@ def cluster_lock_42stake( addrs_data=cluster_manager.cache.addrs_data, pool_name=pool_name, ) - return cluster_obj, pool_id + yield cluster_obj, pool_id + + script_stake_address = cluster_obj.g_stake_address.gen_stake_addr( + addr_name=f"script_stake_check_{clusterlib.get_rand_str(4)}", + stake_script_file=plutus_script, + ) + if cluster_obj.g_query.get_stake_addr_info(script_stake_address): + cluster_manager.set_needs_respin() @pytest.fixture