search

fastapi / typer

Typer, build great CLIs. Easy to code. Based on Python type hints.
https://typer.tiangolo.com/
MIT License
15.66k stars 666 forks source link

Run tests with warnings treated as errors #379

Open davidism opened 2 years ago

davidism commented 2 years ago

To avoid unexpected incompatibilities with Python and dependencies, it is recommended to run tests with warnings treated as errors. Many libraries, and Python itself, issue DeprecationWarning messages when deprecated code is used, so that libraries can be notified early and adapt.

Click 8.0 deprecated some code that Typer used, and the next feature release 8.1 removed the previously deprecated code. Click issued deprecation warnings during this period which explained the deprecation, when it would be removed, and what to use instead. Typer wasn't testing warnings, so it didn't see this and wasn't able to make a proactive compatibility fix.

Note that this isn't specific to code in Click. Python itself, or dependencies in test/docs environments, may also change in ways that can be detected if warnings are treated as errors. It's also a good idea to build docs with that setting for the same reason.

davidism commented 2 years ago

This can be accomplished by adding a pytest.ini file with the following:

[pytest]
filterwarnings =
    error

If there are certain warnings that you want to deliberately keep, you can use with pytest.warns(...) to test them. If there are certain warnings that you deliberately want to ignore, you can add rules for them to filterwarnings, see https://docs.python.org/3/library/warnings.html#warning-filter.

Mkdocs doesn't appear to have a command line switch for warnings as errors like Sphinx does, but you should be able to set the PYTHONWARNINGS=error env var to do the same thing.

jofegan commented 2 years ago

Interesting. I enabled this in my own project just now. It immediately flagged two of my unit tests for not explicitly closing file descriptors - which was true. I added calls to fd.close() and it was happy with that, which actually improved the tests because the functions they are testing are expected to return open file descriptors, and now I'm explicitly closing them which is a test that they were open, if you see what I mean.

gabrieldemarmiesse commented 2 years ago

It can be nice to have 2 CI runs in parralel. One with warnings and one without, for better feedback about what went wrong for contributors