From 3855bf9606c7d1f168aa6e1b453fcc8c7feb3e1f Mon Sep 17 00:00:00 2001 From: antazoey Date: Sun, 12 Jan 2025 21:14:02 -0600 Subject: [PATCH] fix: test-account negative index did not refer to correct account (#2444) Co-authored-by: antazoey --- src/ape/managers/accounts.py | 21 ++++++++++------ src/ape_test/accounts.py | 5 ++++ tests/functional/test_accounts.py | 41 +++++++++++++++++++++++++------ 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/ape/managers/accounts.py b/src/ape/managers/accounts.py index e45939204e..69ebc31906 100644 --- a/src/ape/managers/accounts.py +++ b/src/ape/managers/accounts.py @@ -41,8 +41,7 @@ class TestAccountManager(list, ManagerAccessMixin): @log_instead_of_fail(default="") def __repr__(self) -> str: - accounts_str = ", ".join([a.address for a in self.accounts]) - return f"[{accounts_str}]" + return f"" @cached_property def containers(self) -> dict[str, TestAccountContainerAPI]: @@ -54,6 +53,13 @@ def containers(self) -> dict[str, TestAccountContainerAPI]: for plugin_name, (container_type, account_type) in account_types } + @property + def hd_path(self) -> str: + """ + The HD path used for generating the test accounts. + """ + return self.config_manager.get_config("test").hd_path + @property def accounts(self) -> Iterator[AccountAPI]: for container in self.containers.values(): @@ -76,15 +82,14 @@ def __getitem__(self, account_id): @__getitem__.register def __getitem_int(self, account_id: int): - if account_id in self._accounts_by_index: - return self._accounts_by_index[account_id] - - original_account_id = account_id if account_id < 0: account_id = len(self) + account_id + if account_id in self._accounts_by_index: + return self._accounts_by_index[account_id] + account = self.containers["test"].get_test_account(account_id) - self._accounts_by_index[original_account_id] = account + self._accounts_by_index[account_id] = account return account @__getitem__.register @@ -265,7 +270,7 @@ def __iter__(self) -> Iterator[AccountAPI]: @log_instead_of_fail(default="") def __repr__(self) -> str: - return "[" + ", ".join(repr(a) for a in self) + "]" + return "" @cached_property def test_accounts(self) -> TestAccountManager: diff --git a/src/ape_test/accounts.py b/src/ape_test/accounts.py index a0abea03a5..d14f09fcbd 100644 --- a/src/ape_test/accounts.py +++ b/src/ape_test/accounts.py @@ -14,6 +14,7 @@ from ape.exceptions import ProviderNotConnectedError, SignatureError from ape.types.signatures import MessageSignature, TransactionSignature from ape.utils._web3_compat import sign_hash +from ape.utils.misc import log_instead_of_fail from ape.utils.testing import ( DEFAULT_NUMBER_OF_TEST_ACCOUNTS, DEFAULT_TEST_HD_PATH, @@ -113,6 +114,10 @@ def alias(self) -> str: def address(self) -> "AddressType": return self.network_manager.ethereum.decode_address(self.address_str) + @log_instead_of_fail(default="") + def __repr__(self) -> str: + return f"<{self.__class__.__name__}_{self.index} {self.address_str}>" + def sign_message(self, msg: Any, **signer_options) -> Optional[MessageSignature]: # Convert str and int to SignableMessage if needed if isinstance(msg, str): diff --git a/tests/functional/test_accounts.py b/tests/functional/test_accounts.py index 9f330de27f..a66c8a02ae 100644 --- a/tests/functional/test_accounts.py +++ b/tests/functional/test_accounts.py @@ -412,12 +412,19 @@ def test_send_transaction_sets_defaults(sender, receiver): assert receipt.required_confirmations == 0 +def test_account_index_access(accounts): + account = accounts[0] + assert account.index == 0 + last_account = accounts[-1] + assert last_account.index == len(accounts) - 1 + + def test_accounts_splice_access(accounts): - a, b = accounts[:2] - assert a == accounts[0] - assert b == accounts[1] - c = accounts[-1] - assert c == accounts[len(accounts) - 1] + alice, bob = accounts[:2] + assert alice == accounts[0] + assert bob == accounts[1] + cat = accounts[-1] + assert cat == accounts[len(accounts) - 1] expected = (len(accounts) // 2) if len(accounts) % 2 == 0 else (len(accounts) // 2 + 1) assert len(accounts[::2]) == expected @@ -612,9 +619,9 @@ def test_custom_num_of_test_accounts_config(accounts, project): assert len(accounts) == custom_number_of_test_accounts -def test_test_accounts_repr(accounts): +def test_test_accounts_repr(accounts, config): actual = repr(accounts) - assert all(a.address in actual for a in accounts) + assert config.get_config("test").hd_path in actual def test_account_comparison_to_non_account(core_account): @@ -629,11 +636,20 @@ def test_create_account(accounts): assert isinstance(created_account, TestAccount) assert created_account.index == length_at_start + length_at_start = len(accounts) second_created_account = accounts.generate_test_account() + assert len(accounts) == length_at_start + 1 assert created_account.address != second_created_account.address assert second_created_account.index == created_account.index + 1 + # Last index should now refer to the last-created account. + last_idx_acct = accounts[-1] + assert last_idx_acct.index == second_created_account.index + assert last_idx_acct.address == second_created_account.address + assert last_idx_acct.address != accounts[0].address + assert last_idx_acct.address != created_account.address + def test_dir(core_account): actual = dir(core_account) @@ -951,3 +967,14 @@ def test_get_deployment_address(owner, vyper_contract_container): assert instance_1.address == deployment_address_1 instance_2 = owner.deploy(vyper_contract_container, 490) assert instance_2.address == deployment_address_2 + + +def test_repr(account_manager): + """ + NOTE: __repr__ should be simple and fast! + Previously, we showed the repr of all the accounts. + That was a bad idea, as that can be very unnecessarily slow. + Hence, this test exists to ensure care is taken. + """ + actual = repr(account_manager) + assert actual == ""