Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pytest-memray raises pytest.PytestReturnNotNoneWarning warnings #60

Closed
WilliamJamieson opened this issue Dec 1, 2022 · 4 comments · Fixed by #61
Closed

pytest-memray raises pytest.PytestReturnNotNoneWarning warnings #60

WilliamJamieson opened this issue Dec 1, 2022 · 4 comments · Fixed by #61

Comments

@WilliamJamieson
Copy link

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

  • Python(s): python3.8, python3.9, python3.10, python3.11
  • pip freeze for minimal python3.10 tests
astropy @ file:///home/runner/work/astropy/astropy/.tox/.tmp/package/1/astropy-5.3.dev53%2Bg483b8cb63.tar.gz
attrs==22.1.0
commonmark==0.9.1
coverage==6.5.0
exceptiongroup==1.0.4
execnet==1.9.0
hypothesis==6.58.2
iniconfig==1.1.1
Jinja2==3.1.2
MarkupSafe==2.1.1
memray==1.4.1
numpy==1.23.5
packaging==21.3
pluggy==1.0.0
psutil==5.9.4
pyerfa==2.0.0.1
Pygments==2.13.0
pyparsing==3.0.9
pytest==7.2.0
pytest-arraydiff==0.5.0
pytest-astropy==0.10.0
pytest-astropy-header==0.2.2
pytest-cov==4.0.0
pytest-doctestplus==0.12.1
pytest-filter-subpackage==0.1.1
pytest-memray==1.3.2
pytest-mock==3.10.0
pytest-openfiles==0.5.0
pytest-remotedata==0.3.3
pytest-xdist==3.0.2
PyYAML==6.0
rich==12.6.0
sortedcontainers==2.4.0
tomli==2.0.1
@pablogsal
Copy link
Member

@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
Copy link
Member

@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
Copy link
Author

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
Copy link
Member

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:

bloomberg/memray#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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants