lato icon indicating copy to clipboard operation
lato copied to clipboard

tests fail after adding types to test helper function

Open toinbis opened this issue 1 year ago • 2 comments

https://github.com/pgorecki/lato/blob/18c2afa419f195a380db6f447a292bd3e00fb30a/tests/test_transaction_context.py#L8

after modifying tests/test_transaction_context.py

from

def add(a, b):
    return a + b

to

def add(a: int, b: int) -> int:
    return a + b

test suite fails:

========================================================== short test summary info ===========================================================
FAILED tests/test_transaction_context.py::test_call_with_dependencies - assert 4 == 3
FAILED tests/test_transaction_context.py::test_call_with_kwarg_and_dependency - assert 22 == 12
======================================================== 2 failed, 43 passed in 0.16s ========================================================

P.S. Thanks a lot for creating and open source'ing python-ddd repo ant lato! Welki dzeki! :)

toinbis avatar Apr 01 '24 17:04 toinbis

This problem arises due to the limitations of https://github.com/pgorecki/lato/blob/4c5f0ece381f331ab5a5d97a40bba2b7bf27cb31/lato/dependency_provider.py#L112

Function arguments are first resolved by type, then by name. After your modification, both a and b have int type, so type resolution (resolve by int and int) takes precedence over name resolution (resolve by a and b), which leads to incorrect values being injected during a function call. For sure this logic could be more sophisticated, but at this point I have no plans for working on it, but please feel free to contribute :)

As a workaround, you can implement your own DependencyProvider that implements your own resolve_func_params and instantiate an app or transaction context with it using

app = Application(dependency_provider=CustomDependencyProvider(...))

pgorecki avatar Apr 02 '24 09:04 pgorecki

Thanks @pgoreck for prompt and clear answeri ! Need to think how I want to approach this :) Kindly please let's keep this issue open for a little bit while. Thanks!

toinbis avatar Apr 02 '24 12:04 toinbis