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.

Sign up for GitHub

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

Jump to bottom

PytestReturnNotNoneWarning test_security_updates.py::test_should_run #6694

Open
legoktm opened this issue Nov 30, 2022 · 3 comments · May be fixed by #6696
Open

PytestReturnNotNoneWarning test_security_updates.py::test_should_run #6694

legoktm opened this issue Nov 30, 2022 · 3 comments · May be fixed by #6696
Labels
good first issue

Comments

@legoktm
Copy link
Member

legoktm commented Nov 30, 2022

This is a good first issue for new contributors to take on, if you have any questions, please ask on the task or in our Gitter room!

Description

Because the function is named test_should_run, pytest treats it as a test rather than a function.

Steps to Reproduce

make build-debs

Expected Behavior

No warnings

Actual Behavior

    builder-focal/tests/test_security_updates.py::test_should_run
      /home/user/QubesIncoming/dev/securedrop/.venv/lib/python3.9/site-packages/_pytest/python.py:199: PytestReturnNotNoneWarning: Expected None, but builder-focal/tests/test_security_updates.py::test_should_run returned False, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
        warnings.warn(

Comments

Renaming it to should_run() would fix this.
@evansandoval
Copy link

evansandoval commented Dec 1, 2022 •
edited
Loading

Hello! I've never contributed to an open source project, but I'd love to try this out. Is the only thing I need to do rename the function? I was just wondering what the scope of the issue was -- do I need to find where the function is run in other files?

@sssoleileraaa
Copy link
Contributor

Hi @evansandoval, that's correct. After renaming the function, you'll want to update wherever that function is called to use the new name so that the tests don't break. You can also open up a draft PR to see that the tests still pass after the name change.

wallbloggerbeing added a commit to wallbloggerbeing/securedrop that referenced this issue Dec 1, 2022
@wallbloggerbeing
Changed variable names
3b58fdc 
Adressing Issue freedomofpress#6694
Changed names of variables to not treat the functions as PyTest
@wallbloggerbeing wallbloggerbeing linked a pull request Dec 1, 2022 that will close this issue
Open
@legoktm
Copy link
Member Author

legoktm commented Dec 1, 2022

Hi @evansandoval, someone else already submitted a PR for this one :/ Would you be interested if I found another good first issue for you to work on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Changed variable name wallbloggerbeing/securedrop
3 participants
@legoktm @sssoleileraaa @evansandoval