fix(injector): resolve issue with Injector requires all annotations #188
Conversation
injector library requires all annotation of function available in the runtime, not only dependencies under "Inject" annotation.
|
Will fix tests and linting |
|
The changes here look good at first glance, thoguh I'm not overly familiar with the injector library. Do you happen to have experience with it @Daverball? I think I might be able to take a look at this tonight otherwise 👍 |
|
@sondrelg I have never used it myself, so I can't really provide any useful input beyond a surface review. |
|
I would be glad if someone would check this rather than me. I updated the pull request with a bug found by the college because It feels like a general rule for |
| ), | ||
| ], | ||
| ) | ||
| @pytest.mark.parametrize(('enabled', 'expected'), [(True, set())]) |
There was a problem hiding this comment.
parametrize hardly makes sense if you only provide one parametrization, also the docstring and test name no longer make any sense. It might be better to refactor and merge some of these tests into more of a regression style test where you provide a list of source code samples and the expected output, similar to the TCXXX test cases. That way it will also be easier to cover all the different kinds of arguments (i.e. *args in addition to **kwargs, and signatures that use both * and / in them)
injector library requires all annotation of function available in the runtime, not only dependencies under "Inject" annotation.