Re: [libgpiod][PATCH 18/22] bindings: python: tests: add missing type annotations

[Vincent Fazio <vfazio@xxxxxxxxx>]

On Tue, Oct 8, 2024 at 8:32 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
>
> I admit I don't know any better but does it really make sense to do it
> for individual test cases?

Some projects type their tests while others don't make it a priority. The way I
see it, the overhead is minimal when adding a new test so why not add it.

One of the caveats is that mypy does not type check the bodies of functions
that have untyped function definitions. So:

  def _internal_fn(arg: str) -> str:
    return arg

  def test_fn():  # will not be type checked by default
    _internal_fn(10)  # so this will not raise a type error

It seems important to ensure the test cases are either abiding by the typed
library interface or knowingly using invalid arguments to test "negative" cases
(type errors which are suppressed in patch 19) for callers who do not leverage
or ignore the library's type annotations.

When fixing the type annotations for gpiod, I used the tests and examples as a
reference for what the call interface is expected to support.

For the situation above re untyped function definitions, mypy does have a knob
that allows type checking the bodies of untyped functions. If we used that, it
could be argued that the majority of this patch could be dropped.

So, I guess some questions:

Do you want the test suite type checked?

If not, a lot of the patches touching tests can be dropped

If you do, to what degree? Do you want function bodies type checked?

For example, mypy identified the problems in patch 14 and 16. Even if untyped
functions aren't checked, those problems would still have been flagged.

If you think there's value in type checking functions, do you want to use
explicitly typed function signatures or leverage `check_untyped_defs` [0] (this
could be added to the mypy configuration in pyproject.toml).


>
> Bart

[0]: https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-check-untyped-defs