tests: add specific exceptions on tests #1286
Conversation
tests/helpers/load_state_tests.py
Outdated
| from ethereum.utils.hexadecimal import hex_to_bytes | ||
| from ethereum_spec_tools.evm_tools.loaders.fixture_loader import Load | ||
|
|
||
| FORK_SPECIFIC_EXCEPTIONS = { |
There was a problem hiding this comment.
is there a better/preferred way to do this instead?
src/ethereum/exceptions.py
Outdated
|
|
||
| class NonceTooHighError(InvalidTransaction): | ||
| """ | ||
| Thrown when a transaction's nonce is higher than 64 bits. |
There was a problem hiding this comment.
| Thrown when a transaction's nonce is higher than 64 bits. | |
| Thrown when a transaction's nonce is greater than `2**64 - 2`. |
I believe this is more correct?
src/ethereum/exceptions.py
Outdated
| """ | ||
|
|
||
|
|
||
| class NonceTooHighError(InvalidTransaction): |
There was a problem hiding this comment.
| class NonceTooHighError(InvalidTransaction): | |
| class NonceOverflowError(InvalidTransaction): |
src/ethereum/prague/transactions.py
Outdated
| if the transaction is invalid. | ||
| validation. It throws an `InsufficientTransactionGasError` exception if | ||
| the transaction does not provide enough gas to cover the intrinsic cost, | ||
| and a `NonceTooHighError` exception if the nonce is higher than 64 bits. |
There was a problem hiding this comment.
Hm, is this technically correct? We use a >= in the condition, so I think the docstring is off by one.
There was a problem hiding this comment.
We try really hard to avoid fork-specific code where we can. Instead we rely on the Hardfork class to do the heavy lifting.
You could maybe use module('exceptions') to load the fork-specific module, then iterate through the classes defined in it?
There was a problem hiding this comment.
thanks, updated it!
tests/helpers/load_state_tests.py
Outdated
| ) | ||
| all_exceptions = common_exceptions + fork_specific_exceptions | ||
|
|
||
| with pytest.raises(all_exceptions): |
There was a problem hiding this comment.
I believe the JSON fixture contains a string representing the expected exception. Is it possible to map that to exactly one exception type?
There was a problem hiding this comment.
I found that the block_exceptions from legacy tests/blockchain test fixtures are very different from the state transition exceptions. (and the state_transition method from the fork is used to raise the error caught here).
This specific function also seems to work with blockchaintests fixtures so couldn't account for the expected exception from state_tests. If i'm thinking wrong about this - do lmk!
…MAX_CODE_SIZE across multiple modules
update exceptions for other forks fix tests use fork module rename to NonceOverFlowError revert load test catching specific exc style: indent comment line
SamWilsn
force-pushed
the
souradeep/exc_tests
branch
from
7cf1a56 to
3385821
Compare
What was wrong?
Related to Issue #1021
How was it fixed?
InvalidTransactionon pytest.raises with specific exception typeInsufficientTransactionGasError,NonceTooHighErrorandInitCodeTooLargeError) for transaction validation, extendingInvalidTransactionCute Animal Picture