bloomberg / pytest-memray

pytest plugin for easy integration of memray memory profiler
https://pytest-memray.readthedocs.io/en/latest/
Apache License 2.0
335 stars 24 forks source link

`pytest-memray` raises `pytest.PytestReturnNotNoneWarning` warnings #60

Closed WilliamJamieson closed 1 year ago

WilliamJamieson commented 1 year ago

Bug Report

I would like to begin using pytest-memray as part of the test suite for the astropy package. However, when running the test suite of astropy with the --memray flag most of the astropy tests fail with a warning like

pytest.PytestReturnNotNoneWarning: Expected None, but ..., which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?

Note that astropy by default turns all warnings into errors, and does not wish to change this behavior.

Is it expected behavior for pytest-memray to attempt to return a value from each unit test? If so this runs contrary to pytest-dev/pytest#7337.

Alternatively, could this issue be an interaction between pytest-memray and some of the other pytest plugins used by astropy?

Input Code

A test PR: astropy/astropy#14090 has been opened on astropy to introduce pytest-memray into its testing. One can see the full set of failures in this CI job: https://github.com/astropy/astropy/actions/runs/3595032036/jobs/6054041039.

To reproduce this issue without running the full astropy test suite it is sufficient to run:

pytest --memray astropy/constants/tests/test_constant.py

after cloning https://github.com/WilliamJamieson/astropy/tree/CI/memray and installing it with pip install -e ".[test]". Note that removing https://github.com/WilliamJamieson/astropy/blob/351db18ee700484fff03f202415834613a9bccda/setup.cfg#L135 will turn off astropy turning warnings into errors and allow everything to pass, but with all the warnings being raised.

Expected behavior/code

We expect pytest --memray when run on astropy to run without raising pytest.PytestReturnNotNoneWarning warnings.

Environment

pablogsal commented 1 year ago

@gaborbernat can you give this a go? I think is just that we are returning the result from the hook when we should not return anything

pablogsal commented 1 year ago

@WilliamJamieson thanks a lot for opening the issue! We will address this ASAP!

Apart from this do you have any other feedback positive or negative of using the plugin or the profiler directly? 🙏

WilliamJamieson commented 1 year ago

I have been using memray for a few projects for awhile (in particular https://github.com/spacetelescope/jwst and its dependencies) and it works really well.

We have been struggling with how to deal with memory usage profiling (and finding places for improving memory usage) for https://github.com/spacetelescope/jwst (as memory usage has become an issue for us). Additionally, finding some pesky memory leaks in it for awhile now. When I saw pytest-memray mentioned here I thought I would try it out, so I am very much in the phase of just trying to get it working within our stack.

Note that in particular, #52 would be extremely useful when running as part of our regression suite, and it is one of the main reasons I am experimenting with pytest-memray.

pablogsal commented 1 year ago

Fantastic! Let's see if we can fix this for you as well as iterate in the leaks plugin to ensure it works as best as possible.

Btw, regarding the profiling itself, in case you have some spare cycles (please don't feel forced or anything) could you leave a small comment here:

https://github.com/bloomberg/memray/discussions/226

About your experience using memray in the projects you mentioned? We are trying to collect all feedback possible and the different ways people use memray and a small comment will make a huge difference.

Again, please don't feel forced or anything :)