Allow state handler functions with default arguments in v3

[lotruheawea]

Have you read the Contributing Guidelines on issues?

• I have read the Contributing Guidelines on issues.

Description

In version 3, state handler functions need to have the arguments as described by StateHandlerFull, StateHandlerNoAction and so on. In function _set_function_state_handler and _state_handler_dict, there are checks on how many parameters there are in the function, e,g. by having len(inspect.signature(f).parameters) != 2 calls.

I would like to replace this by something like this

parameters = inspect.signature(handler).parameters
parameters_without_defaults = {k: v for k, v in parameters.items() if
                                  v.default is v.empty}
...
if len(parameters_without_defaults) != 2:
    ...

This would generate more flexibility in defining state_handler functions. The TypeAliases would need be extended.

Motivation

The current implementation forbids the usage of extra parameters to be passed on to state handler functions. However, this might be a requirement when fixtures are used. E.g. in test_provider in the test_01_fastapi_provider.py, it might not be sufficient to use only the server as a fixture, but another one, where we pass the database object that we want to modify by the state handler. This requirement is definitely there for our use case.

This currently cannot be passed on to the state_handler_function as fixtures are not available in normal functions. It could be relieved by using functools.partial. The test_provider_code could then look like:

def test_provider(server: str, my_fixture) -> None:
    partial_state_handler = partial(
        provider_state_handler, my_extra_arg=my_fixture
    )

    verifier = (
        Verifier("v3_http_provider")
        .add_transport(url=server)
        .add_source("examples/pacts/v3_http_consumer-v3_http_provider.json")
        .state_handler(partial_state_handler, teardown=True)
    )
    verifier.verify()

def provider_state_handler(
    state: str,
    action: str,
    _parameters: dict[str, Any] | None,
    my_extra_arg: Any | None = None
) -> None:
    if action == "setup":
        mapping = {
            "user doesn't exists": mock_user_doesnt_exist,
            "user exists": mock_user_exists,
            "the specified user doesn't exist": mock_post_request_to_create_user,
            "user is present in DB": mock_delete_request_to_delete_user,
        }
        # TODO: there might be a cleaner and more general approach here
        if my_extra_arg is None:
            mapping[state]()
        else:
            mapping[state](my_extra_arg)

    if action == "teardown":
        mapping = {
            "user doesn't exists": verify_user_doesnt_exist_mock,
            "user exists": verify_user_exists_mock,
            "the specified user doesn't exist": verify_mock_post_request_to_create_user,
            "user is present in DB": verify_mock_delete_request_to_delete_user,
        }
        mapping[state]()

I am unsure if this feature has been requested by other users. For us, it is a blocker.

Have you tried building it?

I can definitely create a PR for this, if approved by the maintainers.
I need support on how to run the complete test suite though. I currently still get many

FileNotFoundError: [Errno 2] No such file or directory: './pact-python/tests/v3/compatibility_suite/definition/features/V1/http_consumer.feature'

errors when running hatch run test. The definition subfolder is completely empty.

Also I am unsure how to extend the TypeAliases, to include optional default arguments.

Self-service

• I'd be willing to contribute this feature to Pact Python myself.

[YOU54F]

@JP-Ellis would be better to advise on the python implementation, however with regards to this comment.

I need support on how to run the complete test suite though.

The specific files you are missing, are part of the pact compatibility suite which is a git submodule, which requires initialisation. It is noted in the contributing guide :)

https://github.com/pact-foundation/pact-python/blob/main/CONTRIBUTING.md#installation

You will also need to run git submodule init if you want to run tests, as Pact Python makes use of the Pact Compatibility Suite.

[lotruheawea]

@YOU54F , thank you for your reply. I actually did that and it is still missing. The folder compatibility_suite is there (with subfolders util and definition) but the subfolder definition is empty.

[JP-Ellis]

Thanks for bringing this to my attention!

On reflection, I do agree that by naïvely inspecting the number of parameters only, there are a number of edge cases that are not accounted for. In fact, thinking of the various situations, even just checking for arguments which don't have default won't be quite sufficient, as it is possible to have a keyword-only required argument which would not work in the underlying function call as all arguments are passed as position arguments.

On further thinking about it, there's also the edge-case of *args and **kwargs. I think the following would take into account possible variations/combinations of parameter kinds:

import inspect

def f(a: int = 1, /, b: int = 2, *, c: int = 3): ...
def g(a: int, /, b: int, *, c: int): ...
def h(a: int, /, b: int, *args, c: int, **kwargs): ...

for fn in [f, g, h]:
  print("Function:", fn.__name__)
  for param in inspect.signature(fn).parameters.values():
      print(f"- {param.name} ({param.kind}): {param.default}")

Function: f
- a (POSITIONAL_ONLY): 1
- b (POSITIONAL_OR_KEYWORD): 2
- c (KEYWORD_ONLY): 3
Function: g
- a (POSITIONAL_ONLY): <class 'inspect._empty'>
- b (POSITIONAL_OR_KEYWORD): <class 'inspect._empty'>
- c (KEYWORD_ONLY): <class 'inspect._empty'>
Function: h
- a (POSITIONAL_ONLY): <class 'inspect._empty'>
- b (POSITIONAL_OR_KEYWORD): <class 'inspect._empty'>
- args (VAR_POSITIONAL): <class 'inspect._empty'>
- c (KEYWORD_ONLY): <class 'inspect._empty'>
- kwargs (VAR_KEYWORD): <class 'inspect._empty'>

In the case were there might be VAR_POSITIONAL arguments, we also cannot make any assertions at runtime.

I think in terms of the refactor, it would be best to create a separate utility function to check these parameters and ensure that:

• There are no required keyword arguments
• There are at least n positional arguments
• There are no more than n required positional arguments
• If there are VAR_POSITIONAL arguments, then we maybe raise a info`-level warning (it would be common enough not to warrant a warning-level warning)

There's also the question of the type annotations; and that's something I'm not entirely sure how to best handle. I might have to play around with that a bit, but it might be sufficient to simply add ... in the function arguments.

Lastly, just to make it even more fun, there's also the need to take into account async functions (as requested on Slack, and I'm about to create a ticket here to track that).

[github-actions]

🤖 Great news! We've labeled this issue as smartbear-supported and created a tracking ticket in PactFlow's Jira (PACT-3871). We'll keep work public and post updates here. Meanwhile, feel free to check out our docs. Thanks for your patience!

[lotruheawea]

@JP-Ellis , you assigned this ticket to yourself. I initially proposed to submit a PR, would this still be helpful?

[JP-Ellis]

I've actually got something mostly working now on my computer, with a PR coming soon (most likely tomorrow)

I hope I didn't accidentally duplicate your work 🫤