More tests for method class and cleanup from PR #2383 comments

Author: fselmo

tests/core/method-class/test_method.py

@@ -3,6 +3,9 @@
 )
 import pytest
 
+from eth_utils import (
+    ValidationError,
+)
 from eth_utils.toolz import (
     compose,
 )
@@ -69,7 +72,7 @@ def test_get_formatters_non_falsy_config_retrieval():
     first_formatter = (method.request_formatters(method_name).first,)
     all_other_formatters = method.request_formatters(method_name).funcs
     assert len(first_formatter + all_other_formatters) == 2
-    # assert method.request_formatters('eth_nonmatching') == 'nonmatch'
+    assert (method.request_formatters('eth_getBalance').first,) == first_formatter
 
 
 def test_input_munger_parameter_passthrough_matching_arity():
@@ -89,24 +92,68 @@ def test_input_munger_parameter_passthrough_mismatch_arity():
         method.input_munger(object(), ['first', 'second', 'third'], {})
 
 
-def test_input_munger_falsy_config_result_in_default_munger():
+def test_default_input_munger_with_no_input_parameters():
     method = Method(
-        mungers=[],
+        json_rpc_method='eth_method',
+    )
+    assert method.input_munger(object(), [], {}) == []
+
+
+@pytest.mark.parametrize('empty', ([], (), None), ids=['empty-list', 'empty-tuple', 'None'])
+def test_empty_input_munger_with_no_input_parameters(empty):
+    method = Method(
+        mungers=empty,
         json_rpc_method='eth_method',
     )
     assert method.input_munger(object(), [], {}) == []
 
 
 def test_default_input_munger_with_input_parameters():
     method = Method(
-        mungers=[],
         json_rpc_method='eth_method',
     )
     assert method.input_munger(object(), [1], {}) == [1]
 
 
+@pytest.mark.parametrize('empty', ([], (), None), ids=['empty-list', 'empty-tuple', 'None'])
+def test_empty_input_mungers_with_input_parameters(empty):
+    method = Method(
+        mungers=empty,
+        json_rpc_method='eth_method',
+    )
+    assert method.input_munger(object(), [1], {}) == [1]
+
+
+@pytest.mark.parametrize('empty', ([], (), None), ids=['empty-list', 'empty-tuple', 'None'])
+def test_empty_mungers_for_property_with_no_input_parameters(empty):
+    method = Method(
+        is_property=True,
+        mungers=empty,
+        json_rpc_method='eth_method',
+    )
+    assert method.input_munger(object(), [], {}) == ()
+
+
+def test_property_with_input_parameters_raises_ValidationError():
+    method = Method(
+        is_property=True,
+        json_rpc_method='eth_method',
+    )
+    with pytest.raises(ValidationError, match='Parameters cannot be passed to a property'):
+        method.input_munger(object(), [1], {})
+
+
+def test_property_with_mungers_raises_ValidationError():
+    with pytest.raises(ValidationError, match='Mungers cannot be used with a property'):
+        Method(
+            is_property=True,
+            mungers=[lambda m, z, y: 'success'],
+            json_rpc_method='eth_method',
+        )
+
+
 @pytest.mark.parametrize(
-    "method_config,args,kwargs,expected_request_result,expected_result_formatters_len",
+    "method_config,args,kwargs,expected_request_result",
     (
         (
             {
@@ -115,17 +162,15 @@ def test_default_input_munger_with_input_parameters():
             [],
             {},
             ValueError,
-            2
         ),
         (
             {
                 'mungers': [],
                 'json_rpc_method': 'eth_getBalance',
             },
-            ['unexpected_argument'],
+            ['only_the_first_argument_but_expects_two'],
             {},
             IndexError,
-            2
         ),
         (
             {
@@ -135,7 +180,6 @@ def test_default_input_munger_with_input_parameters():
             ['0x0000000000000000000000000000000000000000', 3],
             {},
             ('eth_getBalance', (('0x' + '00' * 20), "0x3")),
-            2
         ),
         (
             {
@@ -145,7 +189,6 @@ def test_default_input_munger_with_input_parameters():
             ['0x0000000000000000000000000000000000000000', 3],
             {},
             ('eth_getBalance', (('0x' + '00' * 20), "0x3")),
-            2
         ),
         (
             {
@@ -158,7 +201,6 @@ def test_default_input_munger_with_input_parameters():
             [1, 2, 3, ('0x' + '00' * 20)],
             {},
             ('eth_getBalance', (('0x' + '00' * 20), "1")),
-            2,
         ),
         (
             {
@@ -171,7 +213,6 @@ def test_default_input_munger_with_input_parameters():
             [1, 2, 3, 4],
             {},
             TypeError,
-            2,
         ),
         (
             {
@@ -181,7 +222,6 @@ def test_default_input_munger_with_input_parameters():
             ('0x0000000000000000000000000000000000000000', 3),
             {},
             ('eth_getBalance', ('0x0000000000000000000000000000000000000000', '0x3')),
-            2,
         ),
         (
             {
@@ -190,7 +230,6 @@ def test_default_input_munger_with_input_parameters():
             ('0x0000000000000000000000000000000000000000', 3),
             {},
             ('eth_getBalance', ('0x0000000000000000000000000000000000000000', '0x3')),
-            2,
         ),
         (
             {
@@ -203,7 +242,6 @@ def test_default_input_munger_with_input_parameters():
             [('0x' + '00' * 20), 1, 2, 3],
             {},
             ('eth_getBalance', (('0x' + '00' * 20), '1')),
-            2,
         ),
         (
             {
@@ -213,7 +251,6 @@ def test_default_input_munger_with_input_parameters():
             [],
             {},
             ('eth_chainId', ()),
-            2,
         ),
         (
             {
@@ -223,16 +260,15 @@ def test_default_input_munger_with_input_parameters():
             [],
             {},
             ('eth_chainId', ()),
-            2,
         ),
     ),
     ids=[
         'raises-error-no-rpc-method',
-        'test-unexpected-arg',
+        'test-missing-argument',
         'test-rpc-method-as-string',
         'test-rpc-method-as-callable',
         'test-arg-munger',
-        'test-munger-wrong-length-arg',
+        'test-munger-too-many-args',
         'test-request-formatters-default-root-munger-explicit',
         'test-request-formatters-default-root-munger-implicit',
         'test-mungers-and-request-formatters',
@@ -245,7 +281,7 @@ def test_process_params(
         args,
         kwargs,
         expected_request_result,
-        expected_result_formatters_len):
+):
 
     if isclass(expected_request_result) and issubclass(expected_request_result, Exception):
         with pytest.raises(expected_request_result):
@@ -257,7 +293,9 @@ def test_process_params(
         assert request_params == expected_request_result
         first_formatter = (output_formatter[0].first,)
         all_other_formatters = output_formatter[0].funcs
-        assert len(first_formatter + all_other_formatters) == expected_result_formatters_len
+
+        # the expected result formatters length is 2
+        assert len(first_formatter + all_other_formatters) == 2
 
 
 def keywords(module, keyword_one, keyword_two):

tests/core/module-class/test_module.py

@@ -59,17 +59,20 @@ def test_attach_methods_to_module(web3_with_external_modules):
 def test_attach_methods_with_mungers(web3_with_external_modules):
     w3 = web3_with_external_modules
 
+    # `method1` uses `eth_getBlockByNumber` but makes use of unique mungers
     w3.module1.attach_methods({
         'method1': Method('eth_getBlockByNumber', mungers=[
-            lambda _method, block_id, f, _z: (block_id, f),
-            lambda _m, block_id, _f: (block_id - 1,),
+            lambda _method, block_id, full_transactions: (block_id, full_transactions),
+            # take the user-provided `block_id` and subtract 1
+            lambda _method, block_id, full_transactions: (block_id - 1, full_transactions),
         ]),
     })
 
-    assert w3.eth.get_block(0)['baseFeePerGas'] == 1000000000
-    assert w3.eth.get_block(1)['baseFeePerGas'] == 875000000
+    assert w3.eth.get_block(0, False)['baseFeePerGas'] == 1000000000
+    assert w3.eth.get_block(1, False)['baseFeePerGas'] == 875000000
 
-    # `method1` should take a higher block number than `eth_getBlockByNumber` due to mungers and no
-    # other params should matter
-    assert w3.module1.method1(1, False, '_is_never_used_')['baseFeePerGas'] == 1000000000
-    assert w3.module1.method1(2, '_will_be_overridden_', None)['baseFeePerGas'] == 875000000
+    # Testing the mungers work:
+    # `method1` also calls 'eth_getBlockByNumber' but subtracts 1 from the user-provided `block_id`
+    # due to the second munger. So, `0` from above is a `1` here and `1` is `2`.
+    assert w3.module1.method1(1, False)['baseFeePerGas'] == 1000000000
+    assert w3.module1.method1(2, False)['baseFeePerGas'] == 875000000

web3/method.py

@@ -15,6 +15,9 @@
 )
 import warnings
 
+from eth_utils import (
+    ValidationError,
+)
 from eth_utils.curried import (
     to_tuple,
 )
@@ -62,6 +65,9 @@ def inner(args: Any) -> TReturn:
 
 
 def _set_mungers(mungers: Optional[Sequence[Munger]], is_property: bool) -> Sequence[Any]:
+    if is_property and mungers:
+        raise ValidationError("Mungers cannot be used with a property.")
+
     return (
         mungers if mungers
         else [default_munger] if is_property
@@ -70,10 +76,9 @@ def _set_mungers(mungers: Optional[Sequence[Munger]], is_property: bool) -> Sequ
 
 
 def default_munger(_module: "Module", *args: Any, **kwargs: Any) -> Tuple[()]:
-    if not args and not kwargs:
-        return ()
-    else:
-        raise TypeError("Parameters passed to method without parameter mungers defined.")
+    if args or kwargs:
+        raise ValidationError("Parameters cannot be passed to a property.")
+    return ()
 
 
 def default_root_munger(_module: "Module", *args: Any) -> List[Any]: