perf: only check trace in tests if has a reason to (#1128)

.gitignore

@@ -26,7 +26,6 @@ share/python-wheels/
 .installed.cfg
 *.egg
 MANIFEST
-contracts/
 
 # PyInstaller
 #  Usually these files are written by a python script from a template

src/ape/api/providers.py

@@ -944,7 +944,17 @@ def send_transaction(self, txn: TransactionAPI) -> ReceiptAPI:
         try:
             txn_hash = self.web3.eth.send_raw_transaction(txn.serialize_transaction())
         except ValueError as err:
-            raise self.get_virtual_machine_error(err) from err
+            vm_err = self.get_virtual_machine_error(err)
+
+            if "nonce too low" in str(vm_err):
+                # Add additional nonce information
+                new_err_msg = f"Nonce '{txn.nonce}' is too low"
+                raise VirtualMachineError(
+                    base_err=vm_err.base_err, message=new_err_msg, code=vm_err.code
+                ) from err
+
+            else:
+                raise vm_err from err
 
         required_confirmations = (
             txn.required_confirmations

src/ape/pytest/fixtures.py

@@ -178,10 +178,14 @@ def capture(self, transaction_hash: str, track_gas: Optional[bool] = None):
             return
 
         self.receipt_map[source_id][transaction_hash] = receipt
+        do_track_gas = self.config_wrapper.track_gas if track_gas is None else track_gas
+
+        if not do_track_gas:
+            # Only capture trace if has a reason to.
+            return
 
         # Merge-in the receipt's gas report with everything so far.
         call_tree = receipt.call_tree
-        do_track_gas = self.config_wrapper.track_gas if track_gas is None else track_gas
         exclusions = self.config_wrapper.gas_exclusions
         if exclusions:
             contract_address = receipt.receiver

src/ape/pytest/plugin.py

@@ -68,7 +68,7 @@ def is_module(v):
     config.pluginmanager.register(session, "ape-test")
 
     # Include custom fixtures for project, accounts etc.
-    fixtures = PytestApeFixtures(config, receipt_capture)
+    fixtures = PytestApeFixtures(config_wrapper, receipt_capture)
     config.pluginmanager.register(fixtures, "ape-fixtures")
 
 

src/ape/pytest/runners.py

@@ -147,7 +147,7 @@ def pytest_collection_finish(self, session):
         outcome = yield
 
         # Only start provider if collected tests.
-        if not outcome.get_result() and session.items and not self.network_manager.active_provider:
+        if not outcome.get_result() and session.items:
             self._provider_context.push_provider()
             self._provider_is_connected = True
 
@@ -156,26 +156,28 @@ def pytest_terminal_summary(self, terminalreporter):
         Add a section to terminal summary reporting.
         When ``--gas`` is active, outputs the gas profile report.
         """
-        if self.config_wrapper.track_gas:
-            terminalreporter.section("Gas Profile")
-
-            if not self.provider.supports_tracing:
-                terminalreporter.write_line(
-                    f"{LogLevel.ERROR.name}: Provider '{self.provider.name}' does not support "
-                    f"transaction tracing and is unable to display a gas profile.",
-                    red=True,
-                )
-                return
-
-            gas_report = self.receipt_capture.gas_report
-            if gas_report:
-                tables = parse_gas_table(gas_report)
-                rich_print(*tables)
-            else:
-
-                terminalreporter.write_line(
-                    f"{LogLevel.WARNING.name}: No gas usage data found.", yellow=True
-                )
+        if not self.config_wrapper.track_gas:
+            return
+
+        terminalreporter.section("Gas Profile")
+
+        if not self.provider.supports_tracing:
+            terminalreporter.write_line(
+                f"{LogLevel.ERROR.name}: Provider '{self.provider.name}' does not support "
+                f"transaction tracing and is unable to display a gas profile.",
+                red=True,
+            )
+            return
+
+        gas_report = self.receipt_capture.gas_report
+        if gas_report:
+            tables = parse_gas_table(gas_report)
+            rich_print(*tables)
+        else:
+
+            terminalreporter.write_line(
+                f"{LogLevel.WARNING.name}: No gas usage data found.", yellow=True
+            )
 
     def pytest_unconfigure(self):
         if self._provider_is_connected and self.config_wrapper.disconnect_providers_after:

src/ape_geth/provider.py

@@ -36,6 +36,7 @@
 from ape.utils import generate_dev_accounts, raises_not_implemented
 
 DEFAULT_SETTINGS = {"uri": "http://localhost:8545"}
+GETH_DEV_CHAIN_ID = 1337
 
 
 class GethDevProcess(LoggingMixin, BaseGethProcess):
@@ -50,7 +51,7 @@ def __init__(
         port: int,
         mnemonic: str,
         number_of_accounts: PositiveInt,
-        chain_id: int = 1337,
+        chain_id: int = GETH_DEV_CHAIN_ID,
         initial_balance: Union[str, int] = to_wei(10000, "ether"),
     ):
         self.data_dir = base_directory / "dev"
@@ -61,6 +62,11 @@ def __init__(
             rpc_addr=hostname,
             rpc_port=port,
             network_id=chain_id,
+            ws_enabled=False,
+            ws_addr=None,
+            ws_origins=None,
+            ws_port=None,
+            ws_api=None,
         )
 
         # Ensure a clean data-dir.
@@ -87,6 +93,7 @@ def __init__(
                 "istanbulBlock": 0,
                 "berlinBlock": 0,
                 "londonBlock": 0,
+                "parisBlock": 0,
                 "clique": {"period": 0, "epoch": 30000},
             },
             "alloc": {a.address: {"balance": str(initial_balance)} for a in accounts},
@@ -305,6 +312,17 @@ class GethDev(TestProviderAPI, BaseGethProvider):
     _process: Optional[GethDevProcess] = None
     name: str = "geth"
 
+    @property
+    def chain_id(self) -> int:
+        return GETH_DEV_CHAIN_ID
+
+    def __repr__(self):
+        if self._process is None:
+            # Exclude chain ID when not connected
+            return "<geth>"
+
+        return super().__repr__()
+
     def connect(self):
         self._set_web3()
         if not self.is_connected:
@@ -349,18 +367,41 @@ def disconnect(self):
 
         super().disconnect()
 
+    @raises_not_implemented
+    def snapshot(self) -> SnapshotID:
+        # TODO: Replace with impl below after
+        #  https://github.com/ethereum/go-ethereum/issues/26154 resolved
+        pass
+
+    def _snapshot(self) -> SnapshotID:
+        return self.get_block("latest").number or 0
+
+    @raises_not_implemented
     def revert(self, snapshot_id: SnapshotID):
+        # TODO: Replace with impl below after
+        #  https://github.com/ethereum/go-ethereum/issues/26154 resolved
+        pass
+
+    def _revert(self, snapshot_id: SnapshotID):
         if isinstance(snapshot_id, int):
-            block_number = str(to_hex(snapshot_id))
+            block_number_int = snapshot_id
+            block_number_hex_str = str(to_hex(snapshot_id))
         elif isinstance(snapshot_id, bytes):
-            block_number = str(add_0x_prefix(HexStr(snapshot_id.hex())))
+            block_number_hex_str = add_0x_prefix(HexStr(snapshot_id.hex()))
+            block_number_int = int(block_number_hex_str, 16)
         else:
-            block_number = str(snapshot_id)
-
-        self._make_request("debug_setHead", [block_number])
-
-    def snapshot(self) -> SnapshotID:
-        return self.get_block("latest").number or 0
+            block_number_hex_str = add_0x_prefix(HexStr(snapshot_id))
+            block_number_int = int(snapshot_id, 16)
+
+        current_block = self.get_block("latest").number
+        if block_number_int == current_block:
+            # Head is already at this block.
+            return
+        elif block_number_int > block_number_int:
+            logger.error("Unable to set head to future block.")
+            return
+
+        self._make_request("debug_setHead", [block_number_hex_str])
 
     @raises_not_implemented
     def set_timestamp(self, new_timestamp: int):

tests/conftest.py

@@ -20,6 +20,7 @@
 # Needed to test tracing support in core `ape test` command.
 pytest_plugins = ["pytester"]
 geth_process_test = pytest.mark.xdist_group(name="geth-tests")
+GETH_URI = "http://127.0.0.1:5550"
 
 
 @pytest.fixture(autouse=True)
@@ -129,45 +130,83 @@ def runner():
     yield CliRunner()
 
 
-@pytest.fixture(scope="session")
-def networks_connected_to_tester():
-    with ape.networks.parse_network_choice("::test"):
+@pytest.fixture
+def networks_disconnected():
+    provider = ape.networks.active_provider
+    ape.networks.active_provider = None
+
+    try:
         yield ape.networks
+    finally:
+        ape.networks.active_provider = provider
 
 
 @pytest.fixture
-def networks_disconnected(networks):
-    provider = networks.active_provider
-    networks.active_provider = None
-    yield networks
-    networks.active_provider = provider
+def ethereum(networks):
+    return networks.ethereum
+
+
+@pytest.fixture(autouse=True)
+def eth_tester_provider():
+    if not ape.networks.active_provider or ape.networks.provider.name != "test":
+        with ape.networks.ethereum.local.use_provider("test") as provider:
+            yield provider
+    else:
+        yield ape.networks.provider
 
 
 @pytest.fixture
-def ethereum(networks):
-    return networks.ethereum
+def networks_connected_to_tester(eth_tester_provider):
+    return eth_tester_provider.network_manager
 
 
-@pytest.fixture(scope="session")
-def eth_tester_provider(networks_connected_to_tester):
-    yield networks_connected_to_tester.provider
+@pytest.fixture
+def geth_provider(networks):
+    if not networks.active_provider or networks.provider.name != "geth":
+        with networks.ethereum.local.use_provider(
+            "geth", provider_settings={"uri": GETH_URI}
+        ) as provider:
+            yield provider
+    else:
+        yield networks.provider
 
 
-@pytest.fixture(autouse=True)
-def isolation(chain, eth_tester_provider):
+@contextmanager
+def _isolation():
+    if ape.networks.active_provider is None:
+        raise AssertionError("Isolation should only be used with a connected provider.")
+
+    init_network_name = ape.chain.provider.network.name
+    init_provider_name = ape.chain.provider.name
+
     try:
-        snapshot = chain.snapshot()
+        snapshot = ape.chain.snapshot()
     except APINotImplementedError:
+        # Provider not used or connected in test.
         snapshot = None
 
     yield
 
-    if snapshot is not None:
-        try:
-            chain.restore(snapshot)
-        except UnknownSnapshotError:
-            # Assume snapshot removed for testing reasons
-            pass
+    if (
+        snapshot is None
+        or ape.networks.active_provider is None
+        or ape.chain.provider.network.name != init_network_name
+        or ape.chain.provider.name != init_provider_name
+    ):
+        return
+
+    try:
+        ape.chain.restore(snapshot)
+    except UnknownSnapshotError:
+        # Assume snapshot removed for testing reasons
+        # or the provider was not needed to be connected for the test.
+        pass
+
+
+@pytest.fixture(autouse=True)
+def eth_tester_isolation(eth_tester_provider):
+    with _isolation():
+        yield
 
 
 @pytest.fixture(scope="session")

tests/functional/conftest.py

@@ -1,6 +1,7 @@
 import json
 import threading
 import time
+from contextlib import contextmanager
 from distutils.dir_util import copy_tree
 from pathlib import Path
 from typing import Dict, Optional
@@ -87,22 +88,32 @@ def owner(test_accounts):
     return test_accounts[2]
 
 
-@pytest.fixture(scope="session")
-def keyfile_account(sender, keyparams, temp_accounts_path, eth_tester_provider):
-    test_keyfile_path = temp_accounts_path / f"{ALIAS}.json"
-    yield _make_keyfile_account(temp_accounts_path, ALIAS, keyparams, sender)
+@contextmanager
+def _temp_keyfile_account(base_path: Path, alias: str, keyparams, sender):
+    test_keyfile_path = base_path / f"{alias}.json"
 
-    if test_keyfile_path.is_file():
-        test_keyfile_path.unlink()
+    if not test_keyfile_path.is_file():
+        account = _make_keyfile_account(base_path, alias, keyparams, sender)
+    else:
+        account = ape.accounts.load(ALIAS)
 
+    try:
+        yield account
+    finally:
+        if test_keyfile_path.is_file():
+            test_keyfile_path.unlink()
 
-@pytest.fixture(scope="session")
-def second_keyfile_account(sender, keyparams, temp_accounts_path, eth_tester_provider):
-    test_keyfile_path = temp_accounts_path / f"{ALIAS_2}.json"
-    yield _make_keyfile_account(temp_accounts_path, ALIAS_2, keyparams, sender)
 
-    if test_keyfile_path.is_file():
-        test_keyfile_path.unlink()
+@pytest.fixture
+def keyfile_account(sender, keyparams, temp_accounts_path):
+    with _temp_keyfile_account(temp_accounts_path, ALIAS, keyparams, sender) as account:
+        yield account
+
+
+@pytest.fixture
+def second_keyfile_account(sender, keyparams, temp_accounts_path):
+    with _temp_keyfile_account(temp_accounts_path, ALIAS_2, keyparams, sender) as account:
+        yield account
 
 
 def _make_keyfile_account(base_path: Path, alias: str, params: Dict, funder):
@@ -161,12 +172,14 @@ def contract_container(
 
 
 @pytest.fixture(params=("solidity", "vyper"))
-def contract_instance(request, solidity_contract_instance, vyper_contract_instance):
+def contract_instance(
+    eth_tester_provider, request, solidity_contract_instance, vyper_contract_instance
+):
     return solidity_contract_instance if request.param == "solidity" else vyper_contract_instance
 
 
 @pytest.fixture
-def ds_note_test_contract(vyper_contract_type, owner, eth_tester_provider):
+def ds_note_test_contract(eth_tester_provider, vyper_contract_type, owner):
     contract_type = ContractType.parse_raw(DS_NOTE_TEST_CONTRACT_TYPE)
     contract_container = ContractContainer(contract_type=contract_type)
     return contract_container.deploy(sender=owner)