eth-abi icon indicating copy to clipboard operation
eth-abi copied to clipboard

Maybe use a registry fixture to test custom registrations

Open davesque opened this issue 6 years ago • 5 comments

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.

davesque avatar Jun 05 '18 20:06 davesque

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)

carver avatar Jun 05 '18 21:06 carver

That seems like the most natural way to do it.

davesque avatar Jun 05 '18 21:06 davesque

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).

pipermerriam avatar Jun 06 '18 03:06 pipermerriam

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

davesque avatar Jun 06 '18 03:06 davesque

:+1: registry.encode_single is better

carver avatar Jun 06 '18 18:06 carver