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

Async result warn #5742

Merged
merged 1 commit into from Aug 15, 2019
Merged

Conversation

graingert
Copy link
Member

  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Add yourself to AUTHORS in alphabetical order;

@nicoddemus
Copy link
Member

Follow up to #5734 I assume? Can you please rebase given that #5734 was merged already? Thanks

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #5742 into master will increase coverage by 0.8%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #5742     +/-   ##
========================================
+ Coverage    93.1%   93.9%   +0.8%     
========================================
  Files         117     117             
  Lines       25884   25888      +4     
  Branches     2498    2499      +1     
========================================
+ Hits        24100   24311    +211     
+ Misses       1444    1249    -195     
+ Partials      340     328     -12
Impacted Files Coverage Δ
testing/acceptance_test.py 96.49% <ø> (+0.77%) ⬆️
src/_pytest/python.py 92.79% <100%> (+0.15%) ⬆️
testing/test_terminal.py 98.43% <0%> (+0.31%) ⬆️
testing/test_config.py 99.5% <0%> (+0.33%) ⬆️
testing/python/fixtures.py 99.09% <0%> (+0.56%) ⬆️
src/_pytest/pathlib.py 90.67% <0%> (+1.03%) ⬆️
src/_pytest/assertion/util.py 92.36% <0%> (+1.14%) ⬆️
src/_pytest/capture.py 93.01% <0%> (+1.31%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ba774a...6b9d729. Read the comment docs.

@nicoddemus
Copy link
Member

Do we really need this? I mean, Python itself already issues a warning which is clear enough IMHO:

async def foo():
    pass

def test_foo():
    return foo()
====================== warnings summary ======================
test_async.py::test_foo
  c:\users\bruno.esss.co\pytest\src\_pytest\python.py:165: RuntimeWarning: coroutine 'foo' was never awaited
    testfunction(**testargs)

-- Docs: https://docs.pytest.org/en/latest/warnings.html

@nicoddemus
Copy link
Member

Oh for the future: new warnings should target features, because they might break test suites. No need to change this now because we are about to release 5.1.0. 👍

@graingert
Copy link
Member Author

I'm experiencing the problem with twisted Deferreds, however. Where they are awaitable but no error is thrown if you do not await them and there is no way to determine if a function will return one statically like inspect.iscoro...

@nicoddemus
Copy link
Member

Oh I see, thanks!

@nicoddemus nicoddemus merged commit 2d613a0 into pytest-dev:master Aug 15, 2019
@graingert graingert deleted the async-result-warn branch August 15, 2019 12:50
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 this pull request may close these issues.

None yet

2 participants