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

🐛 Fix internal Trio test warnings #5547

Merged
merged 4 commits into from Oct 31, 2022
Merged

🐛 Fix internal Trio test warnings #5547

merged 4 commits into from Oct 31, 2022

Conversation

samuelcolvin
Copy link
Sponsor Collaborator

@samuelcolvin samuelcolvin commented Oct 27, 2022

Hi, without this fastapi is failing again due to trio warnings while testing. See #5545 and pydantic/pydantic#4662.

There are two problems here (or one causing two issues):

There's a runtime warning from trio:

RuntimeWarning: You seem to already have a custom sys.excepthook handler installed. I'll skip installing Trio's custom handler, but this means MultiErrors will not show full tracebacks.

This was being raised by some tests, but also by the warning line 'ignore::trio.TrioDeprecationWarning', - hence the new warning ignore has to come before that line.

I've no idea what change is causing this, would be great if every test dep could be frozen, e.g. with pip-compile, so these issues don't keep coming up.

Copy link
Contributor

@iudeen iudeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to freeze test dependencies as samuelcolvin mentioned.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2022

@github-actions github-actions bot temporarily deployed to pull request October 28, 2022 12:06 Inactive
@github-actions
Copy link
Contributor

📝 Docs preview for commit 798da51 at: https://635bc607486c4f0cd687c4cd--fastapi.netlify.app

@samuelcolvin
Copy link
Sponsor Collaborator Author

@tiangolo you probably want to remove pytest-cov, I understand it's no longer maintained and can be replaced by just

coverage run -m pytest

I haven't removed it here since you've set some configuration options that will need to move to pyproject.toml.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (67b6679) compared to base (f67b19f).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #5547   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          540       540           
  Lines        13946     13946           
=========================================
  Hits         13946     13946           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions github-actions bot temporarily deployed to pull request October 28, 2022 12:13 Inactive
@github-actions
Copy link
Contributor

📝 Docs preview for commit 67b6679 at: https://635bc7905025240e73fc0f8d--fastapi.netlify.app

@jujumilk3
Copy link
Contributor

jujumilk3 commented Oct 29, 2022

This PR needed to me too. thx.
I applied it to #4573.

jujumilk3 added a commit to jujumilk3/fastapi that referenced this pull request Oct 29, 2022
@tiangolo tiangolo changed the title fix more trio test warnings 🐛 Fix internal Trio test warnings Oct 31, 2022
@tiangolo
Copy link
Owner

Thanks @samuelcolvin! 🙇

Indeed, it's a good idea to lock the dependencies for tests (not the library ones), although I'm not sure how to integrate that with Hatch, I know Flit didn't support locking either, Poetry is the only one I know that supports locks.

Also not sure how to automatize all that so that it's automated in CI. 🤔 But anyway, I'll think about it and keep an eye out to see if there's a way to improve that here.

And thanks for the pointer about pytest-cov! I took a note about it here.

@tiangolo tiangolo merged commit ee4b2bb into master Oct 31, 2022
@tiangolo tiangolo deleted the fix-test-warnings2 branch October 31, 2022 13:36
@samuelcolvin
Copy link
Sponsor Collaborator Author

👍 thanks @tiangolo.

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

Successfully merging this pull request may close these issues.

None yet

5 participants