DM-22199: Add decorators for iterating over tests #76
Conversation
If you don't specify an exception, generally you want to catch any exception, so it should default to Exception rather than AssertionError (a test failure).
PaulPrice
force-pushed
the
tickets/DM-22199
branch
from
aee1dab
to
e7b99e1
Compare
python/lsst/utils/tests.py
Outdated
| Note that the values are embedded in the class name. | ||
| """ | ||
| def decorator(cls): | ||
| num = len(next(iter(settings.values()))) |
There was a problem hiding this comment.
There is a lot going on in this function with lots of lengths turning up so some commentary would be nice. Maybe "Number of elements in first parameter provided"? I think that if you call this with a str value that is not a list you will end up with the length of the string as this num so we probably want to handle that nicely since they probably made a mistake rather than expecting a test class for each letter.
We often iterate over multiple values in tests. We have been writing this iteration explicitly, which is annoying (for classes, this involves subclassing; for methods, this means the values that caused a failure aren't clear unless special efforts are made). These decorators handle the iteration for the user, making it much easier to write and understand tests.
PaulPrice
force-pushed
the
tickets/DM-22199
branch
from
5bb6cc6
to
105d512
Compare
| """ | ||
| num = len(next(iter(settings.values()))) # Number of settings | ||
| for name, values in settings.items(): | ||
| if isinstance(values, str): |
There was a problem hiding this comment.
I think you have to do this as a separate loop, first. If the first item in settings is a string, num will get set to the length of that string instead of 1 and will then trigger the assertion.
There was a problem hiding this comment.
Well spotted! I've pushed a fix.
No description provided.