pytest-dev / pluggy

A minimalist production ready plugin system
https://pluggy.readthedocs.io/en/latest/
MIT License
1.23k stars 121 forks source link

Fix the issue where passing keyword argumentss was not effective #475

Open DeathlessDogface opened 7 months ago

DeathlessDogface commented 7 months ago

Fix the issue where passing keyword argumentss was not effective, and enhanced the test cases for the _multicall() function

The issue like this:

test code

import pluggy
import pytest
hookspec = pluggy.HookspecMarker("myproject")
hookimpl = pluggy.HookimplMarker("myproject")

class MySpec:
    """A hook specification namespace."""

    @hookspec
    def myhook(self, arg1, arg2):
        """My special little hook that you can customize."""

class Plugin_1:
    """A hook implementation namespace."""

    @hookimpl
    def myhook(self, arg1, arg2=1):
        print("inside Plugin_1.myhook()")
        return arg1 + arg2

class Plugin_2:
    """A 2nd hook implementation namespace."""

    @hookimpl
    def myhook(self, arg1, arg2=1):
        print("inside Plugin_2.myhook()")
        return arg1 - arg2

def test_case():
    # create a manager and add the spec
    pm = pluggy.PluginManager("myproject")
    pm.add_hookspecs(MySpec)

    # register plugins
    pm.register(Plugin_1())
    pm.register(Plugin_2())

    # call our ``myhook`` hook
    results = pm.hook.myhook(arg1=1, arg2=2)
    assert results == [1 - 2, 1 + 2]

test result

(venv) PS C:\Code\DXN4\demo> pytest .\pluggy_demo2.py         
========================== test session starts ============================
        pm.register(Plugin_1())
        pm.register(Plugin_2())

        # call our ``myhook`` hook
        results = pm.hook.myhook(arg1=1, arg2=2)
>       assert results == [1 - 2, 1 + 2]
E       assert [0, 2] == [-1, 3]
E         At index 0 diff: 0 != -1
E         Use -v to get more diff

pluggy_demo2.py:44: AssertionError
--------------------------------------------------------------------------------------------------------------- Captured stdout call ---------------------------------------------------------------------------------------------------------------- 
inside Plugin_2.myhook()
inside Plugin_1.myhook()
======================== short test summary info ============================= 
FAILED pluggy_demo2.py::test_case - assert [0, 2] == [-1, 3]
========================== 1 failed in 0.15s ================================ 
(venv) PS C:\Code\DXN4\demo>
DeathlessDogface commented 7 months ago

I'm torn on this one, as we have a pending design/implementation action for specifically choosen default values (for backward/forward compatibility of hook impl's/specs as parameters are added/removed

part of me wants to formally forbid default values other than None so they can specifically be used to manage addition/removal of parameters in specs/impls

I'm sorry, I just saw this reply. Now I understand the comments in the code, thank you. However, I would still like to recommend my changes to you again.