Breaking behavior in latest release (1.0.1)

[Kuwagata]

A change in the latest release of ddt (#22 included in 1.0.1) has seemingly introduced breaking behavior for wrapped tests expecting a dictionary object when receiving test data, as exemplified below:

test_data.json

{
  "test_name": {
    "arg_one": "test1",
    "arg_two": "test2",
    "arg_three": "test3"
  }
}

Original test code:

@ddt.file_data('test_data.json')
def test_something(inputs):
    arg_one = inputs["arg_one"]
    arg_two = inputs["arg_two"]
    arg_three = inputs["arg_three"]
    # Perform tests here
    pass

New test code:

@ddt.file_data('test_data.json')
def test_something(arg_one, arg_two, arg_three):
    # Perform tests here
    pass

Since this behavior appears to be non-configurable and backward-incompatible, this seems more like a major version change rather than a simple patch. Can this change be reverted at least for the time being?

[txels]

Thanks for pointing this out. I didn't realise that was breaking existing behaviour.
I will see how to deal with that. In the meantime, pin your dependency to 1.0.0 if that gets you going, as I'm not sure when I can get some time to revert this/sort this out.

[Kuwagata]

I've pinned it for now, so that should be enough to keep things rolling.

Thanks @txels !

[pradyunsg]

What's the status of this?

[txels]

@pradyunsg nothing has been done on this yet. Do you want to take a look?

[pradyunsg]

AFAICT, this behavior shouldn't have changed in a patch-version bump. That was a bad mistake.

Since then, another minor version bump has occurred. I recommend the OP to port their tests now, since it's been a while since this change happened. Currently, some people might be using the newer versions (1.0.1+) and it won't be nice to suddenly break their code, intentionally.

It might be easy to write a decorator that handles both cases seamlessly for OP, if the expected number of arguments is 1 then pass a dictionary, if it's more, pass them as keyword arguments.

PS: I am indeed working on parts of this project. Something along the lines of #29, with some other ideas I have in my head. Once that's done, I would like to see all those changes come out as ddt 2.0.0.

---

I just wrote this decorator... It should work with Py2/3. It's untested though. You can possibly remove the conditionals in the start of the function if you are going to selectively apply only on your ddt test functions using the old format. Actually, that makes more sense. Because then you would clearly mark which test would need updating.

def adapt_if_needed(func):
    """ Adapt functions expecting ddt's old argument format to run with the newer one.
    """

    argspec = inspect.getargspec(func)

    # If it catches variable arguments, it's probably fine to ignore it
    if argspec.varargs is not None or argspec.keywords is not None:
        return func

    # Count required arguments
    required_args = len(argspec.args)
    if argspec.defaults:
        required_args -= len(argspec.defaults)

    # If it takes more than one argument, it's probably fine.
    if required_args > 1:
        return func

    # adapt for older version.
    @functools.wraps(func)
    def wrapper(**kwargs):
        func(kwargs)

    return wrapper

[BenjamenMeyer]

In all honesty, looking at PR #22 which is what seems to have caused this issue I would really suggest outright reverting #22 and then just telling people to break down the dictionary themselves. There's nothing stopping them from using a dictionary to manage how many entries they have and splitting it out doesn't really add much code to test.

Though if they really want to keep PR #22's functionality, perhaps it shouldn't be part of unpack but it's own unpack_dict to make that kind of functionality explicit.

@pradyunsg sorry...I'm not really seeing benefit to your proposed migration as it seems lacking in matching up arguments especially due to the assumption that a test that previously worked (as @Kuwagata exemplified) would only have 1 parameter, and thinking it through it would seem to me at least not an easy problem to try to determine how many parameters should be able to be matched up. So making the dict unpacking done via its own specific decorator would seem to be the most logical path. Anyone that has come to rely on the new methodology should be able to migrate easily to using a new decorator, which is what you're asking people that have been using the old methodology to do any way. And my guess is that people using the new method are probably fewer in number than people using the old method.

[pradyunsg]

That sounds good too! Having a special decorator for dictionaries sounds like a nice idea. I would like a release period with the addition of the new decorator and a clear warning that the behavior shall change in the next minor version bump.

[pradyunsg]

Bump. Could @txels make this change?

[wswld]

I think as it is 2018 and whoever is using the lib now is probably using the newer approach anyway. I'm closing this for now.