mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-08-25 20:41:21 +02:00
Merge bitcoin/bitcoin#33104: test: Perform backup filename checks in migrate_and_get_rpc in wallet_migration.py
4b80147feb
test: Perform backup filename checks in migrate_and_get_rpc (Ava Chow) Pull request description: Some test cases were unnecessarily checking the backup filename, which involved setting the mocktime before `migrate_and_get_rpc`. However, this could cause a failure if the test was slow since `migrate_and_get_rpc` also sets the mocktime. Since it also already checks that the backup file is named correctly, there's no need for those tests to also do their own mocktime and filename check. The CI failure can be reproduced locally by adding a sleep to `migrate_and_get_rpc`: ```diff diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 704204425c7..e87a6100623 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -129,6 +129,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(w["warnings"], ["This wallet is a legacy wallet and will need to be migrated with migratewallet before it can be loaded"]) # Mock time so that we can check the backup filename. + time.sleep(1) mocked_time = int(time.time()) self.master_node.setmocktime(mocked_time) # Migrate, checking that rescan does not occur ``` Fixes #33096 ACKs for top commit: fjahr: reACK4b80147feb
Sammie05: tACK4b80147
pablomartin4btc: utACK4b80147feb
rkrux: ACK4b80147feb
Tree-SHA512: 045d4acf2ad0b56a7083ff2ee5ef09f0d74ad097c01a290660daca096c71fc07109848024256d84f74abbc87dd52691d160f9968b3654726626d3dbd21a84ab6
This commit is contained in:
@@ -128,7 +128,8 @@ class WalletMigrationTest(BitcoinTestFramework):
|
||||
if w["name"] == wallet_name:
|
||||
assert_equal(w["warnings"], ["This wallet is a legacy wallet and will need to be migrated with migratewallet before it can be loaded"])
|
||||
|
||||
# Mock time so that we can check the backup filename.
|
||||
# migratewallet uses current time in naming the backup file, set a mock time
|
||||
# to check that this works correctly.
|
||||
mocked_time = int(time.time())
|
||||
self.master_node.setmocktime(mocked_time)
|
||||
# Migrate, checking that rescan does not occur
|
||||
@@ -148,8 +149,10 @@ class WalletMigrationTest(BitcoinTestFramework):
|
||||
else:
|
||||
backup_prefix = os.path.basename(os.path.realpath(self.old_node.wallets_path / wallet_name))
|
||||
|
||||
expected_backup_path = self.master_node.wallets_path / f"{backup_prefix}_{mocked_time}.legacy.bak"
|
||||
backup_filename = f"{backup_prefix}_{mocked_time}.legacy.bak"
|
||||
expected_backup_path = self.master_node.wallets_path / backup_filename
|
||||
assert_equal(str(expected_backup_path), migrate_info['backup_path'])
|
||||
assert {"name": backup_filename} not in self.master_node.listwalletdir()["wallets"]
|
||||
|
||||
return migrate_info, wallet
|
||||
|
||||
@@ -593,12 +596,7 @@ class WalletMigrationTest(BitcoinTestFramework):
|
||||
self.generate(self.master_node, 1)
|
||||
bals = wallet.getbalances()
|
||||
|
||||
# migratewallet uses current time in naming the backup file, set a mock time
|
||||
# to check that this works correctly.
|
||||
curr_time = int(time.time())
|
||||
self.master_node.setmocktime(curr_time)
|
||||
migrate_res, wallet = self.migrate_and_get_rpc(relative_name)
|
||||
self.master_node.setmocktime(0)
|
||||
|
||||
# Check that the wallet was migrated, knows the right txid, and has the right balance.
|
||||
assert wallet.gettransaction(txid)
|
||||
@@ -665,12 +663,7 @@ class WalletMigrationTest(BitcoinTestFramework):
|
||||
self.log.info("Test migration of the wallet named as the empty string")
|
||||
wallet = self.create_legacy_wallet("")
|
||||
|
||||
# Set time to verify backup existence later
|
||||
curr_time = int(time.time())
|
||||
self.master_node.setmocktime(curr_time)
|
||||
|
||||
res, wallet = self.migrate_and_get_rpc("")
|
||||
self.master_node.setmocktime(0)
|
||||
info = wallet.getwalletinfo()
|
||||
assert_equal(info["descriptors"], True)
|
||||
assert_equal(info["format"], "sqlite")
|
||||
@@ -678,14 +671,9 @@ class WalletMigrationTest(BitcoinTestFramework):
|
||||
walletdir_list = wallet.listwalletdir()
|
||||
assert {"name": info["walletname"]} in [{"name": w["name"]} for w in walletdir_list["wallets"]]
|
||||
|
||||
# Check backup existence and its non-empty wallet filename
|
||||
backup_filename = f"default_wallet_{curr_time}.legacy.bak"
|
||||
backup_path = self.master_node.wallets_path / backup_filename
|
||||
assert backup_path.exists()
|
||||
assert_equal(str(backup_path), res['backup_path'])
|
||||
assert {"name": backup_filename} not in walletdir_list["wallets"]
|
||||
|
||||
self.master_node.setmocktime(0)
|
||||
# Make sure the backup uses a non-empty filename
|
||||
# migrate_and_get_rpc already checks for backup file existence
|
||||
assert os.path.basename(res["backup_path"]).startswith("default_wallet")
|
||||
|
||||
def test_direct_file(self):
|
||||
self.log.info("Test migration of a wallet that is not in a wallet directory")
|
||||
|
Reference in New Issue
Block a user