Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion src/ape/api/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/ape/pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/ape/pytest/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand Down
44 changes: 23 additions & 21 deletions src/ape/pytest/runners.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand Down
59 changes: 50 additions & 9 deletions src/ape_geth/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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"
Expand All @@ -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.
Expand All @@ -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},
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
antazoey marked this conversation as resolved.
Show resolved Hide 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):
Expand Down
83 changes: 61 additions & 22 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
41 changes: 27 additions & 14 deletions tests/functional/conftest.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
Loading