ethereum / eth-abi

Ethereum ABI utilities for python
MIT License
242 stars 269 forks source link

Maybe use a registry fixture to test custom registrations #77

Open davesque opened 6 years ago

davesque commented 6 years ago

What was wrong?

In the following integration tests that test custom registrations, our method of testing is a bit stateful since it registers then unregisters coders for the custom data type. If one test fails, that could lead to a confusing failure in another test when the registry complains that a registration already exists for the custom type:

https://github.com/ethereum/eth-abi/blob/master/tests/test_integration/test_custom_registrations.py#L69-L99

How can it be fixed?

We could modify those tests to use a fixture registry instance that is created per test run. If we want to continue to do true integration testing via encode_single and decode_single, we will probably need to modify the API to support use of a custom registry instance.

carver commented 6 years ago

we will probably need to modify the API to support use of a custom registry instance.

Something like an optional registry kwarg, maybe? eg~ encode_single(typ, arg, registry=None)

davesque commented 6 years ago

That seems like the most natural way to do it.

pipermerriam commented 6 years ago

I've wanted to do this the other way around.

# eth_abi/__init__.py

from eth_abi.wherever import default_registry

encode_single = default_registry.encode_single
decode_single = default_registry.decode_single
...

So in tests we can just have a registry test fixture which we can then call these functions directly (and still probably some smoke tests to ensure that the default registry works as expected).

davesque commented 6 years ago

Yep, I remember some conversations we had about that now. Seems reasonable.

carver commented 6 years ago

:+1: registry.encode_single is better