wolever / parameterized

Parameterized testing with any Python test framework
Other
836 stars 105 forks source link

Meaningful class names for tests when using parameterized_class with dictionaries (suggestion) #87

Closed erezsh closed 4 years ago

erezsh commented 4 years ago

Right now to get meaningful names, I have to use the tuple syntax:

@parameterized_class(("name", "optimized"), [
    ("Normal", True),
    ("Unoptimized", False)
])

However, I would prefer to use the dict syntax, as it's more flexible and less error-prone for me. But, that would result in unnamed class variations, according to current implementation.

I think you should just take the first key of every dictionary, as all dictionaries in Python3 are ordered (and there is a Python2 backport), and it matches how the function currently works.

Alternatively, you can allow an optional argument that specifies what is the "name" key.

matthijskooijman commented 4 years ago

This indeed does not seem to be possible, relevant code is here.

A related suggestion would be to allow excluding the numerical suffix (e.g. _0) which is now applied in addition to the name suffix (making it consistent with the @parameterized.expand, which only applies the name/string suffix when present).

erezsh commented 4 years ago

Yes, that would also help. I have to keep remembering the order of the tests just so I can run them individually, because each one has a number, and that isn't necessary.

Btw I'd be very happy to submit a Pull Request for this, if I knew that it would be accepted.

matthijskooijman commented 4 years ago

making it consistent with the @parameterized.expand, which only applies the name/string suffix when present

Oh, this is actually not true, it also adds the numerical suffix by default. I misread the code before, which is here: https://github.com/wolever/parameterized/blob/39ab19f464e860b144b8d082d1b7935483a3f25f/parameterized/parameterized.py#L259-L264

Regardless, omitting the numerical suffix seems useful to me in both cases.

wolever commented 4 years ago

This is a great idea!

My thoughts:

  1. IMO copying the behaviour of @parameterized.expand makes sense, if only for consistency.

  2. For Python 2 compat, I feel like a reasonable algorithm would be "use the name key if one exists, otherwise sort keys alphabetically and take the first string value" (although I'd be happy with anything reasonable here).

Thanks!

erezsh commented 4 years ago

Ideally you would refactor expand and parameterized_class to use the same function for creating names.

But for now, I'll just create a PR for the specific things I need (in a few days when I find the time).

wolever commented 4 years ago

Actually, it looks like I've got to tweak the code for parameterized class names anyway, so I can get this fix in shortly 🙂

wolever commented 4 years ago

I've push'd this in v0.7.4. Please take a look and let me know what you think!

erezsh commented 4 years ago

Yeah, seems to be working